From cf091ea0ebd7713da368e831e80e72b26c7538b3 Mon Sep 17 00:00:00 2001 From: Kartikaya Gupta Date: Wed, 1 Feb 2017 10:38:16 -0500 Subject: [PATCH] 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 | 28 ---------------------- dom/ipc/TabParent.cpp | 12 ++++++++++ dom/ipc/TabParent.h | 6 +++++ widget/cocoa/nsChildView.mm | 24 +++++++++---------- widget/nsBaseWidget.cpp | 37 +++++++++++++++++++++++++++++ widget/nsBaseWidget.h | 14 +++++++++++ widget/windows/nsWindow.cpp | 13 ++-------- xpfe/appshell/LiveResizeListener.h | 27 +++++++++++++++++++++ xpfe/appshell/moz.build | 3 ++- xpfe/appshell/nsIXULWindow.idl | 9 +++++++ xpfe/appshell/nsXULWindow.cpp | 11 +++++++++ 11 files changed, 132 insertions(+), 52 deletions(-) create mode 100644 xpfe/appshell/LiveResizeListener.h diff --git a/browser/base/content/tabbrowser.xml b/browser/base/content/tabbrowser.xml index 842ef1173b5c..ad88ce6b8a24 100644 --- a/browser/base/content/tabbrowser.xml +++ b/browser/base/content/tabbrowser.xml @@ -92,9 +92,6 @@ [] - - null - [] @@ -4926,28 +4923,7 @@ observerService = mozilla::services::GetObserverService(); - - if (!observerService) { - return; + nsCocoaWindow* windowWidget = mGeckoChild ? mGeckoChild->GetXULWindowWidget() : nullptr; + if (windowWidget) { + windowWidget->NotifyLiveResizeStarted(); } - - observerService->NotifyObservers(nullptr, "live-resize-start", nullptr); } - (void)viewDidEndLiveResize { - nsCOMPtr observerService = mozilla::services::GetObserverService(); - - if (!observerService) { - return; + // mGeckoChild may legitimately be null here. It should also have been null + // in viewWillStartLiveResize, so there's no problem. However if we run into + // cases where the windowWidget was non-null in viewWillStartLiveResize but + // is null here, that might be problematic because we might get stuck with + // a content process that has the displayport suppressed. If that scenario + // arises (I'm not sure that it does) we will need to handle it gracefully. + nsCocoaWindow* windowWidget = mGeckoChild ? mGeckoChild->GetXULWindowWidget() : nullptr; + if (windowWidget) { + windowWidget->NotifyLiveResizeStopped(); } - - observerService->NotifyObservers(nullptr, "live-resize-end", nullptr); } - (NSColor*)vibrancyFillColorForThemeGeometryType:(nsITheme::ThemeGeometryType)aThemeGeometryType diff --git a/widget/nsBaseWidget.cpp b/widget/nsBaseWidget.cpp index cbdc0008397d..2194cc5e7a1c 100644 --- a/widget/nsBaseWidget.cpp +++ b/widget/nsBaseWidget.cpp @@ -12,6 +12,7 @@ #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" @@ -249,6 +250,7 @@ WidgetShutdownObserver::Unregister() void nsBaseWidget::Shutdown() { + NotifyLiveResizeStopped(); RevokeTransactionIdAllocator(); DestroyCompositor(); FreeShutdownObserver(); @@ -2093,6 +2095,41 @@ nsBaseWidget::UpdateSynthesizedTouchState(MultiTouchInput* aState, 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 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() { diff --git a/widget/nsBaseWidget.h b/widget/nsBaseWidget.h index c953cde5dc09..8892b7ed9a41 100644 --- a/widget/nsBaseWidget.h +++ b/widget/nsBaseWidget.h @@ -37,6 +37,8 @@ class gfxContext; namespace mozilla { class CompositorVsyncDispatcher; +class LiveResizeListener; + #ifdef ACCESSIBILITY namespace a11y { class Accessible; @@ -398,6 +400,13 @@ public: 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 @@ -709,6 +718,11 @@ protected: mozilla::Maybe 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> mLiveResizeListeners; + #ifdef DEBUG protected: static nsAutoString debug_GuiEventToString(mozilla::WidgetGUIEvent* aGuiEvent); diff --git a/widget/windows/nsWindow.cpp b/widget/windows/nsWindow.cpp index 06765f22da84..29bd1b3e8666 100644 --- a/widget/windows/nsWindow.cpp +++ b/widget/windows/nsWindow.cpp @@ -5644,13 +5644,7 @@ nsWindow::ProcessMessage(UINT msg, WPARAM& wParam, LPARAM& lParam, // within a ENTERSIZEMOVE to consider this a live resize event. if (mResizeState == IN_SIZEMOVE) { mResizeState = RESIZING; - nsCOMPtr observerService = - mozilla::services::GetObserverService(); - - if (observerService) { - observerService->NotifyObservers(nullptr, "live-resize-start", - nullptr); - } + NotifyLiveResizeStarted(); } break; } @@ -6077,10 +6071,7 @@ void nsWindow::FinishLiveResizing(ResizeState aNewState) { if (mResizeState == RESIZING) { - nsCOMPtr observerService = mozilla::services::GetObserverService(); - if (observerService) { - observerService->NotifyObservers(nullptr, "live-resize-end", nullptr); - } + NotifyLiveResizeStopped(); } mResizeState = aNewState; ForcePresent(); diff --git a/xpfe/appshell/LiveResizeListener.h b/xpfe/appshell/LiveResizeListener.h new file mode 100644 index 000000000000..f41dcfaea97d --- /dev/null +++ b/xpfe/appshell/LiveResizeListener.h @@ -0,0 +1,27 @@ +/* -*- 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 "nsISupportsImpl.h" + +namespace mozilla { + +class LiveResizeListener { +public: + virtual void LiveResizeStarted() = 0; + virtual void LiveResizeStopped() = 0; + + NS_INLINE_DECL_PURE_VIRTUAL_REFCOUNTING + +protected: + virtual ~LiveResizeListener() {} +}; + +} // namespace mozilla + +#endif // mozilla_LiveResizeListener_h diff --git a/xpfe/appshell/moz.build b/xpfe/appshell/moz.build index 5cfe4192481f..089580b6aea0 100644 --- a/xpfe/appshell/moz.build +++ b/xpfe/appshell/moz.build @@ -19,6 +19,7 @@ XPIDL_SOURCES += [ XPIDL_MODULE = 'appshell' EXPORTS += [ + 'LiveResizeListener.h', 'nsAppShellCID.h', ] @@ -39,4 +40,4 @@ LOCAL_INCLUDES += [ FINAL_LIBRARY = 'xul' -include('/ipc/chromium/chromium-config.mozbuild') \ No newline at end of file +include('/ipc/chromium/chromium-config.mozbuild') diff --git a/xpfe/appshell/nsIXULWindow.idl b/xpfe/appshell/nsIXULWindow.idl index ceeb7f1b4d90..27468df825c1 100644 --- a/xpfe/appshell/nsIXULWindow.idl +++ b/xpfe/appshell/nsIXULWindow.idl @@ -13,12 +13,19 @@ * 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>); + [scriptable, uuid(d6d7a014-e28d-4c9d-8727-1cf6d870619b)] interface nsIXULWindow : nsISupports { @@ -51,6 +58,8 @@ interface nsIXULWindow : nsISupports 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 diff --git a/xpfe/appshell/nsXULWindow.cpp b/xpfe/appshell/nsXULWindow.cpp index c9c0dc9ad134..1b18966df36a 100644 --- a/xpfe/appshell/nsXULWindow.cpp +++ b/xpfe/appshell/nsXULWindow.cpp @@ -344,6 +344,17 @@ nsXULWindow::GetPrimaryTabParent(nsITabParent** aTab) return NS_OK; } +nsTArray> +nsXULWindow::GetLiveResizeListeners() +{ + nsTArray> listeners; + if (mPrimaryTabParent) { + TabParent* parent = static_cast(mPrimaryTabParent.get()); + listeners.AppendElement(parent); + } + return listeners; +} + NS_IMETHODIMP nsXULWindow::AddChildWindow(nsIXULWindow *aChild) { // we're not really keeping track of this right now