[MD] Apply appropriate call to action styling to infobar buttons.

chrome/browser/ui/views/infobars/confirm_infobar.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/infobars/confirm_infobar.h"
 
 #include <utility>
 
 #include "base/logging.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/ui/views/elevation_icon_setter.h"
 #include "components/infobars/core/confirm_infobar_delegate.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/window_open_disposition.h"
 #include "ui/views/controls/button/label_button.h"
+#include "ui/views/controls/button/md_text_button.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/link.h"
 
 // InfoBarService -------------------------------------------------------------
 
 scoped_ptr<infobars::InfoBar> InfoBarService::CreateConfirmInfoBar(
     scoped_ptr<ConfirmInfoBarDelegate> delegate) {
   return make_scoped_ptr(new ConfirmInfoBar(std::move(delegate)));
 }
 
@@ skipping 37 matching lines @@
 }
 
 void ConfirmInfoBar::ViewHierarchyChanged(
     const ViewHierarchyChangedDetails& details) {
   if (details.is_add && details.child == this && (label_ == nullptr)) {
     ConfirmInfoBarDelegate* delegate = GetDelegate();
     label_ = CreateLabel(delegate->GetMessageText());
     AddViewToContentArea(label_);
 
     if (delegate->GetButtons() & ConfirmInfoBarDelegate::BUTTON_OK) {
-      ok_button_ = CreateTextButton(
-          this, delegate->GetButtonLabel(ConfirmInfoBarDelegate::BUTTON_OK));
+      if (ui::MaterialDesignController::IsModeMaterial()) {
+        views::MdTextButton* button = CreateMdTextButton(
+            this, delegate->GetButtonLabel(ConfirmInfoBarDelegate::BUTTON_OK));
+        // If this is the only button, weak call to action. Otherwise, strong
+        // call to action.
+        button->SetCallToAction(
+            delegate->GetButtons() == ConfirmInfoBarDelegate::BUTTON_OK
+                ? views::MdTextButton::WEAK_CALL_TO_ACTION
+                : views::MdTextButton::STRONG_CALL_TO_ACTION);
+        ok_button_ = button;
+      } else {
+        ok_button_ = CreateTextButton(
+            this, delegate->GetButtonLabel(ConfirmInfoBarDelegate::BUTTON_OK));
+      }

[Peter Kasting, 2016/03/25 04:57:48]

Nit: It seems unfortunate to have this mostly-duplicated block, especially
because it's easy to miss the difference in how the call to action is set
between the two buttons.

Something like this refactoring would use the same code to create both buttons. 
You could also collapse the first helper into the second helper and write things
more like they appear in the current CL, but this seemed a little cleaner.

namespace {

views::LabelButton* CreateButton(
    ConfirmInfoBarDelegate* delegate,
    ConfirmInfoBarDelegate::InfoBarButton button_type) {
  if (!ui::MaterialDesignController::IsModeMaterial())
    return CreateTextButton(this, delegate->GetButtonLabel(button_type));

  views::MdTextButton* button =
      CreateMdTextButton(this, delegate->GetButtonLabel(button_type));

  // When there are two buttons, the OK button gets a strong call to action and
  // the cancel button gets none.
  const views::MdTextButton::CallToAction two_button_cta =
      (button_type == ConfirmInfoBarDelegate::BUTTON_OK)
          ? views::MdTextButton::STRONG_CALL_TO_ACTION
          : views::MdTextButton::NO_CALL_TO_ACTION;
  button->SetCallToAction((delegate->GetButtons() == button_type)
                              ? views::MdTextButton::WEAK_CALL_TO_ACTION
                              : two_button_cta);
  return button;
}

}  // namespace

views::LabelButton* ConfirmInfoBar::CreateAndAddButton(
    ConfirmInfoBarDelegate::InfoBarButton button_type) {
  views::LabelButton* button = CreateButton(GetDelegate(), button_type);
  AddViewToContentArea(button);
  button->SizeToPreferredSize();
  return button;
}

void ConfirmInfoBar::ViewHierarchyChanged(...) {
  ...
    if (delegate->GetButtons() & ConfirmInfoBarDelegate::BUTTON_OK) {
      ok_button_ = CreateAndAddButton(ConfirmInfoBarDelegate::BUTTON_OK);
      if (delegate->OKButtonTriggersUACPrompt()) {
        ...
      }
    }
    if (delegate->GetButtons() & ConfirmInfoBarDelegate::BUTTON_CANCEL) {
      cancel_button_ =
          CreateAndAddButton(ConfirmInfoBarDelegate::BUTTON_CANCEL);
    }

[Evan Stade, 2016/03/28 19:13:43]

I don't see this as a huge win because I don't see that much duplication. The
additional ternary is not really de-duping anything. I take consolation from the
thought of how this code will look like when the non-material path is gone (no
need for temporary button var for example).

       if (delegate->OKButtonTriggersUACPrompt()) {
         elevation_icon_setter_.reset(new ElevationIconSetter(
             ok_button_,
             base::Bind(&ConfirmInfoBar::Layout, base::Unretained(this))));
       }
       AddViewToContentArea(ok_button_);
       ok_button_->SizeToPreferredSize();
     }
 
     if (delegate->GetButtons() & ConfirmInfoBarDelegate::BUTTON_CANCEL) {
-      cancel_button_ = CreateTextButton(
-          this,
-          delegate->GetButtonLabel(ConfirmInfoBarDelegate::BUTTON_CANCEL));
+      if (ui::MaterialDesignController::IsModeMaterial()) {
+        views::MdTextButton* button = CreateMdTextButton(
+            this,
+            delegate->GetButtonLabel(ConfirmInfoBarDelegate::BUTTON_CANCEL));
+        // If this is the only button, weak call to action. Otherwise, no call
+        // to action.
+        button->SetCallToAction(
+            delegate->GetButtons() == ConfirmInfoBarDelegate::BUTTON_CANCEL
+                ? views::MdTextButton::WEAK_CALL_TO_ACTION
+                : views::MdTextButton::NO_CALL_TO_ACTION);
+        cancel_button_ = button;
+      } else {
+        cancel_button_ = CreateTextButton(
+            this,
+            delegate->GetButtonLabel(ConfirmInfoBarDelegate::BUTTON_CANCEL));
+      }
       AddViewToContentArea(cancel_button_);
       cancel_button_->SizeToPreferredSize();
     }
 
     base::string16 link_text(delegate->GetLinkText());
     link_ = CreateLink(link_text, this);
     AddViewToContentArea(link_);
   }
 
   // This must happen after adding all other children so InfoBarView can ensure
@@ skipping 35 matching lines @@
 }
 
 int ConfirmInfoBar::NonLabelWidth() const {
   int width = (label_->text().empty() || (!ok_button_ && !cancel_button_)) ?
       0 : kEndOfLabelSpacing;
   if (ok_button_)
     width += ok_button_->width() + (cancel_button_ ? kButtonButtonSpacing : 0);
   width += cancel_button_ ? cancel_button_->width() : 0;
   return width + ((link_->text().empty() || !width) ? 0 : kEndOfLabelSpacing);
 }