Bug 1255214 - Only repaint GTK scrollbar button if its enablement actually changed. r=mstange
authorBotond Ballo <botond@mozilla.com>
Wed, 20 Apr 2016 19:49:09 -0400
changeset 332444 9f69fc58466ed28fd178c3328995cb5082dab809
parent 332443 5554679e1574dee489253aaaa6c2e1320d9aceba
child 332445 bcfbc52d5b5db28cbd65253f9f56eddbc7bc2932
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1255214
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 1255214 - Only repaint GTK scrollbar button if its enablement actually changed. r=mstange MozReview-Commit-ID: ITLeksQzvVM
gfx/src/nsITheme.h
layout/base/RestyleManager.cpp
layout/xul/nsScrollbarFrame.cpp
widget/cocoa/nsNativeThemeCocoa.h
widget/cocoa/nsNativeThemeCocoa.mm
widget/gtk/nsNativeThemeGTK.cpp
widget/gtk/nsNativeThemeGTK.h
widget/windows/nsNativeThemeWin.cpp
widget/windows/nsNativeThemeWin.h
--- a/gfx/src/nsITheme.h
+++ b/gfx/src/nsITheme.h
@@ -10,16 +10,17 @@
 #define nsITheme_h_
 
 #include "nsISupports.h"
 #include "nsCOMPtr.h"
 #include "nsColor.h"
 #include "Units.h"
 
 struct nsRect;
+class nsAttrValue;
 class nsPresContext;
 class nsRenderingContext;
 class nsDeviceContext;
 class nsIFrame;
 class nsIAtom;
 class nsIWidget;
 
 // IID for the nsITheme interface
@@ -123,17 +124,18 @@ public:
 
   /**
    * Returns what we know about the transparency of the widget.
    */
   virtual Transparency GetWidgetTransparency(nsIFrame* aFrame, uint8_t aWidgetType)
   { return eUnknownTransparency; }
 
   NS_IMETHOD WidgetStateChanged(nsIFrame* aFrame, uint8_t aWidgetType, 
-                                nsIAtom* aAttribute, bool* aShouldRepaint)=0;
+                                nsIAtom* aAttribute, bool* aShouldRepaint,
+                                const nsAttrValue* aOldValue)=0;
 
   NS_IMETHOD ThemeChanged()=0;
 
   virtual bool WidgetAppearanceDependsOnWindowFocus(uint8_t aWidgetType)
   { return false; }
 
   virtual bool NeedToClearBackgroundBehindWidget(nsIFrame* aFrame,
                                                  uint8_t aWidgetType)
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -1186,17 +1186,17 @@ RestyleManager::ContentStateChanged(nsIC
       hint = nsChangeHint_ReconstructFrame;
     } else {
       uint8_t app = primaryFrame->StyleDisplay()->mAppearance;
       if (app) {
         nsITheme *theme = mPresContext->GetTheme();
         if (theme && theme->ThemeSupportsWidget(mPresContext,
                                                 primaryFrame, app)) {
           bool repaint = false;
-          theme->WidgetStateChanged(primaryFrame, app, nullptr, &repaint);
+          theme->WidgetStateChanged(primaryFrame, app, nullptr, &repaint, nullptr);
           if (repaint) {
             NS_UpdateHint(hint, nsChangeHint_RepaintFrame);
           }
         }
       }
     }
 
     pseudoType = primaryFrame->StyleContext()->GetPseudoType();
@@ -1315,17 +1315,18 @@ RestyleManager::AttributeChanged(Element
 
   if (primaryFrame) {
     // See if we have appearance information for a theme.
     const nsStyleDisplay* disp = primaryFrame->StyleDisplay();
     if (disp->mAppearance) {
       nsITheme *theme = mPresContext->GetTheme();
       if (theme && theme->ThemeSupportsWidget(mPresContext, primaryFrame, disp->mAppearance)) {
         bool repaint = false;
-        theme->WidgetStateChanged(primaryFrame, disp->mAppearance, aAttribute, &repaint);
+        theme->WidgetStateChanged(primaryFrame, disp->mAppearance, aAttribute,
+            &repaint, aOldValue);
         if (repaint)
           NS_UpdateHint(hint, nsChangeHint_RepaintFrame);
       }
     }
 
     // let the frame deal with it now, so we don't have to deal later
     primaryFrame->AttributeChanged(aNameSpaceID, aAttribute, aModType);
     // XXXwaterson should probably check for IB split siblings
--- a/layout/xul/nsScrollbarFrame.cpp
+++ b/layout/xul/nsScrollbarFrame.cpp
@@ -244,16 +244,19 @@ nsScrollbarFrame::MoveToNewPosition()
   nsCOMPtr<nsIContent> content = GetContent();
 
   // get the current pos
   int32_t curpos = nsSliderFrame::GetCurrentPosition(content);
 
   // get the max pos
   int32_t maxpos = nsSliderFrame::GetMaxPosition(content);
 
+  // save the old curpos
+  int32_t oldCurpos = curpos;
+
   // increment the given amount
   if (mIncrement) {
     curpos += mIncrement;
   }
 
   // make sure the current position is between the current and max positions
   if (curpos < 0) {
     curpos = 0;
@@ -292,14 +295,17 @@ nsScrollbarFrame::MoveToNewPosition()
   }
   // See if we have appearance information for a theme.
   const nsStyleDisplay* disp = StyleDisplay();
   nsPresContext* presContext = PresContext();
   if (disp->mAppearance) {
     nsITheme *theme = presContext->GetTheme();
     if (theme && theme->ThemeSupportsWidget(presContext, this, disp->mAppearance)) {
       bool repaint;
-      theme->WidgetStateChanged(this, disp->mAppearance, nsGkAtoms::curpos, &repaint);
+      nsAttrValue oldValue;
+      oldValue.SetTo(oldCurpos);
+      theme->WidgetStateChanged(this, disp->mAppearance, nsGkAtoms::curpos,
+          &repaint, &oldValue);
     }
   }
   content->UnsetAttr(kNameSpaceID_None, nsGkAtoms::smooth, false);
   return curpos;
 }
--- a/widget/cocoa/nsNativeThemeCocoa.h
+++ b/widget/cocoa/nsNativeThemeCocoa.h
@@ -64,17 +64,18 @@ public:
 
   virtual bool GetWidgetOverflow(nsDeviceContext* aContext, nsIFrame* aFrame,
                                    uint8_t aWidgetType, nsRect* aOverflowRect) override;
 
   NS_IMETHOD GetMinimumWidgetSize(nsPresContext* aPresContext, nsIFrame* aFrame,
                                   uint8_t aWidgetType,
                                   mozilla::LayoutDeviceIntSize* aResult, bool* aIsOverridable) override;
   NS_IMETHOD WidgetStateChanged(nsIFrame* aFrame, uint8_t aWidgetType, 
-                                nsIAtom* aAttribute, bool* aShouldRepaint) override;
+                                nsIAtom* aAttribute, bool* aShouldRepaint,
+                                const nsAttrValue* aOldValue) override;
   NS_IMETHOD ThemeChanged() override;
   bool ThemeSupportsWidget(nsPresContext* aPresContext, nsIFrame* aFrame, uint8_t aWidgetType) override;
   bool WidgetIsContainer(uint8_t aWidgetType) override;
   bool ThemeDrawsFocusForWidget(uint8_t aWidgetType) override;
   bool ThemeNeedsComboboxDropmarker() override;
   virtual bool WidgetAppearanceDependsOnWindowFocus(uint8_t aWidgetType) override;
   virtual bool NeedToClearBackgroundBehindWidget(nsIFrame* aFrame,
                                                  uint8_t aWidgetType) override;
--- a/widget/cocoa/nsNativeThemeCocoa.mm
+++ b/widget/cocoa/nsNativeThemeCocoa.mm
@@ -3583,17 +3583,18 @@ nsNativeThemeCocoa::GetMinimumWidgetSize
 
   return NS_OK;
 
   NS_OBJC_END_TRY_ABORT_BLOCK_NSRESULT;
 }
 
 NS_IMETHODIMP
 nsNativeThemeCocoa::WidgetStateChanged(nsIFrame* aFrame, uint8_t aWidgetType, 
-                                     nsIAtom* aAttribute, bool* aShouldRepaint)
+                                       nsIAtom* aAttribute, bool* aShouldRepaint,
+                                       const nsAttrValue* aOldValue)
 {
   // Some widget types just never change state.
   switch (aWidgetType) {
     case NS_THEME_WINDOW_TITLEBAR:
     case NS_THEME_TOOLBOX:
     case NS_THEME_TOOLBAR:
     case NS_THEME_STATUSBAR:
     case NS_THEME_STATUSBAR_PANEL:
--- a/widget/gtk/nsNativeThemeGTK.cpp
+++ b/widget/gtk/nsNativeThemeGTK.cpp
@@ -18,16 +18,17 @@
 #include "nsNameSpaceManager.h"
 #include "nsGfxCIID.h"
 #include "nsTransform2D.h"
 #include "nsMenuFrame.h"
 #include "prlink.h"
 #include "nsIDOMHTMLInputElement.h"
 #include "nsRenderingContext.h"
 #include "nsGkAtoms.h"
+#include "nsAttrValueInlines.h"
 
 #include "mozilla/EventStates.h"
 #include "mozilla/Services.h"
 
 #include <gdk/gdkprivate.h>
 #include <gtk/gtk.h>
 
 #include "gfxContext.h"
@@ -171,16 +172,25 @@ nsNativeThemeGTK::GetTabMarginPixels(nsI
     IsBottomTab(aFrame) ? aFrame->GetUsedMargin().top
     : aFrame->GetUsedMargin().bottom;
 
   return std::min<gint>(MOZ_GTK_TAB_MARGIN_MASK,
                 std::max(0,
                        aFrame->PresContext()->AppUnitsToDevPixels(-margin)));
 }
 
+static bool ShouldScrollbarButtonBeDisabled(int32_t aCurpos, int32_t aMaxpos,
+                                            uint8_t aWidgetType)
+{
+  return ((aCurpos == 0 && (aWidgetType == NS_THEME_SCROLLBAR_BUTTON_UP ||
+                            aWidgetType == NS_THEME_SCROLLBAR_BUTTON_LEFT))
+      || (aCurpos == aMaxpos && (aWidgetType == NS_THEME_SCROLLBAR_BUTTON_DOWN ||
+                                 aWidgetType == NS_THEME_SCROLLBAR_BUTTON_RIGHT)));
+}
+
 bool
 nsNativeThemeGTK::GetGtkWidgetAndState(uint8_t aWidgetType, nsIFrame* aFrame,
                                        GtkThemeWidgetType& aGtkWidgetType,
                                        GtkWidgetState* aState,
                                        gint* aWidgetFlags)
 {
   if (aState) {
     if (!aFrame) {
@@ -303,22 +313,19 @@ nsNativeThemeGTK::GetGtkWidgetAndState(u
         if (aWidgetType == NS_THEME_SCROLLBAR_BUTTON_UP ||
             aWidgetType == NS_THEME_SCROLLBAR_BUTTON_DOWN ||
             aWidgetType == NS_THEME_SCROLLBAR_BUTTON_LEFT ||
             aWidgetType == NS_THEME_SCROLLBAR_BUTTON_RIGHT) {
           // set the state to disabled when the scrollbar is scrolled to
           // the beginning or the end, depending on the button type.
           int32_t curpos = CheckIntAttr(aFrame, nsGkAtoms::curpos, 0);
           int32_t maxpos = CheckIntAttr(aFrame, nsGkAtoms::maxpos, 100);
-          if ((curpos == 0 && (aWidgetType == NS_THEME_SCROLLBAR_BUTTON_UP ||
-                aWidgetType == NS_THEME_SCROLLBAR_BUTTON_LEFT)) ||
-              (curpos == maxpos &&
-               (aWidgetType == NS_THEME_SCROLLBAR_BUTTON_DOWN ||
-                aWidgetType == NS_THEME_SCROLLBAR_BUTTON_RIGHT)))
+          if (ShouldScrollbarButtonBeDisabled(curpos, maxpos, aWidgetType)) {
             aState->disabled = true;
+          }
 
           // In order to simulate native GTK scrollbar click behavior,
           // we set the active attribute on the element to true if it's
           // pressed with any mouse button.
           // This allows us to show that it's active without setting :active
           else if (CheckBooleanAttr(aFrame, nsGkAtoms::active))
             aState->active = true;
 
@@ -1639,17 +1646,18 @@ nsNativeThemeGTK::GetMinimumWidgetSize(n
 
   *aResult = *aResult * nsScreenGtk::GetGtkMonitorScaleFactor();
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNativeThemeGTK::WidgetStateChanged(nsIFrame* aFrame, uint8_t aWidgetType, 
-                                     nsIAtom* aAttribute, bool* aShouldRepaint)
+                                     nsIAtom* aAttribute, bool* aShouldRepaint,
+                                     const nsAttrValue* aOldValue)
 {
   // Some widget types just never change state.
   if (aWidgetType == NS_THEME_TOOLBOX ||
       aWidgetType == NS_THEME_TOOLBAR ||
       aWidgetType == NS_THEME_STATUSBAR ||
       aWidgetType == NS_THEME_STATUSBAR_PANEL ||
       aWidgetType == NS_THEME_STATUSBAR_RESIZER_PANEL ||
       aWidgetType == NS_THEME_PROGRESSBAR_CHUNK ||
@@ -1674,17 +1682,35 @@ nsNativeThemeGTK::WidgetStateChanged(nsI
   }
 
   if ((aWidgetType == NS_THEME_SCROLLBAR_BUTTON_UP ||
        aWidgetType == NS_THEME_SCROLLBAR_BUTTON_DOWN ||
        aWidgetType == NS_THEME_SCROLLBAR_BUTTON_LEFT ||
        aWidgetType == NS_THEME_SCROLLBAR_BUTTON_RIGHT) &&
       (aAttribute == nsGkAtoms::curpos ||
        aAttribute == nsGkAtoms::maxpos)) {
-    *aShouldRepaint = true;
+    // If 'curpos' has changed and we are passed its old value, we can
+    // determine whether the button's enablement actually needs to change.
+    if (aAttribute == nsGkAtoms::curpos && aOldValue) {
+      int32_t curpos = CheckIntAttr(aFrame, nsGkAtoms::curpos, 0);
+      int32_t maxpos = CheckIntAttr(aFrame, nsGkAtoms::maxpos, 0);
+      nsAutoString str;
+      aOldValue->ToString(str);
+      nsresult err;
+      int32_t oldCurpos = str.ToInteger(&err);
+      if (str.IsEmpty() || NS_FAILED(err)) {
+        *aShouldRepaint = true;
+      } else {
+        bool disabledBefore = ShouldScrollbarButtonBeDisabled(oldCurpos, maxpos, aWidgetType);
+        bool disabledNow = ShouldScrollbarButtonBeDisabled(curpos, maxpos, aWidgetType);
+        *aShouldRepaint = (disabledBefore != disabledNow);
+      }
+    } else {
+      *aShouldRepaint = true;
+    }
     return NS_OK;
   }
 
   // XXXdwh Not sure what can really be done here.  Can at least guess for
   // specific widgets that they're highly unlikely to have certain states.
   // For example, a toolbar doesn't care about any states.
   if (!aAttribute) {
     // Hover/focus/active changed.  Always repaint.
--- a/widget/gtk/nsNativeThemeGTK.h
+++ b/widget/gtk/nsNativeThemeGTK.h
@@ -45,17 +45,18 @@ public:
 
   NS_IMETHOD GetMinimumWidgetSize(nsPresContext* aPresContext,
                                   nsIFrame* aFrame, uint8_t aWidgetType,
                                   mozilla::LayoutDeviceIntSize* aResult,
                                   bool* aIsOverridable) override;
 
   NS_IMETHOD WidgetStateChanged(nsIFrame* aFrame, uint8_t aWidgetType, 
                                 nsIAtom* aAttribute,
-                                bool* aShouldRepaint) override;
+                                bool* aShouldRepaint,
+                                const nsAttrValue* aOldValue) override;
 
   NS_IMETHOD ThemeChanged() override;
 
   NS_IMETHOD_(bool) ThemeSupportsWidget(nsPresContext* aPresContext,
                                         nsIFrame* aFrame,
                                         uint8_t aWidgetType) override;
 
   NS_IMETHOD_(bool) WidgetIsContainer(uint8_t aWidgetType) override;
--- a/widget/windows/nsNativeThemeWin.cpp
+++ b/widget/windows/nsNativeThemeWin.cpp
@@ -2569,17 +2569,18 @@ nsNativeThemeWin::GetMinimumWidgetSize(n
   ::ReleaseDC(nullptr, hdc);
 
   ScaleForFrameDPI(aResult, aFrame);
   return rv;
 }
 
 NS_IMETHODIMP
 nsNativeThemeWin::WidgetStateChanged(nsIFrame* aFrame, uint8_t aWidgetType, 
-                                     nsIAtom* aAttribute, bool* aShouldRepaint)
+                                     nsIAtom* aAttribute, bool* aShouldRepaint,
+                                     const nsAttrValue* aOldValue)
 {
   // Some widget types just never change state.
   if (aWidgetType == NS_THEME_TOOLBOX ||
       aWidgetType == NS_THEME_WIN_MEDIA_TOOLBOX ||
       aWidgetType == NS_THEME_WIN_COMMUNICATIONS_TOOLBOX ||
       aWidgetType == NS_THEME_WIN_BROWSER_TAB_BAR_TOOLBOX ||
       aWidgetType == NS_THEME_TOOLBAR ||
       aWidgetType == NS_THEME_STATUSBAR || aWidgetType == NS_THEME_STATUSBAR_PANEL ||
--- a/widget/windows/nsNativeThemeWin.h
+++ b/widget/windows/nsNativeThemeWin.h
@@ -51,17 +51,18 @@ public:
   NS_IMETHOD GetMinimumWidgetSize(nsPresContext* aPresContext, nsIFrame* aFrame,
                                   uint8_t aWidgetType,
                                   mozilla::LayoutDeviceIntSize* aResult,
                                   bool* aIsOverridable) override;
 
   virtual Transparency GetWidgetTransparency(nsIFrame* aFrame, uint8_t aWidgetType) override;
 
   NS_IMETHOD WidgetStateChanged(nsIFrame* aFrame, uint8_t aWidgetType, 
-                                nsIAtom* aAttribute, bool* aShouldRepaint) override;
+                                nsIAtom* aAttribute, bool* aShouldRepaint,
+                                const nsAttrValue* aOldValue) override;
 
   NS_IMETHOD ThemeChanged() override;
 
   bool ThemeSupportsWidget(nsPresContext* aPresContext, 
                              nsIFrame* aFrame,
                              uint8_t aWidgetType) override;
 
   bool WidgetIsContainer(uint8_t aWidgetType) override;