Bug 1229220 - Update the scrollbar visibility prefs when initializing a TabChild; r=smaug
authorEhsan Akhgari <ehsan@mozilla.com>
Fri, 11 Mar 2016 18:10:13 -0500
changeset 289361 dc66a53a75fdd729307c7c675d0141d7392da994
parent 289360 5096e12520cd2ac31f554d7b2d7df40671f992ec
child 289362 7960c66bd18957d88369ba0028000d5197564a81
push id30102
push userryanvm@gmail.com
push dateSat, 19 Mar 2016 15:23:17 +0000
treeherdermozilla-central@720fb3d55e28 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1229220
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1229220 - Update the scrollbar visibility prefs when initializing a TabChild; r=smaug 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.
dom/base/BarProps.cpp
dom/base/nsContentUtils.cpp
dom/base/nsContentUtils.h
dom/html/test/test_fullscreen-api.html
dom/ipc/TabChild.cpp
dom/tests/browser/browser.ini
dom/tests/mochitest/bugs/mochitest.ini
dom/tests/mochitest/bugs/test_window_bar.html
xpfe/appshell/nsXULWindow.cpp
--- a/dom/base/BarProps.cpp
+++ b/dom/base/BarProps.cpp
@@ -282,33 +282,17 @@ ScrollbarsProp::SetVisible(bool aVisible
 
   /* Scrollbars, unlike the other barprops, implement visibility directly
      rather than handing off to the superclass (and from there to the
      chrome window) because scrollbar visibility uniquely applies only
      to the window making the change (arguably. it does now, anyway.)
      and because embedding apps have no interface for implementing this
      themselves, and therefore the implementation must be internal. */
 
-  nsCOMPtr<nsIScrollable> scroller =
-    do_QueryInterface(mDOMWindow->GetDocShell());
-
-  if (scroller) {
-    int32_t prefValue;
-
-    if (aVisible) {
-      prefValue = nsIScrollable::Scrollbar_Auto;
-    } else {
-      prefValue = nsIScrollable::Scrollbar_Never;
-    }
-
-    scroller->SetDefaultScrollbarPreferences(
-                nsIScrollable::ScrollOrientation_Y, prefValue);
-    scroller->SetDefaultScrollbarPreferences(
-                nsIScrollable::ScrollOrientation_X, prefValue);
-  }
+  nsContentUtils::SetScrollbarsVisibility(mDOMWindow->GetDocShell(), aVisible);
 
   /* Notably absent is the part where we notify the chrome window using
      GetBrowserChrome()->SetChromeFlags(). Given the possibility of multiple
      DOM windows (multiple top-level windows, even) within a single
      chrome window, the historical concept of a single "has scrollbars"
      flag in the chrome is inapplicable, and we can't tell at this level
      whether we represent the particular DOM window that makes this decision
      for the chrome.
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -151,16 +151,17 @@
 #include "nsIPluginHost.h"
 #include "nsIRequest.h"
 #include "nsIRunnable.h"
 #include "nsIScriptContext.h"
 #include "nsIScriptError.h"
 #include "nsIScriptGlobalObject.h"
 #include "nsIScriptObjectPrincipal.h"
 #include "nsIScriptSecurityManager.h"
+#include "nsIScrollable.h"
 #include "nsIStreamConverterService.h"
 #include "nsIStringBundle.h"
 #include "nsIURI.h"
 #include "nsIURIWithPrincipal.h"
 #include "nsIURL.h"
 #include "nsIWebNavigation.h"
 #include "nsIWindowMediator.h"
 #include "nsIWordBreaker.h"
@@ -8991,8 +8992,29 @@ nsContentUtils::IsSpecificAboutPage(JSOb
     return false;
   }
 
   // Now check the spec itself
   nsAutoCString spec;
   uri->GetSpec(spec);
   return spec.EqualsASCII(aUri);
 }
+
+/* static */ void
+nsContentUtils::SetScrollbarsVisibility(nsIDocShell* aDocShell, bool aVisible)
+{
+  nsCOMPtr<nsIScrollable> scroller = do_QueryInterface(aDocShell);
+
+  if (scroller) {
+    int32_t prefValue;
+
+    if (aVisible) {
+      prefValue = nsIScrollable::Scrollbar_Auto;
+    } else {
+      prefValue = nsIScrollable::Scrollbar_Never;
+    }
+
+    scroller->SetDefaultScrollbarPreferences(
+                nsIScrollable::ScrollOrientation_Y, prefValue);
+    scroller->SetDefaultScrollbarPreferences(
+                nsIScrollable::ScrollOrientation_X, prefValue);
+  }
+}
--- a/dom/base/nsContentUtils.h
+++ b/dom/base/nsContentUtils.h
@@ -2577,16 +2577,18 @@ public:
   /*
    * Returns true iff the provided JSObject is a global, and its URI matches
    * the provided about: URI.
    * @param aGlobal the JSObject whose URI to check, if it is a global.
    * @param aUri the URI to match, e.g. "about:feeds"
    */
   static bool IsSpecificAboutPage(JSObject* aGlobal, const char* aUri);
 
+  static void SetScrollbarsVisibility(nsIDocShell* aDocShell, bool aVisible);
+
 private:
   static bool InitializeEventTable();
 
   static nsresult EnsureStringBundle(PropertiesFile aFile);
 
   static bool CanCallerAccess(nsIPrincipal* aSubjectPrincipal,
                                 nsIPrincipal* aPrincipal);
 
--- a/dom/html/test/test_fullscreen-api.html
+++ b/dom/html/test/test_fullscreen-api.html
@@ -58,17 +58,17 @@ function nextTest() {
     testWindow.close();
   }
   SimpleTest.executeSoon(runNextTest);
 }
 
 function runNextTest() {
   if (gTestIndex < gTestWindows.length) {
     info("Run test " + gTestWindows[gTestIndex]);
-    testWindow = window.open(gTestWindows[gTestIndex], "", "width=500,height=500");
+    testWindow = window.open(gTestWindows[gTestIndex], "", "width=500,height=500,scrollbars=yes");
     // We'll wait for the window to load, then make sure our window is refocused
     // before starting the test, which will get kicked off on "focus".
     // This ensures that we're essentially back on the primary "desktop" on
     // OS X Lion before we run the test.
     testWindow.addEventListener("load", function onload() {
       testWindow.removeEventListener("load", onload, false);
       SimpleTest.waitForFocus(function() {
         SimpleTest.waitForFocus(testWindow.begin, testWindow);
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -837,16 +837,19 @@ TabChild::Init()
   // XXX: ideally, we would set a chrome event handler earlier,
   // and all windows, even the root one, will use the docshell one.
   nsCOMPtr<nsPIDOMWindowOuter> window = do_GetInterface(WebNavigation());
   NS_ENSURE_TRUE(window, NS_ERROR_FAILURE);
   nsCOMPtr<EventTarget> chromeHandler =
     do_QueryInterface(window->GetChromeEventHandler());
   docShell->SetChromeEventHandler(chromeHandler);
 
+  nsContentUtils::SetScrollbarsVisibility(window->GetDocShell(),
+    !!(mChromeFlags & nsIWebBrowserChrome::CHROME_SCROLLBARS));
+
   nsWeakPtr weakPtrThis = do_GetWeakReference(static_cast<nsITabChild*>(this));  // for capture by the lambda
   ContentReceivedInputBlockCallback callback(
       [weakPtrThis](const ScrollableLayerGuid& aGuid,
                     uint64_t aInputBlockId,
                     bool aPreventDefault)
       {
         if (nsCOMPtr<nsITabChild> tabChild = do_QueryReferent(weakPtrThis)) {
           static_cast<TabChild*>(tabChild.get())->ContentReceivedInputBlock(aGuid, aInputBlockId, aPreventDefault);
--- a/dom/tests/browser/browser.ini
+++ b/dom/tests/browser/browser.ini
@@ -4,17 +4,16 @@ support-files =
   page_privatestorageevent.html
   position.html
   test-console-api.html
   test_bug1004814.html
   worker_bug1004814.js
   geo_leak_test.html
 
 [browser_test_toolbars_visibility.js]
-skip-if = e10s
 support-files =
   test_new_window_from_content_child.html
 [browser_bug1008941_dismissGeolocationHanger.js]
 skip-if = buildapp == 'mulet'
 [browser_test__content.js]
 skip-if = e10s
 [browser_ConsoleAPITests.js]
 skip-if = e10s
--- a/dom/tests/mochitest/bugs/mochitest.ini
+++ b/dom/tests/mochitest/bugs/mochitest.ini
@@ -160,13 +160,13 @@ skip-if = toolkit == 'android' #bug 7752
 [test_onerror_message.html]
 [test_protochains.html]
 [test_resize_move_windows.html]
 skip-if = buildapp == 'mulet' || buildapp == 'b2g' || toolkit == 'android' || e10s #Windows can't change size and position on Android # b2g(Windows can't change size and position on B2G) b2g-debug(Windows can't change size and position on B2G) b2g-desktop(Windows can't change size and position on B2G)
 [test_sizetocontent_clamp.html]
 skip-if = buildapp == 'mulet' || buildapp == 'b2g' || toolkit == 'android' || e10s #Windows can't change size on Android # b2g(Windows can't change size on B2G) b2g-debug(Windows can't change size on B2G) b2g-desktop(Windows can't change size on B2G)
 [test_toJSON.html]
 [test_window_bar.html]
-skip-if = buildapp == 'mulet' || buildapp == 'b2g' || toolkit == 'android' || e10s
+skip-if = buildapp == 'mulet' || buildapp == 'b2g' || toolkit == 'android'
 [test_bug1022869.html]
 [test_bug1112040.html]
 [test_bug1160342_marquee.html]
 [test_bug1171215.html]
--- a/dom/tests/mochitest/bugs/test_window_bar.html
+++ b/dom/tests/mochitest/bugs/test_window_bar.html
@@ -43,33 +43,34 @@ function testWindow(w)
     if (prefname === undefined)
       prefname = feature;
 
     if (SpecialPowers.getBoolPref('dom.disable_window_open_feature.' + prefname)) {
       is(w[feature].visible, true, feature + ' should always be true.');
     }
     else {
       // w.location.search == '?true' if we expect the bars to be on, and
-      // '?false' otherwise.
+      // '?false' otherwise.  By default, no bars are enabled, so '?default'
+      // can be handled the same way as '?false'.
       var enabled = w.location.search == '?true';
       is(w[feature].visible, enabled, feature + ' should follow window.open settings.');
     }
   }
 
   checkFeature('menubar');
   checkFeature('toolbar');
   checkFeature('personalbar');
   checkFeature('scrollbars');
   checkFeature('statusbar', 'status');
   checkFeature('locationbar', 'location');
 
   w.close();
 
   numWindows++;
-  if (numWindows == 2) {
+  if (numWindows == 3) {
     // We're done!
     SimpleTest.finish();
   }
 
 }
 
 SimpleTest.waitForExplicitFinish();
 
@@ -82,12 +83,16 @@ var allBarsWindow =
               true);
 
 var noBarsWindow =
   window.open('file_window_bar.html?false', 'no-bars',
               'menubar=no,toolbar=no,location=no,' +
               'personalbar=no,status=no,scrollbars=no',
               false);
 
+var defaultWindow =
+  window.open('file_window_bar.html?default', 'default-bars',
+              'width=500,height=500', false);
+
 </script>
 </pre>
 </body>
 </html>
--- a/xpfe/appshell/nsXULWindow.cpp
+++ b/xpfe/appshell/nsXULWindow.cpp
@@ -2095,26 +2095,17 @@ void nsXULWindow::PlaceWindowLayersBehin
 
 void nsXULWindow::SetContentScrollbarVisibility(bool aVisible)
 {
   nsCOMPtr<nsPIDOMWindowOuter> contentWin(do_GetInterface(mPrimaryContentShell));
   if (!contentWin) {
     return;
   }
 
-  MOZ_ASSERT(contentWin->IsOuterWindow());
-  if (nsPIDOMWindowInner* innerWindow = contentWin->GetCurrentInnerWindow()) {
-    mozilla::ErrorResult rv;
-
-    RefPtr<nsGlobalWindow> window = static_cast<nsGlobalWindow*>(reinterpret_cast<nsPIDOMWindow<nsISupports>*>(innerWindow));
-    RefPtr<mozilla::dom::BarProp> scrollbars = window->GetScrollbars(rv);
-    if (scrollbars) {
-      scrollbars->SetVisible(aVisible, rv);
-    }
-  }
+  nsContentUtils::SetScrollbarsVisibility(contentWin->GetDocShell(), aVisible);
 }
 
 bool nsXULWindow::GetContentScrollbarVisibility()
 {
   // This code already exists in dom/src/base/nsBarProp.cpp, but we
   // can't safely get to that from here as this function is called
   // while the DOM window is being set up, and we need the DOM window
   // to get to that code.