Bug 1668875 - Distinguish theme changes that can and cannot affect style/layout. r=tnikkel

This should make the optimization landed earlier in this bug apply for
some of the NotifyThemeChanged() calls in nsWindow.cpp which are causing
all the extra invalidations.

If we know that system colors/fonts didn't change, we can avoid doing a
bunch of reflow work and the patch from earlier in the bug can avoid
re-rasterizing images too.

Differential Revision: https://phabricator.services.mozilla.com/D94425

dom/ipc/ContentChild.cpp

@@ -2269,9 +2269,9 @@ mozilla::ipc::IPCResult ContentChild::RecvNotifyVisited(
 }
 
 mozilla::ipc::IPCResult ContentChild::RecvThemeChanged(
-    LookAndFeelCache&& aLookAndFeelCache) {
+    LookAndFeelCache&& aLookAndFeelCache, widget::ThemeChangeKind aKind) {
   LookAndFeel::SetCache(aLookAndFeelCache);
-  LookAndFeel::NotifyChangedAllWindows();
+  LookAndFeel::NotifyChangedAllWindows(aKind);
   return IPC_OK();
 }
 

dom/ipc/ContentChild.h

@@ -14,6 +14,7 @@
 #include "mozilla/dom/RemoteType.h"
 #include "mozilla/ipc/InputStreamUtils.h"
 #include "mozilla/ipc/ProtocolUtils.h"
+#include "mozilla/widget/ThemeChangeKind.h"
 #include "mozilla/LookAndFeel.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/UniquePtr.h"
@@ -301,8 +302,8 @@ class ContentChild final : public PContentChild,
                                                  const bool& haveBidiKeyboards);
 
   mozilla::ipc::IPCResult RecvNotifyVisited(nsTArray<VisitedQueryResult>&&);
-  mozilla::ipc::IPCResult RecvThemeChanged(
-      LookAndFeelCache&& aLookAndFeelCache);
+  mozilla::ipc::IPCResult RecvThemeChanged(LookAndFeelCache&& aLookAndFeelCache,
+                                           widget::ThemeChangeKind);
 
   mozilla::ipc::IPCResult RecvUpdateSystemParameters(
       nsTArray<SystemParameterKVPair>&& aUpdates);

dom/ipc/PContent.ipdl

@@ -85,6 +85,7 @@ using mozilla::dom::TabId from "mozilla/dom/ipc/IdType.h";
 using mozilla::dom::ContentParentId from "mozilla/dom/ipc/IdType.h";
 using mozilla::LayoutDeviceIntPoint from "Units.h";
 using struct LookAndFeelCache from "mozilla/widget/WidgetMessageUtils.h";
+using mozilla::widget::ThemeChangeKind from "mozilla/widget/WidgetMessageUtils.h";
 using class mozilla::dom::MessagePort from "mozilla/dom/MessagePort.h";
 using class mozilla::dom::ipc::StructuredCloneData from "mozilla/dom/ipc/StructuredCloneData.h";
 using mozilla::OriginAttributes from "mozilla/ipc/BackgroundUtils.h";
@@ -560,10 +561,10 @@ child:
     async NotifyVisited(VisitedQueryResult[] uri);
 
     /**
-     * Tell the child that the system theme has changed, and that a repaint
-     * is necessary.
+     * Tell the child that the system theme has changed, and that a repaint is
+     * necessary.
      */
-    async ThemeChanged(LookAndFeelCache lookAndFeelCache);
+    async ThemeChanged(LookAndFeelCache aCache, ThemeChangeKind aKind);
 
     async UpdateSystemParameters(SystemParameterKVPair[] aUpdates);
 

layout/base/PresShell.cpp

@@ -9924,7 +9924,9 @@ PresShell::Observe(nsISupports* aSubject, const char* aTopic,
   }
 
   if (!nsCRT::strcmp(aTopic, "look-and-feel-changed")) {
-    ThemeChanged();
+    // See how LookAndFeel::NotifyChangedAllWindows encodes this.
+    auto kind = widget::ThemeChangeKind(reinterpret_cast<uintptr_t>(aData));
+    ThemeChanged(kind);
     return NS_OK;
   }
 

layout/base/PresShell.h

@@ -27,6 +27,7 @@
 #include "mozilla/WeakPtr.h"
 #include "mozilla/layers/FocusTarget.h"
 #include "mozilla/layout/LayoutTelemetryTools.h"
+#include "mozilla/widget/ThemeChangeKind.h"
 #include "nsColor.h"
 #include "nsCOMArray.h"
 #include "nsCoord.h"
@@ -1246,7 +1247,11 @@ class PresShell final : public nsStubDocumentObserver,
 
   // Widget notificiations
   void WindowSizeMoveDone();
-  void ThemeChanged() { mPresContext->ThemeChanged(); }
+
+  void ThemeChanged(widget::ThemeChangeKind aChangeKind) {
+    mPresContext->ThemeChanged(aChangeKind);
+  }
+
   void BackingScaleFactorChanged() { mPresContext->UIResolutionChangedSync(); }
 
   MOZ_CAN_RUN_SCRIPT

layout/base/nsPresContext.cpp

@@ -203,6 +203,7 @@ nsPresContext::nsPresContext(dom::Document* aDocument, nsPresContextType aType)
       mPrefBidiDirection(false),
       mPrefScrollbarSide(0),
       mPendingThemeChanged(false),
+      mPendingThemeChangeKind(0),
       mPendingUIResolutionChanged(false),
       mPostedPrefChangedRunnable(false),
       mIsGlyph(false),
@@ -1352,11 +1353,13 @@ nsITheme* nsPresContext::EnsureTheme() {
   return mTheme;
 }
 
-void nsPresContext::ThemeChanged() {
+void nsPresContext::ThemeChanged(widget::ThemeChangeKind aKind) {
   // NOTE(emilio): This ideally wouldn't need to be a _TEXT() marker, but
   // otherwise the stack is not captured, see bug 1670046.
   PROFILER_MARKER_TEXT("ThemeChanged", LAYOUT, MarkerStack::Capture(), ""_ns);
 
+  mPendingThemeChangeKind |= unsigned(aKind);
+
   if (!mPendingThemeChanged) {
     sLookAndFeelChanged = true;
     sThemeChanged = true;
@@ -1374,6 +1377,9 @@ void nsPresContext::ThemeChanged() {
 void nsPresContext::ThemeChangedInternal() {
   mPendingThemeChanged = false;
 
+  const auto kind = widget::ThemeChangeKind(mPendingThemeChangeKind);
+  mPendingThemeChangeKind = 0;
+
   // Tell the theme that it changed, so it can flush any handles to stale theme
   // data.
   if (mTheme && sThemeChanged) {
@@ -1396,7 +1402,7 @@ void nsPresContext::ThemeChangedInternal() {
       ContentParent::GetAll(cp);
       LookAndFeelCache lnfCache = LookAndFeel::GetCache();
       for (ContentParent* c : cp) {
-        Unused << c->SendThemeChanged(lnfCache);
+        Unused << c->SendThemeChanged(lnfCache, kind);
       }
     }
   }
@@ -1413,15 +1419,18 @@ void nsPresContext::ThemeChangedInternal() {
   // queries on them.
   //
   // Changes in theme can change system colors (whose changes are properly
-  // reflected in computed style data), system fonts (whose changes are not),
-  // and -moz-appearance (whose changes likewise are not), so we need to
-  // recascade for the first, and reflow for the rest.
+  // reflected in computed style data), system fonts (whose changes are
+  // some reflected (like sizes and such) and some not), and -moz-appearance
+  // (whose changes are not), so we need to recascade for the first, and reflow
+  // for the rest.
+  auto restyleHint = (kind & widget::ThemeChangeKind::Style)
+                         ? RestyleHint::RecascadeSubtree()
+                         : RestyleHint{0};
+  auto changeHint = (kind & widget::ThemeChangeKind::Layout)
+                        ? NS_STYLE_HINT_REFLOW
+                        : nsChangeHint(0);
   MediaFeatureValuesChanged(
-      {
-          RestyleHint::RecascadeSubtree(),
-          NS_STYLE_HINT_REFLOW,
-          MediaFeatureChangeReason::SystemMetricsChange,
-      },
+      {restyleHint, changeHint, MediaFeatureChangeReason::SystemMetricsChange},
       MediaFeatureChangePropagation::All);
 }
 

layout/base/nsPresContext.h

@@ -22,6 +22,7 @@
 #include "mozilla/TimeStamp.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/WeakPtr.h"
+#include "mozilla/widget/ThemeChangeKind.h"
 #include "nsColor.h"
 #include "nsCompatibility.h"
 #include "nsCoord.h"
@@ -789,7 +790,7 @@ class nsPresContext : public nsISupports, public mozilla::SupportsWeakPtr {
    * Classic). Otherwise, the OS is telling us that the native theme for the
    * platform has changed.
    */
-  void ThemeChanged();
+  void ThemeChanged(mozilla::widget::ThemeChangeKind);
 
   /*
    * Notify the pres context that the resolution of the user interface has
@@ -1264,6 +1265,11 @@ class nsPresContext : public nsISupports, public mozilla::SupportsWeakPtr {
   bool mInflationDisabledForShrinkWrap;
 
  protected:
+  static constexpr size_t kThemeChangeKindBits = 2;
+  static_assert(unsigned(mozilla::widget::ThemeChangeKind::AllBits) <=
+                    (1u << kThemeChangeKindBits) - 1,
+                "theme change kind doesn't fit");
+
   unsigned mInteractionTimeEnabled : 1;
   unsigned mHasPendingInterrupt : 1;
   unsigned mPendingInterruptFromTest : 1;
@@ -1279,6 +1285,8 @@ class nsPresContext : public nsISupports, public mozilla::SupportsWeakPtr {
   unsigned mPrefBidiDirection : 1;
   unsigned mPrefScrollbarSide : 2;
   unsigned mPendingThemeChanged : 1;
+  // widget::ThemeChangeKind
+  unsigned mPendingThemeChangeKind : kThemeChangeKindBits;
   unsigned mPendingUIResolutionChanged : 1;
   unsigned mPostedPrefChangedRunnable : 1;
 

widget/LookAndFeel.h

@@ -14,6 +14,7 @@
 #include "nsColor.h"
 #include "nsString.h"
 #include "nsTArray.h"
+#include "mozilla/widget/ThemeChangeKind.h"
 
 struct gfxFontStyle;
 
@@ -547,7 +548,7 @@ class LookAndFeel {
    */
   static LookAndFeelCache GetCache();
   static void SetCache(const LookAndFeelCache& aCache);
-  static void NotifyChangedAllWindows();
+  static void NotifyChangedAllWindows(widget::ThemeChangeKind);
 };
 
 }  // namespace mozilla

widget/ThemeChangeKind.h

@@ -0,0 +1,35 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* 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_widget_ThemeChangeKind
+#define mozilla_widget_ThemeChangeKind
+
+#include "mozilla/TypedEnumBits.h"
+
+namespace mozilla::widget {
+
+enum class ThemeChangeKind : uint8_t {
+  // This is the cheapest change, no need to forcibly recompute style and/or
+  // layout.
+  MediaQueriesOnly = 0,
+  // Style needs to forcibly be recomputed because some of the stuff that may
+  // have changed, like system colors, are reflected in the computed style but
+  // not in the specified style.
+  Style = 1 << 0,
+  // Layout needs to forcibly be recomputed because some of the stuff that may
+  // have changed is layout-dependent, like system font.
+  Layout = 1 << 1,
+  // The union of the two flags above.
+  StyleAndLayout = Style | Layout,
+  // For IPC serialization purposes.
+  AllBits = Style | Layout,
+};
+
+MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(ThemeChangeKind)
+
+}  // namespace mozilla::widget
+
+#endif

widget/WidgetMessageUtils.h

@@ -7,6 +7,7 @@
 
 #include "ipc/IPCMessageUtils.h"
 #include "mozilla/LookAndFeel.h"
+#include "mozilla/widget/ThemeChangeKind.h"
 #include "nsIWidget.h"
 
 namespace IPC {
@@ -32,6 +33,12 @@ struct ParamTraits<LookAndFeelInt> {
   }
 };
 
+template <>
+struct ParamTraits<mozilla::widget::ThemeChangeKind>
+    : public BitFlagsEnumSerializer<mozilla::widget::ThemeChangeKind,
+                                    mozilla::widget::ThemeChangeKind::AllBits> {
+};
+
 template <>
 struct ParamTraits<LookAndFeelFont> {
   typedef LookAndFeelFont paramType;

widget/android/GeckoSystemStateListener.h

@@ -19,7 +19,10 @@ class GeckoSystemStateListener final
  public:
   static void OnDeviceChanged() {
     MOZ_ASSERT(NS_IsMainThread());
-    mozilla::LookAndFeel::NotifyChangedAllWindows();
+    // TODO(emilio, bug 1673318): This could become more granular and avoid work
+    // if we get whether these are layout/style-affecting from the caller.
+    mozilla::LookAndFeel::NotifyChangedAllWindows(
+        widget::ThemeChangeKind::StyleAndLayout);
   }
 };
 

widget/cocoa/nsChildView.mm

@@ -2502,7 +2502,10 @@ NSEvent* gLastDragMouseDownEvent = nil;  // [strong]
 }
 
 - (void)systemMetricsChanged {
-  if (mGeckoChild) mGeckoChild->NotifyThemeChanged();
+  // TODO(emilio): We could make this more fine-grained by only passing true
+  // here when system colors / fonts change, but right now we tunnel all the
+  // relevant notifications through here.
+  if (mGeckoChild) mGeckoChild->NotifyThemeChanged(widget::ThemeChangeKind::StyleAndLayout);
 }
 
 - (void)scrollbarSystemMetricChanged {

widget/gtk/nsWindow.cpp

@@ -3991,7 +3991,8 @@ void nsWindow::OnWindowStateEvent(GtkWidget* aWidget,
 }
 
 void nsWindow::ThemeChanged() {
-  NotifyThemeChanged();
+  // Everything could've changed.
+  NotifyThemeChanged(ThemeChangeKind::StyleAndLayout);
 
   if (!mGdkWindow || MOZ_UNLIKELY(mIsDestroyed)) return;
 
@@ -4017,8 +4018,9 @@ void nsWindow::OnDPIChanged() {
   if (mWidgetListener) {
     if (PresShell* presShell = mWidgetListener->GetPresShell()) {
       presShell->BackingScaleFactorChanged();
-      // Update menu's font size etc
-      presShell->ThemeChanged();
+      // Update menu's font size etc.
+      // This affects style / layout because it affects system font sizes.
+      presShell->ThemeChanged(ThemeChangeKind::StyleAndLayout);
     }
     mWidgetListener->UIResolutionChanged();
   }
@@ -4027,12 +4029,9 @@ void nsWindow::OnDPIChanged() {
 void nsWindow::OnCheckResize() { mPendingConfigures++; }
 
 void nsWindow::OnCompositedChanged() {
-  if (mWidgetListener) {
-    if (PresShell* presShell = mWidgetListener->GetPresShell()) {
-      // Update CSD after the change in alpha visibility
-      presShell->ThemeChanged();
-    }
-  }
+  // Update CSD after the change in alpha visibility. This only affects
+  // system metrics, not other theme shenanigans.
+  NotifyThemeChanged(ThemeChangeKind::MediaQueriesOnly);
 }
 
 void nsWindow::OnScaleChanged(GtkAllocation* aAllocation) {

widget/moz.build

@@ -181,6 +181,7 @@ EXPORTS.mozilla.widget += [
     "PuppetBidiKeyboard.h",
     "Screen.h",
     "ScreenManager.h",
+    "ThemeChangeKind.h",
     "WidgetMessageUtils.h",
     "WindowSurface.h",
 ]

widget/nsBaseWidget.cpp

@@ -1673,12 +1673,12 @@ void nsBaseWidget::NotifySizeMoveDone() {
   }
 }
 
-void nsBaseWidget::NotifyThemeChanged() {
+void nsBaseWidget::NotifyThemeChanged(ThemeChangeKind aKind) {
   if (!mWidgetListener) {
     return;
   }
   if (PresShell* presShell = mWidgetListener->GetPresShell()) {
-    presShell->ThemeChanged();
+    presShell->ThemeChanged(aKind);
   }
 }
 

widget/nsBaseWidget.h

@@ -14,6 +14,7 @@
 #include "mozilla/layers/APZCCallbackHelper.h"
 #include "mozilla/layers/CompositorOptions.h"
 #include "mozilla/layers/NativeLayer.h"
+#include "mozilla/widget/ThemeChangeKind.h"
 #include "nsRect.h"
 #include "nsIWidget.h"
 #include "nsWidgetsCID.h"
@@ -349,7 +350,7 @@ class nsBaseWidget : public nsIWidget, public nsSupportsWeakReference {
 
   // Should be called by derived implementations to notify on system color and
   // theme changes.
-  void NotifyThemeChanged();
+  void NotifyThemeChanged(mozilla::widget::ThemeChangeKind);
   void NotifyUIStateChanged(UIStateChangeType aShowFocusRings);
 
 #ifdef ACCESSIBILITY

widget/nsXPLookAndFeel.cpp

@@ -294,7 +294,8 @@ void nsXPLookAndFeel::IntPrefChanged(nsLookAndFeelIntPref* data) {
 #endif
   }
 
-  NotifyChangedAllWindows();
+  // Int prefs can't change our system colors or fonts.
+  NotifyChangedAllWindows(widget::ThemeChangeKind::MediaQueriesOnly);
 }
 
 // static
@@ -320,7 +321,8 @@ void nsXPLookAndFeel::FloatPrefChanged(nsLookAndFeelFloatPref* data) {
 #endif
   }
 
-  NotifyChangedAllWindows();
+  // Float prefs can't change our system colors or fonts.
+  NotifyChangedAllWindows(widget::ThemeChangeKind::MediaQueriesOnly);
 }
 
 // static
@@ -354,7 +356,8 @@ void nsXPLookAndFeel::ColorPrefChanged(unsigned int index,
 #endif
   }
 
-  NotifyChangedAllWindows();
+  // Color prefs affect style, because they by definition change system colors.
+  NotifyChangedAllWindows(widget::ThemeChangeKind::Style);
 }
 
 void nsXPLookAndFeel::InitFromPref(nsLookAndFeelIntPref* aPref) {
@@ -1031,9 +1034,10 @@ void nsXPLookAndFeel::RecordTelemetry() {
 namespace mozilla {
 
 // static
-void LookAndFeel::NotifyChangedAllWindows() {
+void LookAndFeel::NotifyChangedAllWindows(widget::ThemeChangeKind aKind) {
   if (nsCOMPtr<nsIObserverService> obs = services::GetObserverService()) {
-    obs->NotifyObservers(nullptr, "look-and-feel-changed", nullptr);
+    obs->NotifyObservers(nullptr, "look-and-feel-changed",
+                         reinterpret_cast<char16_t*>(uintptr_t(aKind)));
   }
 }
 

widget/windows/nsWindow.cpp

@@ -5215,7 +5215,9 @@ bool nsWindow::ProcessMessage(UINT msg, WPARAM& wParam, LPARAM& lParam,
       break;
 
     case WM_SYSCOLORCHANGE:
-      NotifyThemeChanged();
+      // No need to invalidate layout for system color changes, but we need to
+      // invalidate style.
+      NotifyThemeChanged(widget::ThemeChangeKind::Style);
       break;
 
     case WM_THEMECHANGED: {
@@ -5226,7 +5228,8 @@ bool nsWindow::ProcessMessage(UINT msg, WPARAM& wParam, LPARAM& lParam,
       UpdateNonClientMargins();
       nsUXThemeData::UpdateNativeThemeInfo();
 
-      NotifyThemeChanged();
+      // We assume pretty much everything could've changed here.
+      NotifyThemeChanged(widget::ThemeChangeKind::StyleAndLayout);
 
       // Invalidate the window so that the repaint will
       // pick up the new theme.
@@ -5270,7 +5273,9 @@ bool nsWindow::ProcessMessage(UINT msg, WPARAM& wParam, LPARAM& lParam,
       if (wParam == SPI_SETCLIENTAREAANIMATION ||
           // CaretBlinkTime is cached in nsLookAndFeel
           wParam == SPI_SETKEYBOARDDELAY) {
-        NotifyThemeChanged();
+        // This only affects reduced motion settings and and carent blink time,
+        // so no need to invalidate style / layout.
+        NotifyThemeChanged(widget::ThemeChangeKind::MediaQueriesOnly);
         break;
       }
       if (wParam == SPI_SETFONTSMOOTHING ||
@@ -5281,7 +5286,9 @@ bool nsWindow::ProcessMessage(UINT msg, WPARAM& wParam, LPARAM& lParam,
       if (lParam) {
         auto lParamString = reinterpret_cast<const wchar_t*>(lParam);
         if (!wcscmp(lParamString, L"ImmersiveColorSet")) {
-          NotifyThemeChanged();
+          // This affects system colors (-moz-win-accentcolor), so gotta pass
+          // the style flag.
+          NotifyThemeChanged(widget::ThemeChangeKind::Style);
           break;
         }
         if (IsWin10OrLater() && mWindowType == eWindowType_invisible) {
@@ -5304,7 +5311,8 @@ bool nsWindow::ProcessMessage(UINT msg, WPARAM& wParam, LPARAM& lParam,
         // DBT_DEVTYP_DEVICEINTERFACE in the filter for
         // RegisterDeviceNotification.
         if (hdr->dbch_devicetype == DBT_DEVTYP_DEVICEINTERFACE) {
-          NotifyThemeChanged();
+          // This can only change media queries (any-hover/any-pointer).
+          NotifyThemeChanged(widget::ThemeChangeKind::MediaQueriesOnly);
         }
       }
     } break;
@@ -6147,7 +6155,9 @@ bool nsWindow::ProcessMessage(UINT msg, WPARAM& wParam, LPARAM& lParam,
 
       UpdateNonClientMargins();
       BroadcastMsg(mWnd, WM_DWMCOMPOSITIONCHANGED);
-      NotifyThemeChanged();
+      // TODO: Why is NotifyThemeChanged needed, what does it affect? And can we
+      // make it more granular by tweaking the ChangeKind we pass?
+      NotifyThemeChanged(widget::ThemeChangeKind::StyleAndLayout);
       UpdateGlass();
       Invalidate(true, true, true);
       break;