MacViews: Ensure cancel buttons don't get default button styling.

ui/views/window/dialog_client_view.cc

Index: ui/views/window/dialog_client_view.cc
diff --git a/ui/views/window/dialog_client_view.cc b/ui/views/window/dialog_client_view.cc
index cf50c1c6182b6d613c33ac53ce8d0fbfa7feace7..82817517d4ee906b211c71e9a5da419330928b4c 100644
--- a/ui/views/window/dialog_client_view.cc
+++ b/ui/views/window/dialog_client_view.cc
@@ -15,6 +15,7 @@
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/button/md_text_button.h"
 #include "ui/views/layout/layout_constants.h"
+#include "ui/views/style/platform_style.h"
 #include "ui/views/views_delegate.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/dialog_delegate.h"
@@ -322,10 +323,15 @@ void DialogClientView::ChildVisibilityChanged(View* child) {
 LabelButton* DialogClientView::CreateDialogButton(ui::DialogButton type) {
   const base::string16 title = GetDialogDelegate()->GetDialogButtonLabel(type);
   LabelButton* button = nullptr;
+
+  const bool is_default =
+      GetDialogDelegate()->GetDefaultDialogButton() == type &&
+      (type != ui::DIALOG_BUTTON_CANCEL ||

[karandeepb, 2017/01/04 07:46:51]

This is similar to what we do in DialogDelegate::UpdateButton. This seems a bit
ugly to me, I think GetDefaultDialogButton should not return CANCEL button on
Mac. However, the problem I think is that GetDefaultDialogButton is virtual. 

One solution may be to have a public non-virtual
GetDefaultDialogButtonPublic(say) on DialogDelegate which delegates to a private
virtual GetDefaultDialogButton. Then GetDefaultDialogButtonPublic can take
kDialogDefaultButtonCanBeCancel into account. Alternatively, the existing
overrides can take kDialogDefaultButtonCanBeCancel into account, but that'll be
error prone.

Also, I haven't modified DialogDelegate::GetInitiallyFocusedView to take
kDialogDefaultButtonCanBeCancel into account, since that would cause no view to
be focused initially for some dialogs.

[tapted, 2017/01/04 23:11:40]

IMO the nicer fix would be to merge the Blue/Regular MD Button codepaths some
more, so that the color can be inferred from SetIsDefault(). It looks like, on
MD, the blue button is actually the same class, just with a call to
MdTextButton::SetProminent(true). It's only on non-MD that it's a different
class. So this may get a cleanup when non-MD buttons get torn out.
I think Patti's change will help move the focus along if someone isn't using
full keyboard access - hopefully we don't have to worry about this.

+       PlatformStyle::kDialogDefaultButtonCanBeCancel);
+
   // The default button is always blue in Harmony.
-  if (GetDialogDelegate()->GetDefaultDialogButton() == type &&
-      (ui::MaterialDesignController::IsSecondaryUiMaterial() ||
-       GetDialogDelegate()->ShouldDefaultButtonBeBlue())) {
+  if (is_default && (ui::MaterialDesignController::IsSecondaryUiMaterial() ||
+                     GetDialogDelegate()->ShouldDefaultButtonBeBlue())) {
     button = MdTextButton::CreateSecondaryUiBlueButton(this, title);
   } else {
     button = MdTextButton::CreateSecondaryUiButton(this, title);