This test opens two new windows, once with scrollbars=yes and once with scrollbars=no (along with some other settings). In the latter case, |w['scrollbars'].visible| is expected to be false, but it is true when run with e10s. I'm not sure what the matter is. (The test fails even when only one window is opened.) Maybe setting scrollbars=no does some kind of async round trip to the parent process, and the child process hasn't gotten the reply back yet by the time it queries the value.

Bug 1194897 is related, but it involves the other barprops things, which are derived from the chrome flags. scrollbars seems to poke directly at the docshell to get the information.

dom/tests/browser/browser_test_toolbars_visibility.js could be updated to include this. Also I'm not sure test_window_bar tests anything different, so it might be worth removing it.

Do you have time to look into this, Mike? It involves docshell and barprops.

I'm kinda swamped with e10s perf work right now, which is my highest priority. This has been tracking-e10s+'d, which (presumably) means that we'll revisit this when things calm down, e10s-wise.

If, of course, you feel it needs reconsideration, please set tracking-e10s to ? and we'll discuss in triage next week.

I spent some time debugging test_window_bar.html. It seems like in e10s mode, nsXULWindow::SetContentScrollbarVisibility() is called just like the non-e10s mode. That function finds _some_ inner window in the parent process, and gets the ScrollbarsProp object off of it, and calls SetVisible() on it, but the child process has no idea any of this has ever happened, so as far as it's concerned, the scrollbars are still visible. Presumably the fix would be to do the work similar to SetContentScrollbarVisibility() in the child somewhere appropriate. I don't know where such a place would be.

(In reply to :Ehsan Akhgari from comment #6) > Presumably the fix would be to do the work similar to > SetContentScrollbarVisibility() in the child somewhere appropriate. I don't > know where such a place would be. Perhaps in https://hg.mozilla.org/mozilla-central/file/af7c0cb0798f/dom/ipc/nsIContentParent.cpp#l118 ? We already do the work there to propagate private browsing state from the parent - maybe we can do the same thing with scrollbar visibility.

I have a fix.

This will make sure that window.scrollbars correctly reflects the respective chrome flags in e10s mode. We also update nsXULWindow::SetContentScrollbarVisibility() to the new nsContentUtils helper. That code is responsible for doing this work in the single process case.

Comment on attachment 8729718 [details] [diff] [review] Update the scrollbar visibility prefs when initializing a TabChild Review of attachment 8729718 [details] [diff] [review]: ----------------------------------------------------------------- FWIW, this looks good to me.

Backed out fro M-e10s(2) failure in test_fullscreen-api.html Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/9ccaec771922 Push with failures (has leaks from a different bug before it in its log): https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=66fdeb1acce5 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=23711900&repo=mozilla-inbound 14:37:19 INFO - 1047 INFO TEST-PASS | dom/html/test/test_fullscreen-api.html | [scrollbar] Should only check the current fullscreen element 14:37:19 INFO - 1048 INFO TEST-UNEXPECTED-FAIL | dom/html/test/test_fullscreen-api.html | [scrollbar] Should have vertical scrollbar when [object HTMLHtmlElement] is in fullscreen - got 1600, expected 1585 14:37:19 INFO - SimpleTest.is@SimpleTest/SimpleTest.js:267:5 14:37:19 INFO - is@dom/html/test/file_fullscreen-scrollbar.html:30:25 14:37:19 INFO - assertHasScrollbars@dom/html/test/file_fullscreen-scrollbar.html:54:3 14:37:19 INFO - checkScrollbars@dom/html/test/file_fullscreen-scrollbar.html:73:3 14:37:19 INFO - enteredFullscreenOnRoot@dom/html/test/file_fullscreen-scrollbar.html:100:3 14:37:19 INFO - addFullscreenChangeContinuation/invokeCallback/</<@dom/html/test/file_fullscreen-utils.js:54:50 14:37:19 INFO - setTimeout handler*SimpleTest_setTimeoutShim@SimpleTest/SimpleTest.js:621:12 14:37:19 INFO - addFullscreenChangeContinuation/invokeCallback/<@dom/html/test/file_fullscreen-utils.js:54:33 14:37:19 INFO - FrameRequestCallback*invokeCallback@dom/html/test/file_fullscreen-utils.js:54:5 14:37:19 INFO - onFullscreenChange@dom/html/test/file_fullscreen-utils.js:59:7 14:37:19 INFO - EventListener.handleEvent*addFullscreenChangeContinuation@dom/html/test/file_fullscreen-utils.js:70:3 14:37:19 INFO - begin@dom/html/test/file_fullscreen-scrollbar.html:95:3 14:37:19 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@SimpleTest/SimpleTest.js:740:59 14:37:19 INFO - Not taking screenshot here: see the one that was previously logged 14:37:19 INFO - 1049 INFO TEST-UNEXPECTED-FAIL | dom/html/test/test_fullscreen-api.html | [scrollbar] Should have horizontal scrollbar when [object HTMLHtmlElement] is in fullscreen - got 1200, expected 1185

Hmm, so the thing that this test does that is special is that it calls window.open() with a non-empty features argument without specifying scrollbars. In this case, Firefox 45 and trunk non-e10s mode create a non-scrollable window. But trunk e10s mode, Chrome and Safari all create a scrollable window. My patch "fixes" the e10s mode to be non-scrollable as well (presumably because before nobody was setting the attribute on the docshell in the first place in e10s mode.) I personally think making the window non-scrollable in this case is pretty broken, and we should only honor scrollbars=no in nsWindowWatcher::CalculateChromeFlags(). Olli, what do you think?

We discussed this on IRC yesterday, and smaug asked me to keep the non-e10s behavior and file a follow-up for changing it.