Use the default theme as the fallback UI in Blink

Use the default theme as the fallback UI in Blink

Most native themes don't support zooming or styling required by Blink.
This introduces a FallbackTheme that can be used in these situations.

Blink side change: https://codereview.chromium.org/14108007/

BUG=84973
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=200431

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=200785

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=201085

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=201390

[keishi, 2013-04-23 12:27:22]

sky@ could you review the changes to ui/native_theme
jamesr@ could you review the changes to webkit/glue

Thanks

[sky, 2013-04-23 15:23:43]

From what I remember (and I could be wrong here since I haven't closely followed
the blink side) the default theme in blink means aura. This change here makes
the default theme=gtk. That strikes me as confusing. It's my understanding that
long term we want to use the aura theme every where, which implies default in
chrome should = aura. Is there a reason you're going with gtk as the default and
not aura?

[jamesr, 2013-04-23 17:17:53]

What's the motivation for this change?  There's no bug linked.

[keishi, 2013-04-24 14:50:54]

On 2013/04/23 15:23:43, sky wrote:
> From what I remember (and I could be wrong here since I haven't closely
followed
> the blink side) the default theme in blink means aura. This change here makes
> the default theme=gtk. That strikes me as confusing. It's my understanding
that
> long term we want to use the aura theme every where, which implies default in
> chrome should = aura. Is there a reason you're going with gtk as the default
and
> not aura?

In patch set 2 the fallback theme is based on aura. I don't need the scrollbars
in the fallback theme.

I was going to use aura but I went with gtk because it looked like it was using
extra resource files and gtk was closer to base.

[sky, 2013-04-24 14:56:06]

I'm curious for the answer to James's question as well. Additionally
when would you use the fallback? If its under a zoom won't it look
weird when a bunch of theme elements potentially change with the zoom?

On Wed, Apr 24, 2013 at 7:50 AM,  <keishi@chromium.org> wrote:
> On 2013/04/23 15:23:43, sky wrote:
>>
>>  From what I remember (and I could be wrong here since I haven't closely
>
> followed
>>
>> the blink side) the default theme in blink means aura. This change here
>> makes
>> the default theme=gtk. That strikes me as confusing. It's my understanding
>
> that
>>
>> long term we want to use the aura theme every where, which implies default
>> in
>> chrome should = aura. Is there a reason you're going with gtk as the
>> default
>
> and
>>
>> not aura?
>
>
> In patch set 2 the fallback theme is based on aura. I don't need the
> scrollbars
> in the fallback theme.
>
> I was going to use aura but I went with gtk because it looked like it was
> using
> extra resource files and gtk was closer to base.
>
> https://codereview.chromium.org/14424007/

[keishi, 2013-04-24 14:57:14]

On 2013/04/23 17:17:53, jamesr wrote:
> What's the motivation for this change?  There's no bug linked.

Sorry, I've added a bug id. Right now checkboxes and radio buttons don't get
scaled when zooming the web page. And I want to fix that.

Native widgets don't support zooming so we want to fallback to the default
theme.

I realized that the themes for gtk and aura are different, so for patch set 2 I
am introducing WebFallbackThemeEngine which is separate from
public/default/WebThemeEngine.h

[keishi, 2013-04-24 16:24:47]

On 2013/04/24 14:56:06, sky wrote:
> I'm curious for the answer to James's question as well. Additionally
> when would you use the fallback? If its under a zoom won't it look
> weird when a bunch of theme elements potentially change with the zoom?
The primary objective is to enable zooming of checkboxes and radio buttons.
But we also think we can use the fallback theme to fix issues like
https://code.google.com/p/chromium/issues/detail?id=138670 where the Windows
checkbox can't be drawn on a translucent layer.
The appearance will change when you zoom but I don't think it will be that bad.
Reimplementing the native UI drawing would be a lot of work, is never the same
and requires maintenance on OS updates.

[sky, 2013-04-24 17:11:39]

Please run this by the UX folks.

On Wed, Apr 24, 2013 at 9:24 AM,  <keishi@chromium.org> wrote:
> On 2013/04/24 14:56:06, sky wrote:
>>
>> I'm curious for the answer to James's question as well. Additionally
>> when would you use the fallback? If its under a zoom won't it look
>> weird when a bunch of theme elements potentially change with the zoom?
>
> The primary objective is to enable zooming of checkboxes and radio buttons.
> But we also think we can use the fallback theme to fix issues like
> https://code.google.com/p/chromium/issues/detail?id=138670 where the Windows
> checkbox can't be drawn on a translucent layer.
> The appearance will change when you zoom but I don't think it will be that
> bad.
> Reimplementing the native UI drawing would be a lot of work, is never the
> same
> and requires maintenance on OS updates.
>
> https://codereview.chromium.org/14424007/

[keishi, 2013-04-26 09:23:09]

On 2013/04/24 17:11:39, sky wrote:
> Please run this by the UX folks.
I've asked chrome-ux and got the LGTM from alcor@ and sgabriel@. (sky@, I've
CCed you so it should be in your inbox)

[sky, 2013-04-26 16:13:50]

https://codereview.chromium.org/14424007/diff/5001/ui/native_theme/fallback_t...
File ui/native_theme/fallback_theme.cc (right):

https://codereview.chromium.org/14424007/diff/5001/ui/native_theme/fallback_t...
ui/native_theme/fallback_theme.cc:80: 
Only one new line.

https://codereview.chromium.org/14424007/diff/5001/ui/native_theme/fallback_t...
File ui/native_theme/fallback_theme.h (right):

https://codereview.chromium.org/14424007/diff/5001/ui/native_theme/fallback_t...
ui/native_theme/fallback_theme.h:13: // is missing from the native theme.
Please indicate this is only used when scaling and the native theme doesn't
support the scale.

https://codereview.chromium.org/14424007/diff/5001/ui/native_theme/native_the...
File ui/native_theme/native_theme_aura.cc (left):

https://codereview.chromium.org/14424007/diff/5001/ui/native_theme/native_the...
ui/native_theme/native_theme_aura.cc:22: // Theme colors returned by
GetSystemColor().
AFIACT you're moving these to FallbackTheme. The code in chrome uses
NativeThemeAura though. Won't that mean chrome code isn't going to look right at
all?

[jamesr, 2013-04-26 18:08:21]

webkit/glue/ lgtm once sky's happy with the ui/ changes

[keishi, 2013-04-30 08:16:58]

https://codereview.chromium.org/14424007/diff/5001/ui/native_theme/fallback_t...
File ui/native_theme/fallback_theme.cc (right):

https://codereview.chromium.org/14424007/diff/5001/ui/native_theme/fallback_t...
ui/native_theme/fallback_theme.cc:80: 
On 2013/04/26 16:13:50, sky wrote:
> Only one new line.

Done.

https://codereview.chromium.org/14424007/diff/5001/ui/native_theme/fallback_t...
File ui/native_theme/fallback_theme.h (right):

https://codereview.chromium.org/14424007/diff/5001/ui/native_theme/fallback_t...
ui/native_theme/fallback_theme.h:13: // is missing from the native theme.
On 2013/04/26 16:13:50, sky wrote:
> Please indicate this is only used when scaling and the native theme doesn't
> support the scale.

Done.

https://codereview.chromium.org/14424007/diff/5001/ui/native_theme/native_the...
File ui/native_theme/native_theme_aura.cc (left):

https://codereview.chromium.org/14424007/diff/5001/ui/native_theme/native_the...
ui/native_theme/native_theme_aura.cc:22: // Theme colors returned by
GetSystemColor().
On 2013/04/26 16:13:50, sky wrote:
> AFIACT you're moving these to FallbackTheme. The code in chrome uses
> NativeThemeAura though. Won't that mean chrome code isn't going to look right
at
> all?
I meant to make NativeTheme for Aura inherit FallbackTheme but I forgot.

[keishi, 2013-05-09 12:55:46]

sky@ could you take a look again? Thanks.

[sky, 2013-05-09 16:11:35]

LGTM

https://codereview.chromium.org/14424007/diff/18001/ui/native_theme/fallback_...
File ui/native_theme/fallback_theme.cc (right):

https://codereview.chromium.org/14424007/diff/18001/ui/native_theme/fallback_...
ui/native_theme/fallback_theme.cc:80: SkColor
FallbackTheme::GetSystemColor(ColorId color_id) const {
Make order match header (this should be last).

[keishi, 2013-05-10 02:25:54]

On 2013/05/09 16:11:35, sky wrote:
> LGTM
> 
>
https://codereview.chromium.org/14424007/diff/18001/ui/native_theme/fallback_...
> File ui/native_theme/fallback_theme.cc (right):
> 
>
https://codereview.chromium.org/14424007/diff/18001/ui/native_theme/fallback_...
> ui/native_theme/fallback_theme.cc:80: SkColor
> FallbackTheme::GetSystemColor(ColorId color_id) const {
> Make order match header (this should be last).

Done.
Thanks!

[commit-bot: I haz the power, 2013-05-15 10:48:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/keishi@chromium.org/14424007/31001

[commit-bot: I haz the power, 2013-05-15 11:07:33]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-05-16 00:28:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/keishi@chromium.org/14424007/31001

[commit-bot: I haz the power, 2013-05-16 03:30:53]

Change committed as 200431

[commit-bot: I haz the power, 2013-05-17 03:51:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/keishi@chromium.org/14424007/18002

[commit-bot: I haz the power, 2013-05-17 03:51:35]

Failed to apply patch for ui/native_theme/native_theme_aura.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file ui/native_theme/native_theme_aura.cc
  Hunk #1 FAILED at 19.
  Hunk #2 FAILED at 95.
  2 out of 2 hunks FAILED -- saving rejects to file
ui/native_theme/native_theme_aura.cc.rej

Patch:       ui/native_theme/native_theme_aura.cc
Index: ui/native_theme/native_theme_aura.cc
diff --git a/ui/native_theme/native_theme_aura.cc
b/ui/native_theme/native_theme_aura.cc
index
3e597b5792c3e14b9000cf3c4dce6f9f35ea9648..6cbc2554031ba81ce38abe8ef6eae476838e3b4a
100644
--- a/ui/native_theme/native_theme_aura.cc
+++ b/ui/native_theme/native_theme_aura.cc
@@ -19,59 +19,6 @@ namespace {
 
 const SkColor kMenuBackgroundColor = SK_ColorWHITE;
 
-// Theme colors returned by GetSystemColor().
-const SkColor kInvalidColorIdColor = SkColorSetRGB(255, 0, 128);
-// Windows:
-const SkColor kWindowBackgroundColor = SK_ColorWHITE;
-// Dialogs:
-const SkColor kDialogBackgroundColor = SkColorSetRGB(251, 251, 251);
-// FocusableBorder:
-const SkColor kFocusedBorderColor = SkColorSetRGB(0x4D, 0x90, 0xFE);
-const SkColor kUnfocusedBorderColor = SkColorSetRGB(0xD9, 0xD9, 0xD9);
-// TextButton:
-const SkColor kTextButtonBackgroundColor = SkColorSetRGB(0xDE, 0xDE, 0xDE);
-const SkColor kTextButtonEnabledColor = SkColorSetRGB(0x22, 0x22, 0x22);
-const SkColor kTextButtonDisabledColor = SkColorSetRGB(0x99, 0x99, 0x99);
-const SkColor kTextButtonHighlightColor = SkColorSetRGB(0, 0, 0);
-const SkColor kTextButtonHoverColor = kTextButtonEnabledColor;
-// MenuItem:
-const SkColor kEnabledMenuItemForegroundColor = kTextButtonEnabledColor;
-const SkColor kDisabledMenuItemForegroundColor = kTextButtonDisabledColor;
-const SkColor kFocusedMenuItemBackgroundColor = SkColorSetRGB(0xF1, 0xF1,
0xF1);
-const SkColor kHoverMenuItemBackgroundColor =
-    SkColorSetARGB(204, 255, 255, 255);
-const SkColor kMenuSeparatorColor = SkColorSetRGB(0xED, 0xED, 0xED);
-const SkColor kEnabledMenuButtonBorderColor = SkColorSetARGB(36, 0, 0, 0);
-const SkColor kFocusedMenuButtonBorderColor = SkColorSetARGB(72, 0, 0, 0);
-const SkColor kHoverMenuButtonBorderColor = SkColorSetARGB(72, 0, 0, 0);
-// Label:
-const SkColor kLabelEnabledColor = kTextButtonEnabledColor;
-const SkColor kLabelDisabledColor = kTextButtonDisabledColor;
-const SkColor kLabelBackgroundColor = SK_ColorWHITE;
-// Textfield:
-const SkColor kTextfieldDefaultColor = SK_ColorBLACK;
-const SkColor kTextfieldDefaultBackground = SK_ColorWHITE;
-const SkColor kTextfieldReadOnlyColor = SK_ColorDKGRAY;
-const SkColor kTextfieldReadOnlyBackground = SK_ColorWHITE;
-const SkColor kTextfieldSelectionBackgroundFocused =
-    SkColorSetARGB(0x54, 0x60, 0xA8, 0xEB);
-const SkColor kTextfieldSelectionBackgroundUnfocused = SK_ColorLTGRAY;
-const SkColor kTextfieldSelectionColor =
-    color_utils::AlphaBlend(SK_ColorBLACK,
-        kTextfieldSelectionBackgroundFocused, 0xdd);
-// Tree
-const SkColor kTreeBackground = SK_ColorWHITE;
-const SkColor kTreeTextColor = SK_ColorBLACK;
-const SkColor kTreeSelectedTextColor = SK_ColorBLACK;
-const SkColor kTreeSelectionBackgroundColor = SkColorSetRGB(0xEE, 0xEE, 0xEE);
-const SkColor kTreeArrowColor = SkColorSetRGB(0x7A, 0x7A, 0x7A);
-// Table
-const SkColor kTableBackground = SK_ColorWHITE;
-const SkColor kTableTextColor = SK_ColorBLACK;
-const SkColor kTableSelectedTextColor = SK_ColorBLACK;
-const SkColor kTableSelectionBackgroundColor = SkColorSetRGB(0xEE, 0xEE, 0xEE);
-const SkColor kTableGroupingIndicatorColor = SkColorSetRGB(0xCC, 0xCC, 0xCC);
-
 }  // namespace
 
 namespace ui {
@@ -95,121 +42,6 @@ NativeThemeAura::NativeThemeAura() {
 NativeThemeAura::~NativeThemeAura() {
 }
 
-SkColor NativeThemeAura::GetSystemColor(ColorId color_id) const {
-  // This implementation returns hardcoded colors.
-  SkColor color;
-  if (CommonThemeGetSystemColor(color_id, &color))
-    return color;
-
-  switch (color_id) {
-    // Windows
-    case kColorId_WindowBackground:
-      return kWindowBackgroundColor;
-
-    // Dialogs
-    case kColorId_DialogBackground:
-      return kDialogBackgroundColor;
-
-    // FocusableBorder
-    case kColorId_FocusedBorderColor:
-      return kFocusedBorderColor;
-    case kColorId_UnfocusedBorderColor:
-      return kUnfocusedBorderColor;
-
-    // TextButton
-    case kColorId_TextButtonBackgroundColor:
-      return kTextButtonBackgroundColor;
-    case kColorId_TextButtonEnabledColor:
-      return kTextButtonEnabledColor;
-    case kColorId_TextButtonDisabledColor:
-      return kTextButtonDisabledColor;
-    case kColorId_TextButtonHighlightColor:
-      return kTextButtonHighlightColor;
-    case kColorId_TextButtonHoverColor:
-      return kTextButtonHoverColor;
-
-    // MenuItem
-    case kColorId_EnabledMenuItemForegroundColor:
-      return kEnabledMenuItemForegroundColor;
-    case kColorId_DisabledMenuItemForegroundColor:
-      return kDisabledMenuItemForegroundColor;
-    case kColorId_SelectedMenuItemForegroundColor:
-      return kEnabledMenuItemForegroundColor;
-    case kColorId_FocusedMenuItemBackgroundColor:
-      return kFocusedMenuItemBackgroundColor;
-    case kColorId_HoverMenuItemBackgroundColor:
-      return kHoverMenuItemBackgroundColor;
-    case kColorId_MenuSeparatorColor:
-      return kMenuSeparatorColor;
-    case kColorId_EnabledMenuButtonBorderColor:
-      return kEnabledMenuButtonBorderColor;
-    case kColorId_FocusedMenuButtonBorderColor:
-      return kFocusedMenuButtonBorderColor;
-    case kColorId_HoverMenuButtonBorderColor:
-      return kHoverMenuButtonBorderColor;
-
-    // Label
-    case kColorId_LabelEnabledColor:
-      return kLabelEnabledColor;
-    case kColorId_LabelDisabledColor:
-      return kLabelDisabledColor;
-    case kColorId_LabelBackgroundColor:
-      return kLabelBackgroundColor;
-
-    // Textfield
-    case kColorId_TextfieldDefaultColor:
-      return kTextfieldDefaultColor;
-    case kColorId_TextfieldDefaultBackground:
-      return kTextfieldDefaultBackground;
-    case kColorId_TextfieldReadOnlyColor:
-      return kTextfieldReadOnlyColor;
-    case kColorId_TextfieldReadOnlyBackground:
-      return kTextfieldReadOnlyBackground;
-    case kColorId_TextfieldSelectionColor:
-      return kTextfieldSelectionColor;
-    case kColorId_TextfieldSelectionBackgroundFocused:
-      return kTextfieldSelectionBackgroundFocused;
-    case kColorId_TextfieldSelectionBackgroundUnfocused:
-      return kTextfieldSelectionBackgroundUnfocused;
-
-    // Tree
-    case kColorId_TreeBackground:
-      return kTreeBackground;
-    case kColorId_TreeText:
-      return kTreeTextColor;
-    case kColorId_TreeSelectedText:
-    case kColorId_TreeSelectedTextUnfocused:
-      return kTreeSelectedTextColor;
-    case kColorId_TreeSelectionBackgroundFocused:
-    case kColorId_TreeSelectionBackgroundUnfocused:
-      return kTreeSelectionBackgroundColor;
-    case kColorId_TreeArrow:
-      return kTreeArrowColor;
-
-    // Table
-    case kColorId_TableBackground:
-      return kTableBackground;
-    case kColorId_TableText:
-      return kTableTextColor;
-    case kColorId_TableSelectedText:
-    case kColorId_TableSelectedTextUnfocused:
-      return kTableSelectedTextColor;
-    case kColorId_TableSelectionBackgroundFocused:
-    case kColorId_TableSelectionBackgroundUnfocused:
-      return kTableSelectionBackgroundColor;
-    case kColorId_TableGroupingIndicatorColor:
-      return kTableGroupingIndicatorColor;
-
-    case kColorId_MenuBackgroundColor:
-      return kMenuBackgroundColor;
-    case kColorId_MenuBorderColor:
-      NOTREACHED();
-      break;
-  }
-
-  return kInvalidColorIdColor;
-}
-
 void NativeThemeAura::PaintMenuPopupBackground(
     SkCanvas* canvas,
     const gfx::Size& size,

[commit-bot: I haz the power, 2013-05-17 05:46:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/keishi@chromium.org/14424007/56001

[commit-bot: I haz the power, 2013-05-17 08:53:00]

Retried try job too often on win_rel for step(s) content_browsertests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-05-17 09:10:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/keishi@chromium.org/14424007/56001

[commit-bot: I haz the power, 2013-05-17 11:27:51]

Change committed as 200785

[commit-bot: I haz the power, 2013-05-20 06:50:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/keishi@chromium.org/14424007/77001

[commit-bot: I haz the power, 2013-05-20 13:53:56]

Retried try job too often on win_rel for step(s) base_unittests, browser_tests,
cacheinvalidation_unittests, cc_unittests, check_deps, chrome_frame_net_tests,
chrome_frame_tests, chrome_frame_unittests, chromedriver2_unittests,
components_unittests, content_browsertests, content_unittests, crypto_unittests,
gpu_unittests, installer_util_unittests, ipc_tests, jingle_unittests,
media_unittests, mini_installer_test, nacl_integration, net_unittests,
ppapi_unittests, printing_unittests, remoting_unittests, sql_unittests,
sync_integration_tests, sync_unit_tests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-05-20 14:14:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/keishi@chromium.org/14424007/77001

[commit-bot: I haz the power, 2013-05-20 14:35:13]

Change committed as 201085

[commit-bot: I haz the power, 2013-05-21 14:15:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/keishi@chromium.org/14424007/105001

[commit-bot: I haz the power, 2013-05-21 16:33:42]

Retried try job too often on win_rel for step(s) browser_tests,
chrome_frame_net_tests, chrome_frame_tests, chrome_frame_unittests,
content_browsertests, installer_util_unittests, mini_installer_test,
nacl_integration, sync_integration_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-05-21 16:48:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/keishi@chromium.org/14424007/105001

[commit-bot: I haz the power, 2013-05-22 00:02:09]

Change committed as 201390