The reflow debugging tool GECKO_DISPLAY_REFLOW_RULES_FILE doesn't work when the preference security.sandbox.content.level is 3 (which is now the default on mozilla-central). This is because it relies on reading the file given in the environment variable, in the content process. Using --setpref security.sandbox.content.level=2 works as a workaround. I only actually ever use this to point to a file whose contents are "* 1"; I'm not sure if other layout developers use anything fancier. It's possible we could restructure the debugging to not use a file, but just have the rules in the environment variable. cc:ing a few other people who might use the pref to see what they think about that.

(In reply to David Baron :dbaron: ⌚️UTC-7 from comment #0) > I only actually ever use this to point to a file whose contents are "* 1"; (Same with me -- though I rarely use this feature (though I perhaps should use it more often)). > It's possible we could restructure the debugging to not use a file, > but just have the rules in the environment variable. That seems like a good idea to me. (That might require simplifying the syntax a bit -- to be honest I don't recall the current syntax beyond "* 1" -- but that's perhaps worth doing anyway.)

I use it very rarely, and haven't used any fancy rules IIRC. Removing _FILE from the name and making it contain the rules directly sounds good to me. A simpler alternative would be to just exit with an error message if GECKO_DISPLAY_REFLOW_RULES_FILE is set but opening the file failed, with a suggestion that the above --setpref might be needed?

Too late for fixing in 59. Setting as fix-optional to exclude it from triage, since it has a P3 priority.

I'm still learning about reflow, so I'm interested in using GECKO_DISPLAY_REFLOW_RULES_FILE to better understand what's happening in bug 1452277. I hit the sandbox issue when I tried to rr-record a session. As a quick&dirty local fix, before trying to open the file, I handle some special names that don't actually access any files, e.g.: - GECKO_DISPLAY_REFLOW_RULES_FILE="" -> Logs everything like "* 1". - GECKO_DISPLAY_REFLOW_RULES_FILE="c" -> Logs everything, but only from the content process; nice to reduce output. Anyone interested? Thoughts/suggestions? (In reply to Mats Palmgren (:mats) from comment #2) > Removing _FILE from the name and making it contain the rules directly > sounds good to me. I'd prefer not to have to parse the envvar string (at least as a first quick useful patch here, it could still be done later on if someone needs it) -- It doesn't look trivial to implement, because the parser works with a FILE*. > A simpler alternative would be to just exit with an error message if > GECKO_DISPLAY_REFLOW_RULES_FILE is set but opening the file failed, > with a suggestion that the above --setpref might be needed? It already MOZ_CRASHes; we could add the --setpref suggestion to the message there.

This looks pretty reasonable to me, although I don't think it's worth distinguishing between XRE_IsParentProcess() and XRE_IsE10sParentProcess() (the latter being a subset of the former). Maybe just use a/p/c?

True, thanks for the suggestion, will do.

Comment on attachment 8989329 [details] Bug 1407059 - GECKO_DISPLAY_REFLOW_PROCESSES=a/p/c enables reflow logging in chosen processes - https://reviewboard.mozilla.org/r/254396/#review261214 ::: layout/doc/frame_reflow_debug.html:48 (Diff revision 2) > +<ul> > +<li><code>set GECKO_DISPLAY_REFLOW_PROCESSES=a</code> to log from all processes > +<li><code>set GECKO_DISPLAY_REFLOW_PROCESSES=p</code> to log from the parent process > +<li><code>set GECKO_DISPLAY_REFLOW_PROCESSES=c</code> to log from e10s content processes > +</ul> > +<li>OR create a rules file: maybe mention having to mess with the sandbox pref?

Pushed by gsquelart@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4f695620856d GECKO_DISPLAY_REFLOW_PROCESSES=a/p/c enables reflow logging in chosen processes - r=dbaron