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 318347 9f69fc58466ed28fd178c3328995cb5082dab809
parent 318346 5554679e1574dee489253aaaa6c2e1320d9aceba
child 318348 bcfbc52d5b5db28cbd65253f9f56eddbc7bc2932
push id9480
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 17:12:58 +0000
treeherdermozilla-aurora@0d6a91c76a9e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1255214
milestone48.0a1
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;