Bug 1380629 - Check GetTheme result in nsNativeThemeWin r?jimm draft
authorDoug Thayer <dothayer@mozilla.com>
Mon, 17 Jul 2017 13:55:28 -0700
changeset 610064 90405e4634667a69321dd739f6c29260fc27c9e4
parent 609768 d43779e278d2e4d3e21dba2fcb585a3bf4b1288e
child 637758 20a61f216fb22437083714ca3c02d7ed099dd03b
push id68780
push userbmo:dothayer@mozilla.com
push dateMon, 17 Jul 2017 21:22:26 +0000
reviewersjimm
bugs1380629, 1373079
milestone56.0a1
Bug 1380629 - Check GetTheme result in nsNativeThemeWin r?jimm The fix for bug 1373079 neglected the detail of OpenThemeData that it can return null if no match is found for the specified class name. The set of matching class data sections varies with the default and the classic theme, and the classic theme doesn't have matches for a few of the values that we try to get. This causes us to pass a null theme to subsequent functions, which of course breaks the layout. MozReview-Commit-ID: 5LaR0qZlOzd
widget/windows/nsNativeThemeWin.cpp
widget/windows/nsNativeThemeWin.h
--- a/widget/windows/nsNativeThemeWin.cpp
+++ b/widget/windows/nsNativeThemeWin.cpp
@@ -589,36 +589,36 @@ nsNativeThemeWin::DrawThemedProgressMete
                         &adjClipRect);
 
     if (!QueueAnimatedContentForRefresh(aFrame->GetContent(), 60)) {
       NS_WARNING("unable to animate progress widget!");
     }
   }
 }
 
-nsresult nsNativeThemeWin::GetCachedWidgetBorder(nsIFrame * aFrame, nsUXThemeClass aThemeClass,
-                                                 uint8_t aWidgetType, int32_t aPart, int32_t aState,
+nsresult nsNativeThemeWin::GetCachedWidgetBorder(nsIFrame * aFrame, HTHEME aTheme,
+                                                 nsUXThemeClass aThemeClass, uint8_t aWidgetType,
+                                                 int32_t aPart, int32_t aState,
                                                  nsIntMargin * aResult)
 {
   int32_t cacheIndex = aThemeClass * THEME_PART_DISTINCT_VALUE_COUNT + aPart;
   int32_t cacheBitIndex = cacheIndex / 8;
   uint8_t cacheBit = 1u << (cacheIndex % 8);
 
   if (mBorderCacheValid[cacheBitIndex] & cacheBit) {
     *aResult = mBorderCache[cacheIndex];
     return NS_OK;
   }
 
-  HANDLE theme = nsUXThemeData::GetTheme(aThemeClass);
   // Get our info.
   RECT outerRect; // Create a fake outer rect.
   outerRect.top = outerRect.left = 100;
   outerRect.right = outerRect.bottom = 200;
   RECT contentRect(outerRect);
-  HRESULT res = GetThemeBackgroundContentRect(theme, nullptr, aPart, aState, &outerRect, &contentRect);
+  HRESULT res = GetThemeBackgroundContentRect(aTheme, nullptr, aPart, aState, &outerRect, &contentRect);
 
   if (FAILED(res)) {
     return NS_ERROR_FAILURE;
   }
 
   // Now compute the delta in each direction and place it in our
   // nsIntMargin struct.
   aResult->top = contentRect.top - outerRect.top;
@@ -634,22 +634,22 @@ nsresult nsNativeThemeWin::GetCachedWidg
 
 nsresult nsNativeThemeWin::GetCachedMinimumWidgetSize(nsIFrame * aFrame, HANDLE aTheme,
                                                       nsUXThemeClass aThemeClass, uint8_t aWidgetType,
                                                       int32_t aPart, int32_t aState, THEMESIZE aSizeReq,
                                                       mozilla::LayoutDeviceIntSize * aResult)
 {
   int32_t cachePart = aPart;
 
-  if (aWidgetType == NS_THEME_BUTTON && aSizeReq == TS_MIN) {
-    // In practice, NS_THEME_BUTTON is the only widget type which has an aSizeReq
-    // that varies for us, and it can only be TS_MIN or TS_TRUE. Just stuff that
-    // extra bit into the aPart part of the cache, since BP_Count is well below
-    // THEME_PART_DISTINCT_VALUE_COUNT anyway.
-    cachePart = BP_Count;
+  if (aWidgetType == NS_THEME_BUTTON && aSizeReq == TS_MIN) {
+    // In practice, NS_THEME_BUTTON is the only widget type which has an aSizeReq
+    // that varies for us, and it can only be TS_MIN or TS_TRUE. Just stuff that
+    // extra bit into the aPart part of the cache, since BP_Count is well below
+    // THEME_PART_DISTINCT_VALUE_COUNT anyway.
+    cachePart = BP_Count;
   }
 
   MOZ_ASSERT(aPart < THEME_PART_DISTINCT_VALUE_COUNT);
   int32_t cacheIndex = aThemeClass * THEME_PART_DISTINCT_VALUE_COUNT + cachePart;
   int32_t cacheBitIndex = cacheIndex / 8;
   uint8_t cacheBit = 1u << (cacheIndex % 8);
 
   if (mMinimumWidgetSizeCacheValid[cacheBitIndex] & cacheBit) {
@@ -1930,18 +1930,22 @@ ScaleForFrameDPI(LayoutDeviceIntSize* aS
 
 NS_IMETHODIMP
 nsNativeThemeWin::GetWidgetBorder(nsDeviceContext* aContext, 
                                   nsIFrame* aFrame,
                                   uint8_t aWidgetType,
                                   nsIntMargin* aResult)
 {
   mozilla::Maybe<nsUXThemeClass> themeClass = GetThemeClass(aWidgetType);
+  HTHEME theme = NULL;
+  if (!themeClass.isNothing()) {
+    theme = nsUXThemeData::GetTheme(themeClass.value());
+  }
   nsresult rv = NS_OK;
-  if (themeClass.isNothing()) {
+  if (!theme) {
     rv = ClassicGetWidgetBorder(aContext, aFrame, aWidgetType, aResult);
     ScaleForFrameDPI(aResult, aFrame);
     return rv;
   }
 
   aResult->top = aResult->bottom = aResult->left = aResult->right = 0;
 
   if (!WidgetIsContainer(aWidgetType) ||
@@ -1969,17 +1973,17 @@ nsNativeThemeWin::GetWidgetBorder(nsDevi
 
   if (aWidgetType == NS_THEME_TOOLBAR) {
     // make space for the separator line above all toolbars but the first
     if (state == 0)
       aResult->top = TB_SEPARATOR_HEIGHT;
     return NS_OK;
   }
 
-  rv = GetCachedWidgetBorder(aFrame, themeClass.value(), aWidgetType, part, state, aResult);
+  rv = GetCachedWidgetBorder(aFrame, theme, themeClass.value(), aWidgetType, part, state, aResult);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Remove the edges for tabs that are before or after the selected tab,
   if (aWidgetType == NS_THEME_TAB) {
     if (IsLeftToSelectedTab(aFrame))
       // Remove the right edge, since we won't be drawing it.
       aResult->right = 0;
     else if (IsRightToSelectedTab(aFrame))
@@ -2206,23 +2210,26 @@ nsNativeThemeWin::GetMinimumWidgetSize(n
                                        uint8_t aWidgetType,
                                        LayoutDeviceIntSize* aResult, bool* aIsOverridable)
 {
   aResult->width = aResult->height = 0;
   *aIsOverridable = true;
   nsresult rv = NS_OK;
 
   mozilla::Maybe<nsUXThemeClass> themeClass = GetThemeClass(aWidgetType);
-  if (themeClass.isNothing()) {
+  HTHEME theme = NULL;
+  if (!themeClass.isNothing()) {
+    theme = nsUXThemeData::GetTheme(themeClass.value());
+  }
+  if (!theme) {
     rv = ClassicGetMinimumWidgetSize(aFrame, aWidgetType, aResult, aIsOverridable);
     ScaleForFrameDPI(aResult, aFrame);
     return rv;
   }
 
-  HANDLE theme = nsUXThemeData::GetTheme(themeClass.value());
   switch (aWidgetType) {
     case NS_THEME_GROUPBOX:
     case NS_THEME_NUMBER_INPUT:
     case NS_THEME_TEXTFIELD:
     case NS_THEME_TOOLBOX:
     case NS_THEME_WIN_MEDIA_TOOLBOX:
     case NS_THEME_WIN_COMMUNICATIONS_TOOLBOX:
     case NS_THEME_WIN_BROWSERTABBAR_TOOLBOX:
--- a/widget/windows/nsNativeThemeWin.h
+++ b/widget/windows/nsNativeThemeWin.h
@@ -119,17 +119,17 @@ protected:
   RECT CalculateProgressOverlayRect(nsIFrame* aFrame, RECT* aWidgetRect,
                                     bool aIsVertical, bool aIsIndeterminate,
                                     bool aIsClassic);
   void DrawThemedProgressMeter(nsIFrame* aFrame, int aWidgetType,
                                HANDLE aTheme, HDC aHdc,
                                int aPart, int aState,
                                RECT* aWidgetRect, RECT* aClipRect);
 
-  nsresult GetCachedWidgetBorder(nsIFrame* aFrame, nsUXThemeClass aThemeClass,
+  nsresult GetCachedWidgetBorder(nsIFrame* aFrame, HANDLE aTheme, nsUXThemeClass aThemeClass,
                                  uint8_t aWidgetType, int32_t aPart, int32_t aState,
                                  nsIntMargin* aResult);
 
   nsresult GetCachedMinimumWidgetSize(nsIFrame* aFrame, HANDLE aTheme, nsUXThemeClass aThemeClass,
                                       uint8_t aWidgetType, int32_t aPart, int32_t aState,
                                       THEMESIZE aSizeReq, mozilla::LayoutDeviceIntSize* aResult);
 
 private: