Bug 1328066 - Don't broadcast the live-resize events to all browser windows unnecessarily. r=mstange
☠☠ backed out by 4a43a7579d1c ☠ ☠
authorKartikaya Gupta <kgupta@mozilla.com>
Mon, 30 Jan 2017 17:30:23 -0500
changeset 331701 6ff5cc31048644c02410da222b6626d771e93d1b
parent 331700 6c25418fa54903bba8035feb92a10fbf72077a53
child 331702 74d0d2f2b19cb99c62aba1f7c622100a53105341
push id36671
push userkgupta@mozilla.com
push dateMon, 30 Jan 2017 22:34:47 +0000
treeherderautoland@6ff5cc310486 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1328066
milestone54.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 1328066 - Don't broadcast the live-resize events to all browser windows unnecessarily. r=mstange The machinery for suppressing the displayport during live resizes was using the Observer service. However, in the case of multiple browser windows, this meant that all the open browser windows would have their displayport suppressed if *any* of the browser windows was being resized. This was mostly ok, as the displayport suppression would be turned off once the resize ended. However, the code to kick off a repaint with the unsuppressed displayport would only get triggered on one of the windows (whichever happened to process the unsuppress message last). This patch stops using the Observer service for the implementation machinery, and instead locates the active TabParent of the relevant nsWindow, and invokes the displayport suppression directly on that. This fixes the repainting bug and also avoids unnecessarily broadcasting the suppression/unsuppression notification to windows that don't neccessarily need it. MozReview-Commit-ID: LBHOgOW9KUp
browser/base/content/tabbrowser.xml
dom/ipc/TabParent.cpp
dom/ipc/TabParent.h
widget/cocoa/nsChildView.mm
widget/nsBaseWidget.cpp
widget/nsBaseWidget.h
widget/windows/nsWindow.cpp
xpfe/appshell/LiveResizeListener.h
xpfe/appshell/moz.build
xpfe/appshell/nsIXULWindow.idl
xpfe/appshell/nsXULWindow.cpp
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -87,19 +87,16 @@
         null
       </field>
       <field name="mCurrentBrowser">
         null
       </field>
       <field name="mProgressListeners">
         []
       </field>
-      <field name="mActiveResizeDisplayportSuppression">
-        null
-      </field>
       <field name="mTabsProgressListeners">
         []
       </field>
       <field name="_tabListeners">
         new Map()
       </field>
       <field name="_tabFilters">
         new Map()
@@ -4921,38 +4918,17 @@
         ]]></body>
       </method>
 
       <method name="observe">
         <parameter name="aSubject"/>
         <parameter name="aTopic"/>
         <parameter name="aData"/>
         <body><![CDATA[
-          let browser;
           switch (aTopic) {
-            case "live-resize-start": {
-              browser = this.mCurrentTab.linkedBrowser;
-              let fl = browser.QueryInterface(Ci.nsIFrameLoaderOwner).frameLoader;
-              if (fl && fl.tabParent && !this.mActiveResizeDisplayportSuppression) {
-                fl.tabParent.suppressDisplayport(true);
-                this.mActiveResizeDisplayportSuppression = browser;
-              }
-              break;
-            }
-            case "live-resize-end": {
-              browser = this.mActiveResizeDisplayportSuppression;
-              if (browser) {
-                let fl = browser.QueryInterface(Ci.nsIFrameLoaderOwner).frameLoader;
-                if (fl && fl.tabParent) {
-                  fl.tabParent.suppressDisplayport(false);
-                  this.mActiveResizeDisplayportSuppression = null;
-                }
-              }
-              break;
-            }
             case "contextual-identity-updated": {
               for (let tab of this.tabs) {
                 if (tab.getAttribute("usercontextid") == aData) {
                   ContextualIdentityService.setTabStyle(tab);
                 }
               }
               break;
             }
@@ -4965,18 +4941,16 @@
         ]]></body>
       </method>
 
       <constructor>
         <![CDATA[
           this.mCurrentBrowser = document.getAnonymousElementByAttribute(this, "anonid", "initialBrowser");
           this.mCurrentBrowser.permanentKey = {};
 
-          Services.obs.addObserver(this, "live-resize-start", false);
-          Services.obs.addObserver(this, "live-resize-end", false);
           Services.obs.addObserver(this, "contextual-identity-updated", false);
 
           this.mCurrentTab = this.tabContainer.firstChild;
           const nsIEventListenerService =
             Components.interfaces.nsIEventListenerService;
           let els = Components.classes["@mozilla.org/eventlistenerservice;1"]
                               .getService(nsIEventListenerService);
           els.addSystemEventListener(document, "keydown", this, false);
@@ -5069,18 +5043,16 @@
           // window ID. We switched to a monotonic counter as Date.now() lead
           // to random failures because of colliding IDs.
           return "panel-" + outerID + "-" + (++this._uniquePanelIDCounter);
         ]]></body>
       </method>
 
       <destructor>
         <![CDATA[
-          Services.obs.removeObserver(this, "live-resize-start");
-          Services.obs.removeObserver(this, "live-resize-end");
           Services.obs.removeObserver(this, "contextual-identity-updated");
 
           for (let tab of this.tabs) {
             let browser = tab.linkedBrowser;
             if (browser.registeredOpenURI) {
               this._unifiedComplete.unregisterOpenPage(browser.registeredOpenURI,
                                                        browser.getAttribute("usercontextid") || 0);
               delete browser.registeredOpenURI;
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -3291,16 +3291,28 @@ TabParent::RecvRequestCrossBrowserNaviga
   nsCOMPtr<nsISupports> promise;
   if (NS_FAILED(frameLoader->RequestGroupedHistoryNavigation(aGlobalIndex,
                                                              getter_AddRefs(promise)))) {
     return IPC_FAIL_NO_REASON(this);
   }
   return IPC_OK();
 }
 
+void
+TabParent::LiveResizeStarted()
+{
+  SuppressDisplayport(true);
+}
+
+void
+TabParent::LiveResizeStopped()
+{
+  SuppressDisplayport(false);
+}
+
 NS_IMETHODIMP
 FakeChannel::OnAuthAvailable(nsISupports *aContext, nsIAuthInformation *aAuthInfo)
 {
   nsAuthInformationHolder* holder =
     static_cast<nsAuthInformationHolder*>(aAuthInfo);
 
   if (!net::gNeckoChild->SendOnAuthAvailable(mCallbackId,
                                              holder->User(),
--- a/dom/ipc/TabParent.h
+++ b/dom/ipc/TabParent.h
@@ -3,16 +3,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_tabs_TabParent_h
 #define mozilla_tabs_TabParent_h
 
 #include "js/TypeDecls.h"
+#include "LiveResizeListener.h"
 #include "mozilla/ContentCache.h"
 #include "mozilla/dom/AudioChannelBinding.h"
 #include "mozilla/dom/ipc/IdType.h"
 #include "mozilla/dom/PBrowserParent.h"
 #include "mozilla/dom/PContent.h"
 #include "mozilla/dom/PFilePickerParent.h"
 #include "mozilla/dom/TabContext.h"
 #include "mozilla/EventForwards.h"
@@ -86,16 +87,17 @@ class TabParent final : public PBrowserP
                       , public nsITabParent
                       , public nsIAuthPromptProvider
                       , public nsISecureBrowserUI
                       , public nsIKeyEventInPluginCallback
                       , public nsSupportsWeakReference
                       , public TabContext
                       , public nsAPostRefreshObserver
                       , public nsIWebBrowserPersistable
+                      , public LiveResizeListener
 {
   typedef mozilla::dom::ClonedMessageData ClonedMessageData;
 
   virtual ~TabParent();
 
 public:
   // Helper class for ContentParent::RecvCreateWindow.
   struct AutoUseNewTab;
@@ -588,16 +590,20 @@ public:
                                       float aVolume,
                                       bool aMuted);
   bool SetRenderFrame(PRenderFrameParent* aRFParent);
   bool GetRenderFrameInfo(TextureFactoryIdentifier* aTextureFactoryIdentifier,
                           uint64_t* aLayersId);
 
   mozilla::ipc::IPCResult RecvEnsureLayersConnected() override;
 
+  // LiveResizeListener implementation
+  void LiveResizeStarted() override;
+  void LiveResizeStopped() override;
+
 protected:
   bool ReceiveMessage(const nsString& aMessage,
                       bool aSync,
                       ipc::StructuredCloneData* aData,
                       mozilla::jsipc::CpowHolder* aCpows,
                       nsIPrincipal* aPrincipal,
                       nsTArray<ipc::StructuredCloneData>* aJSONRetVal = nullptr);
 
--- a/widget/cocoa/nsChildView.mm
+++ b/widget/cocoa/nsChildView.mm
@@ -3648,34 +3648,30 @@ NSEvent* gLastDragMouseDownEvent = nil;
 {
   return [[self window] isKindOfClass:[BaseWindow class]] &&
          [(BaseWindow*)[self window] mainChildView] == self &&
          [(BaseWindow*)[self window] drawsContentsIntoWindowFrame];
 }
 
 - (void)viewWillStartLiveResize
 {
-  nsCOMPtr<nsIObserverService> observerService = mozilla::services::GetObserverService();
-
-  if (!observerService) {
-    return;
-  }
-
-  observerService->NotifyObservers(nullptr, "live-resize-start", nullptr);
+  nsCocoaWindow* windowWidget = mGeckoChild ? mGeckoChild->GetXULWindowWidget() : nullptr;
+  if (windowWidget) {
+    windowWidget->NotifyLiveResizeStarted();
+  }
 }
 
 - (void)viewDidEndLiveResize
 {
-  nsCOMPtr<nsIObserverService> observerService = mozilla::services::GetObserverService();
-
-  if (!observerService) {
-    return;
-  }
-
-  observerService->NotifyObservers(nullptr, "live-resize-end", nullptr);
+  // If windowWidget is null here then we need to find some other way to
+  // unsuppress the displayport, or we might get stuck with a content process
+  // that has the displayport permanently suppressed, which would be bad.
+  nsCocoaWindow* windowWidget = mGeckoChild ? mGeckoChild->GetXULWindowWidget() : nullptr;
+  MOZ_ASSERT(windowWidget);
+  windowWidget->NotifyLiveResizeStopped();
 }
 
 - (NSColor*)vibrancyFillColorForThemeGeometryType:(nsITheme::ThemeGeometryType)aThemeGeometryType
 {
   if (!mGeckoChild) {
     return [NSColor whiteColor];
   }
   return mGeckoChild->VibrancyFillColorForThemeGeometryType(aThemeGeometryType);
--- a/widget/nsBaseWidget.cpp
+++ b/widget/nsBaseWidget.cpp
@@ -7,16 +7,17 @@
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/TextEventDispatcher.h"
 #include "mozilla/TextEventDispatcherListener.h"
 
 #include "mozilla/layers/CompositorBridgeChild.h"
 #include "mozilla/layers/CompositorBridgeParent.h"
 #include "mozilla/layers/ImageBridgeChild.h"
+#include "LiveResizeListener.h"
 #include "nsBaseWidget.h"
 #include "nsDeviceContext.h"
 #include "nsCOMPtr.h"
 #include "nsGfxCIID.h"
 #include "nsWidgetsCID.h"
 #include "nsServiceManagerUtils.h"
 #include "nsIKeyEventInPluginCallback.h"
 #include "nsIScreenManager.h"
@@ -244,16 +245,17 @@ WidgetShutdownObserver::Unregister()
     nsContentUtils::UnregisterShutdownObserver(this);
     mRegistered = false;
   }
 }
 
 void
 nsBaseWidget::Shutdown()
 {
+  NotifyLiveResizeStopped();
   RevokeTransactionIdAllocator();
   DestroyCompositor();
   FreeShutdownObserver();
 #if defined(XP_WIN) || defined(MOZ_WIDGET_GTK)
   if (sPluginWidgetList) {
     delete sPluginWidgetList;
     sPluginWidgetList = nullptr;
   }
@@ -2089,16 +2091,51 @@ nsBaseWidget::UpdateSynthesizedTouchStat
         (float)aPointerOrientation,
         (float)aPointerPressure));
   }
 
   return inputToDispatch;
 }
 
 void
+nsBaseWidget::NotifyLiveResizeStarted()
+{
+  // If we have mLiveResizeListeners already non-empty, we should notify those
+  // listeners that the resize stopped before starting anew. In theory this
+  // should never happen because we shouldn't get nested live resize actions.
+  NotifyLiveResizeStopped();
+  MOZ_ASSERT(mLiveResizeListeners.IsEmpty());
+
+  // If we can get the active tab parent for the current widget, suppress
+  // the displayport on it during the live resize.
+  if (!mWidgetListener) {
+    return;
+  }
+  nsCOMPtr<nsIXULWindow> xulWindow = mWidgetListener->GetXULWindow();
+  if (!xulWindow) {
+    return;
+  }
+  mLiveResizeListeners = xulWindow->GetLiveResizeListeners();
+  for (uint32_t i = 0; i < mLiveResizeListeners.Length(); i++) {
+    mLiveResizeListeners[i]->LiveResizeStarted();
+  }
+}
+
+void
+nsBaseWidget::NotifyLiveResizeStopped()
+{
+  if (!mLiveResizeListeners.IsEmpty()) {
+    for (uint32_t i = 0; i < mLiveResizeListeners.Length(); i++) {
+      mLiveResizeListeners[i]->LiveResizeStopped();
+    }
+    mLiveResizeListeners.Clear();
+  }
+}
+
+void
 nsBaseWidget::RegisterPluginWindowForRemoteUpdates()
 {
 #if !defined(XP_WIN) && !defined(MOZ_WIDGET_GTK)
   NS_NOTREACHED("nsBaseWidget::RegisterPluginWindowForRemoteUpdates not implemented!");
   return;
 #else
   MOZ_ASSERT(NS_IsMainThread());
   void* id = GetNativeData(NS_NATIVE_PLUGIN_ID);
--- a/widget/nsBaseWidget.h
+++ b/widget/nsBaseWidget.h
@@ -32,16 +32,18 @@ const mozilla::gfx::SurfaceFormat kScrol
 #endif
 
 class nsIContent;
 class nsAutoRollup;
 class gfxContext;
 
 namespace mozilla {
 class CompositorVsyncDispatcher;
+class LiveResizeListener;
+
 #ifdef ACCESSIBILITY
 namespace a11y {
 class Accessible;
 }
 #endif
 
 namespace gfx {
 class DrawTarget;
@@ -393,16 +395,23 @@ public:
   static nsIRollupListener* GetActiveRollupListener();
 
   void Shutdown();
 
 #if defined(XP_WIN)
   uint64_t CreateScrollCaptureContainer() override;
 #endif
 
+  // These functions should be called at the start and end of a "live" widget
+  // resize (i.e. when the window contents are repainting during the resize,
+  // such as when the user drags a window border). It will suppress the
+  // displayport during the live resize to avoid unneccessary overpainting.
+  void NotifyLiveResizeStarted();
+  void NotifyLiveResizeStopped();
+
 protected:
   // These are methods for CompositorWidgetWrapper, and should only be
   // accessed from that class. Derived widgets can choose which methods to
   // implement, or none if supporting out-of-process compositing.
   virtual bool PreRender(mozilla::widget::WidgetRenderingContext* aContext) {
     return true;
   }
   virtual void PostRender(mozilla::widget::WidgetRenderingContext* aContext)
@@ -704,16 +713,21 @@ protected:
 
     uint32_t mPresShellID;
     FrameMetrics::ViewID mViewID;
     ZoomConstraints mConstraints;
   };
 
   mozilla::Maybe<InitialZoomConstraints> mInitialZoomConstraints;
 
+  // This points to the resize listeners who have been notified that a live
+  // resize is in progress. This should always be empty when a live-resize is
+  // not in progress.
+  nsTArray<RefPtr<mozilla::LiveResizeListener>> mLiveResizeListeners;
+
 #ifdef DEBUG
 protected:
   static nsAutoString debug_GuiEventToString(mozilla::WidgetGUIEvent* aGuiEvent);
   static bool debug_WantPaintFlashing();
 
   static void debug_DumpInvalidate(FILE* aFileOut,
                                    nsIWidget* aWidget,
                                    const LayoutDeviceIntRect* aRect,
--- a/widget/windows/nsWindow.cpp
+++ b/widget/windows/nsWindow.cpp
@@ -5639,23 +5639,17 @@ nsWindow::ProcessMessage(UINT msg, WPARA
 
     case WM_SIZING:
     {
       // When we get WM_ENTERSIZEMOVE we don't know yet if we're in a live
       // resize or move event. Instead we wait for first VM_SIZING message
       // within a ENTERSIZEMOVE to consider this a live resize event.
       if (mResizeState == IN_SIZEMOVE) {
         mResizeState = RESIZING;
-        nsCOMPtr<nsIObserverService> observerService =
-          mozilla::services::GetObserverService();
-
-        if (observerService) {
-          observerService->NotifyObservers(nullptr, "live-resize-start",
-                                           nullptr);
-        }
+        NotifyLiveResizeStarted();
       }
       break;
     }
 
     case WM_MOVING:
       FinishLiveResizing(MOVING);
       if (WinUtils::IsPerMonitorDPIAware()) {
         // Sometimes, we appear to miss a WM_DPICHANGED message while moving
@@ -6072,20 +6066,17 @@ nsWindow::ProcessMessage(UINT msg, WPARA
     return true;
   }
 }
 
 void
 nsWindow::FinishLiveResizing(ResizeState aNewState)
 {
   if (mResizeState == RESIZING) {
-    nsCOMPtr<nsIObserverService> observerService = mozilla::services::GetObserverService();
-    if (observerService) {
-      observerService->NotifyObservers(nullptr, "live-resize-end", nullptr);
-    }
+    NotifyLiveResizeStopped();
   }
   mResizeState = aNewState;
   ForcePresent();
 }
 
 /**************************************************************
  *
  * SECTION: Broadcast messaging
new file mode 100644
--- /dev/null
+++ b/xpfe/appshell/LiveResizeListener.h
@@ -0,0 +1,28 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_LiveResizeListener_h
+#define mozilla_LiveResizeListener_h
+
+#include "nscore.h"
+
+namespace mozilla {
+
+class LiveResizeListener {
+public:
+  virtual void LiveResizeStarted() = 0;
+  virtual void LiveResizeStopped() = 0;
+
+  NS_IMETHOD_(MozExternalRefCountType) AddRef(void) = 0;
+  NS_IMETHOD_(MozExternalRefCountType) Release(void) = 0;
+
+protected:
+  virtual ~LiveResizeListener() {}
+};
+
+} // namespace mozilla
+
+#endif // mozilla_LiveResizeListener_h
--- a/xpfe/appshell/moz.build
+++ b/xpfe/appshell/moz.build
@@ -14,16 +14,17 @@ XPIDL_SOURCES += [
     'nsIWindowMediatorListener.idl',
     'nsIXULBrowserWindow.idl',
     'nsIXULWindow.idl',
 ]
 
 XPIDL_MODULE = 'appshell'
 
 EXPORTS += [
+    'LiveResizeListener.h',
     'nsAppShellCID.h',
 ]
 
 UNIFIED_SOURCES += [
     'nsAppShellFactory.cpp',
     'nsAppShellService.cpp',
     'nsAppShellWindowEnumerator.cpp',
     'nsChromeTreeOwner.cpp',
@@ -34,9 +35,9 @@ UNIFIED_SOURCES += [
 ]
 
 LOCAL_INCLUDES += [
     '/dom/base',
 ]
 
 FINAL_LIBRARY = 'xul'
 
-include('/ipc/chromium/chromium-config.mozbuild')
\ No newline at end of file
+include('/ipc/chromium/chromium-config.mozbuild')
--- a/xpfe/appshell/nsIXULWindow.idl
+++ b/xpfe/appshell/nsIXULWindow.idl
@@ -8,22 +8,29 @@
 
 /**
  * The nsIXULWindow
  *
  * When the window is destroyed, it will fire a "xul-window-destroyed"
  * notification through the global observer service.
  */
 
+%{C++
+#include "LiveResizeListener.h"
+#include "nsTArray.h"
+%}
+
 interface nsIDocShell;
 interface nsIDocShellTreeItem;
 interface nsIXULBrowserWindow;
 interface nsITabParent;
 interface mozIDOMWindowProxy;
 
+native LiveResizeListenerArray(nsTArray<RefPtr<mozilla::LiveResizeListener>>);
+
 [scriptable, uuid(d6d7a014-e28d-4c9d-8727-1cf6d870619b)]
 interface nsIXULWindow : nsISupports
 {
   /**
    * The docshell owning the XUL for this window.
    */
   readonly attribute nsIDocShell docShell;
 
@@ -46,16 +53,18 @@ interface nsIXULWindow : nsISupports
    * In multiprocess case we may not have primaryContentShell but
    * primaryTabParent.
    */
   readonly attribute nsITabParent primaryTabParent;
 
   void tabParentAdded(in nsITabParent aTab, in boolean aPrimary);
   void tabParentRemoved(in nsITabParent aTab);
 
+  [noscript,notxpcom] LiveResizeListenerArray getLiveResizeListeners();
+
   /**
    * Tell this window that it has picked up a child XUL window
    * @param aChild the child window being added
    */
   void addChildWindow(in nsIXULWindow aChild);
 
   /**
    * Tell this window that it has lost a child XUL window
--- a/xpfe/appshell/nsXULWindow.cpp
+++ b/xpfe/appshell/nsXULWindow.cpp
@@ -339,16 +339,27 @@ nsXULWindow::TabParentRemoved(nsITabPare
 NS_IMETHODIMP
 nsXULWindow::GetPrimaryTabParent(nsITabParent** aTab)
 {
   nsCOMPtr<nsITabParent> tab = mPrimaryTabParent;
   tab.forget(aTab);
   return NS_OK;
 }
 
+nsTArray<RefPtr<mozilla::LiveResizeListener>>
+nsXULWindow::GetLiveResizeListeners()
+{
+  nsTArray<RefPtr<mozilla::LiveResizeListener>> listeners;
+  if (mPrimaryTabParent) {
+    TabParent* parent = static_cast<TabParent*>(mPrimaryTabParent.get());
+    listeners.AppendElement(parent);
+  }
+  return listeners;
+}
+
 NS_IMETHODIMP nsXULWindow::AddChildWindow(nsIXULWindow *aChild)
 {
   // we're not really keeping track of this right now
   return NS_OK;
 }
 
 NS_IMETHODIMP nsXULWindow::RemoveChildWindow(nsIXULWindow *aChild)
 {