Implement the 2016Q2 Translate UI designe spec out in

chrome/browser/ui/views/translate/translate_bubble_view.cc

Index: chrome/browser/ui/views/translate/translate_bubble_view.cc
diff --git a/chrome/browser/ui/views/translate/translate_bubble_view.cc b/chrome/browser/ui/views/translate/translate_bubble_view.cc
index 13af4c444318e67b7c02cb8b4ac6f0d87334990d..f4b559b67f6f5b0b54b41ef6e8be05e9fd275042 100644
--- a/chrome/browser/ui/views/translate/translate_bubble_view.cc
+++ b/chrome/browser/ui/views/translate/translate_bubble_view.cc
@@ -25,17 +25,23 @@
 #include "components/prefs/pref_service.h"
 #include "components/translate/core/browser/translate_download_manager.h"
 #include "components/translate/core/browser/translate_manager.h"
+#include "components/translate/core/browser/translate_prefs.h"
 #include "components/translate/core/browser/translate_ui_delegate.h"
 #include "content/public/browser/web_contents.h"
 #include "grit/components_strings.h"
+#include "grit/ui_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/models/combobox_model.h"
 #include "ui/base/models/simple_combobox_model.h"
+#include "ui/base/resource/resource_bundle.h"
+#include "ui/views/bubble/bubble_frame_view.h"
+#include "ui/views/controls/button/blue_button.h"
 #include "ui/views/controls/button/checkbox.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/combobox/combobox.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/link.h"
+#include "ui/views/controls/styled_label.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/grid_layout.h"
 #include "ui/views/layout/layout_constants.h"
@@ -43,6 +49,36 @@
 
 namespace {
 
+const int kQuestionWidth = 200;
+const char kTranslateUIAction[] = "Translate.UIAction";
+enum UIActionType {

[groby-ooo-7-16, 2016/04/27 19:08:24]

Please use TranslateBubbleUiEvent and expand that. Translate already has a lot
of histograms, and each new one comes at a cost.

[ftang, 2016/04/27 21:43:15]

ok.

+  UI_ACTION_ADVANCED_LINK_CLICKED,
+  UI_ACTION_ALWAYS_TRANSLATE_CHECKED,
+  UI_ACTION_ALWAYS_TRANSLATE_UNCHECKED,
+  UI_ACTION_NOPE_MENU_CLICKED,
+  UI_ACTION_NEVER_TRANSLATE_LANGUAGE_MENU_CLICKED,
+  UI_ACTION_NEVER_TRANSLATE_SITE_MENU_CLICKED,
+  UI_ACTION_TRANSLATE_BUTTON_CLICKED,
+  UI_ACTION_DONE_BUTTON_CLICKED,
+  UI_ACTION_CANCEL_BUTTON_CLICKED,
+  UI_ACTION_TRY_AGAIN_BUTTON_CLICKED,
+  UI_ACTION_SHOW_ORIGINAL_BUTTON_CLICKED,
+  UI_ACTION_SETTING_LINK_CLICKED,
+  UI_ACTION_SOURCE_LANGUAGE_MENU_CLICKED,
+  UI_ACTION_TARGET_LANGUAGE_MENU_CLICKED,
+
+  // Insert new items here.
+  UI_ACTION_MAX,
+};
+
+views::LabelButton* CreateBlueButton(views::ButtonListener* listener,
+                                      const base::string16& label,
+                                      int id) {
+  views::LabelButton* button = new views::BlueButton(listener, label);
+  button->set_id(id);
+  return button;
+}
+
 views::LabelButton* CreateLabelButton(views::ButtonListener* listener,
                                       const base::string16& label,
                                       int id) {
@@ -53,14 +89,20 @@ views::LabelButton* CreateLabelButton(views::ButtonListener* listener,
 }
 
 views::Link* CreateLink(views::LinkListener* listener,
-                        int resource_id,
+                        base::string16 text,
                         int id) {
-  views::Link* link = new views::Link(
-      l10n_util::GetStringUTF16(resource_id));
+  views::Link* link = new views::Link(text);
   link->set_listener(listener);
   link->set_id(id);
   return link;
 }
+views::Link* CreateLink(views::LinkListener* listener,
+                        int resource_id,
+                        int id) {
+  return CreateLink(listener,
+                    l10n_util::GetStringUTF16(resource_id),
+                    id);
+}
 
 }  // namespace
 
@@ -160,12 +202,19 @@ void TranslateBubbleView::Init() {
     model_->ShowError(error_type_);
 }
 
+bool TranslateBubbleView::ShouldShowCloseButton() const {
+  return use_2016_q2_ui_;
+}
+

[groby-ooo-7-16, 2016/04/27 19:08:24]

I don't understand why you chose to encapsulate this one, but use
|use_blue_button_|, |show_always_checkbox_|, and |show_icon_| as bare members.

Also, since they never change value, can we get rid of them?

[ftang, 2016/04/27 21:43:15]

Done.

 void TranslateBubbleView::ButtonPressed(views::Button* sender,
                                         const ui::Event& event) {
   HandleButtonPressed(static_cast<ButtonID>(sender->id()));
 }
 
 void TranslateBubbleView::WindowClosing() {
+  if (GetBubbleFrameView()->close_button_clicked()) {
+    model_->DeclineTranslation();

[groby-ooo-7-16, 2016/04/27 19:08:25]

This is invoked when the widget is closed for _any_ reason, IIRC. That does
include dismissal by focus loss, so probably this is a mistake.
 
Can we extract all this logic into a controller class that's actually testable
for proper behavior?

[ftang, 2016/04/27 21:43:15]

Notice this is wrap inside "if (GetBubbleFrameView()->close_button_clicked())"
which only happen when it is closed due to the close button clicked.

[groby-ooo-7-16, 2016/04/29 05:26:14]

I suppose so. I'd rather call DeclineTranslation in the handler for the close
button, but since that's in the BubbleFrameView itself... I guess I'm OK with
it.

+  }
   // The operations for |model_| are valid only when a WebContents is alive.
   // TODO(hajimehoshi): TranslateBubbleViewModel(Impl) should not hold a
   // WebContents as a member variable because the WebContents might be destroyed
@@ -232,6 +281,15 @@ void TranslateBubbleView::LinkClicked(views::Link* source, int event_flags) {
   HandleLinkClicked(static_cast<LinkID>(source->id()));
 }
 
+void TranslateBubbleView::StyledLabelLinkClicked(views::StyledLabel* label,
+                                                 const gfx::Range& range,
+                                                 int event_flags) {
+  SwitchView(TranslateBubbleModel::VIEW_STATE_ADVANCED);
+  UMA_HISTOGRAM_ENUMERATION(kTranslateUIAction,
+                            UI_ACTION_ADVANCED_LINK_CLICKED,
+                            UI_ACTION_MAX);
+}
+
 void TranslateBubbleView::WebContentsDestroyed() {
   GetWidget()->CloseNow();
 }
@@ -261,7 +319,12 @@ TranslateBubbleView::TranslateBubbleView(
       model_(std::move(model)),
       error_type_(error_type),
       is_in_incognito_window_(
-          web_contents && web_contents->GetBrowserContext()->IsOffTheRecord()) {
+          web_contents && web_contents->GetBrowserContext()->IsOffTheRecord()),
+      use_2016_q2_ui_(base::FeatureList::IsEnabled(
+                          translate::kTranslateUI2016Q2)),
+      show_icon_(use_2016_q2_ui_),
+      show_always_checkbox_(use_2016_q2_ui_),
+      use_blue_button_(use_2016_q2_ui_) {
   translate_bubble_view_ = this;
 }
 
@@ -287,6 +350,9 @@ void TranslateBubbleView::HandleButtonPressed(
   switch (sender_id) {
     case BUTTON_ID_TRANSLATE: {
       model_->Translate();
+      UMA_HISTOGRAM_ENUMERATION(kTranslateUIAction,

[groby-ooo-7-16, 2016/04/27 19:08:25]

I'd suggest factoring this out into a ReportUiAction() function. (Ideally, on
the controller mentioned above :)

[ftang, 2016/04/27 21:43:15]

Done.

+                                UI_ACTION_TRANSLATE_BUTTON_CLICKED,
+                                UI_ACTION_MAX);
       break;
     }
     case BUTTON_ID_DONE: {
@@ -300,26 +366,43 @@ void TranslateBubbleView::HandleButtonPressed(
         model_->Translate();
         SwitchView(TranslateBubbleModel::VIEW_STATE_TRANSLATING);
       }
+      UMA_HISTOGRAM_ENUMERATION(kTranslateUIAction,
+                                UI_ACTION_DONE_BUTTON_CLICKED,
+                                UI_ACTION_MAX);
       break;
     }
     case BUTTON_ID_CANCEL: {
       model_->GoBackFromAdvanced();
       UpdateChildVisibilities();
       SizeToContents();
+      UMA_HISTOGRAM_ENUMERATION(kTranslateUIAction,
+                                UI_ACTION_CANCEL_BUTTON_CLICKED,
+                                UI_ACTION_MAX);
       break;
     }
     case BUTTON_ID_TRY_AGAIN: {
       model_->Translate();
+      UMA_HISTOGRAM_ENUMERATION(kTranslateUIAction,
+                                UI_ACTION_TRY_AGAIN_BUTTON_CLICKED,
+                                UI_ACTION_MAX);
       break;
     }
     case BUTTON_ID_SHOW_ORIGINAL: {
       model_->RevertTranslation();
       GetWidget()->Close();
+      UMA_HISTOGRAM_ENUMERATION(kTranslateUIAction,
+                                UI_ACTION_SHOW_ORIGINAL_BUTTON_CLICKED,
+                                UI_ACTION_MAX);
       break;
     }
     case BUTTON_ID_ALWAYS_TRANSLATE: {
       // Do nothing. The state of the checkbox affects only when the 'Done'
       // button is pressed.
+      UMA_HISTOGRAM_ENUMERATION(kTranslateUIAction,
+                                (always_translate_checkbox_->checked() ?
+                                 UI_ACTION_ALWAYS_TRANSLATE_CHECKED :
+                                 UI_ACTION_ALWAYS_TRANSLATE_UNCHECKED),
+                                UI_ACTION_MAX);
       break;
     }
   }
@@ -330,6 +413,9 @@ void TranslateBubbleView::HandleLinkClicked(
   switch (sender_id) {
     case LINK_ID_ADVANCED: {
       SwitchView(TranslateBubbleModel::VIEW_STATE_ADVANCED);
+      UMA_HISTOGRAM_ENUMERATION(kTranslateUIAction,
+                                UI_ACTION_ADVANCED_LINK_CLICKED,
+                                UI_ACTION_MAX);
       break;
     }
     case LINK_ID_LANGUAGE_SETTINGS: {
@@ -340,6 +426,9 @@ void TranslateBubbleView::HandleLinkClicked(
           NEW_FOREGROUND_TAB,
           ui::PAGE_TRANSITION_LINK,
           false));
+      UMA_HISTOGRAM_ENUMERATION(kTranslateUIAction,
+                                UI_ACTION_SETTING_LINK_CLICKED,
+                                UI_ACTION_MAX);
       break;
     }
   }
@@ -349,23 +438,42 @@ void TranslateBubbleView::HandleComboboxPerformAction(
     TranslateBubbleView::ComboboxID sender_id) {
   switch (sender_id) {
     case COMBOBOX_ID_DENIAL: {
-      model_->DeclineTranslation();
-      DenialComboboxIndex index =
-          static_cast<DenialComboboxIndex>(denial_combobox_->selected_index());
+      DenialComboboxIndex index = static_cast<DenialComboboxIndex>(
+          denial_combobox_->selected_index());
       switch (index) {
         case DenialComboboxIndex::DONT_TRANSLATE:
+          if (use_2016_q2_ui_) {
+            // TODO(ftang) find someway to drop down the menu.
+          } else {
+            model_->DeclineTranslation();

[groby-ooo-7-16, 2016/04/27 19:08:24]

Why is this common code factored out into every single branch?

[ftang, 2016/04/27 21:43:15]

Done.

+            GetWidget()->Close();
+            UMA_HISTOGRAM_ENUMERATION(kTranslateUIAction,
+                                      UI_ACTION_NOPE_MENU_CLICKED,
+                                      UI_ACTION_MAX);
+          }
           break;
         case DenialComboboxIndex::NEVER_TRANSLATE_LANGUAGE:
+          model_->DeclineTranslation();
           model_->SetNeverTranslateLanguage(true);
+          GetWidget()->Close();
+          UMA_HISTOGRAM_ENUMERATION(
+              kTranslateUIAction,
+              UI_ACTION_NEVER_TRANSLATE_LANGUAGE_MENU_CLICKED,
+              UI_ACTION_MAX);
           break;
         case DenialComboboxIndex::NEVER_TRANSLATE_SITE:
+          model_->DeclineTranslation();
           model_->SetNeverTranslateSite(true);
+          GetWidget()->Close();
+          UMA_HISTOGRAM_ENUMERATION(
+              kTranslateUIAction,
+              UI_ACTION_NEVER_TRANSLATE_SITE_MENU_CLICKED,
+              UI_ACTION_MAX);
           break;
         default:
           NOTREACHED();
           break;
       }
-      GetWidget()->Close();
       break;
     }
     case COMBOBOX_ID_SOURCE_LANGUAGE: {
@@ -376,6 +484,9 @@ void TranslateBubbleView::HandleComboboxPerformAction(
       model_->UpdateOriginalLanguageIndex(
           source_language_combobox_->selected_index());
       UpdateAdvancedView();
+      UMA_HISTOGRAM_ENUMERATION(kTranslateUIAction,
+                                UI_ACTION_SOURCE_LANGUAGE_MENU_CLICKED,
+                                UI_ACTION_MAX);
       break;
     }
     case COMBOBOX_ID_TARGET_LANGUAGE: {
@@ -386,6 +497,9 @@ void TranslateBubbleView::HandleComboboxPerformAction(
       model_->UpdateTargetLanguageIndex(
           target_language_combobox_->selected_index());
       UpdateAdvancedView();
+      UMA_HISTOGRAM_ENUMERATION(kTranslateUIAction,
+                                UI_ACTION_TARGET_LANGUAGE_MENU_CLICKED,
+                                UI_ACTION_MAX);
       break;
     }
   }
@@ -399,28 +513,74 @@ void TranslateBubbleView::UpdateChildVisibilities() {
 }
 
 views::View* TranslateBubbleView::CreateViewBeforeTranslate() {
+  base::string16 original_language_name =
+      model_->GetLanguageNameAt(model_->GetOriginalLanguageIndex());
+  base::string16 target_language_name =
+      model_->GetLanguageNameAt(model_->GetTargetLanguageIndex());
+
   views::Label* message_label = new views::Label(
       l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_BEFORE_TRANSLATE));
 
-  base::string16 original_language_name =
-      model_->GetLanguageNameAt(model_->GetOriginalLanguageIndex());
+  std::vector<size_t> offsets;
+  views::StyledLabel* styled_label = new views::StyledLabel(
+      l10n_util::GetStringFUTF16(IDS_TRANSLATE_BUBBLE_BEFORE_TRANSLATE2,
+                                 original_language_name,
+                                 target_language_name,
+                                 &offsets),
+      this);
+
+  auto style_info = views::StyledLabel::RangeStyleInfo::CreateForLink();
+
+  styled_label->AddStyleRange(
+      gfx::Range(static_cast<uint32_t>(offsets[0]),

[groby-ooo-7-16, 2016/04/27 19:08:24]

Can we amend gfx::Range to take a size_t? Because this is ugly :)

[ftang, 2016/04/27 21:43:15]

I agree this is ugly but I think that is out of the scope of this cl.

+                 static_cast<uint32_t>(offsets[0] +
+                     original_language_name.length())),
+      style_info);
+
+  styled_label->AddStyleRange(
+      gfx::Range(static_cast<uint32_t>(offsets[1]),
+                 static_cast<uint32_t>(offsets[1] +
+                     target_language_name.length())),
+      style_info);
+
+  styled_label->SetBounds(styled_label->x(),
+                          styled_label->y(),
+                          kQuestionWidth,
+                          styled_label->GetHeightForWidth(kQuestionWidth));
 
   std::vector<base::string16> items(
       static_cast<size_t>(DenialComboboxIndex::MENU_SIZE));
-  items[static_cast<size_t>(DenialComboboxIndex::DONT_TRANSLATE)] =
-      l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_DENY);
+  if (use_2016_q2_ui_) {
+    items[static_cast<size_t>(DenialComboboxIndex::DONT_TRANSLATE)] =
+        l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_ADVANCED);
+  } else {
+    items[static_cast<size_t>(DenialComboboxIndex::DONT_TRANSLATE)] =
+        l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_DENY);
+  }
   items[static_cast<size_t>(DenialComboboxIndex::NEVER_TRANSLATE_LANGUAGE)] =
       l10n_util::GetStringFUTF16(IDS_TRANSLATE_BUBBLE_NEVER_TRANSLATE_LANG,
                                  original_language_name);
   items[static_cast<size_t>(DenialComboboxIndex::NEVER_TRANSLATE_SITE)] =
       l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_NEVER_TRANSLATE_SITE);
 
+
   denial_combobox_model_.reset(new ui::SimpleComboboxModel(items));
   denial_combobox_ = new views::Combobox(denial_combobox_model_.get());
+
   denial_combobox_->set_id(COMBOBOX_ID_DENIAL);
   denial_combobox_->set_listener(this);
   denial_combobox_->SetStyle(views::Combobox::STYLE_ACTION);
 
+  // In an incognito window, "Always translate" checkbox shouldn't be shown.
+  if (show_always_checkbox_ && !is_in_incognito_window_) {
+    always_translate_checkbox_ = new views::Checkbox(base::string16());
+    always_translate_checkbox_->SetChecked(model_->GetAlwaysTranslateChecked());
+    always_translate_checkbox_->set_id(BUTTON_ID_ALWAYS_TRANSLATE);
+    always_translate_checkbox_->SetText(
+        l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_ALWAYS));
+    always_translate_checkbox_->set_listener(this);
+  }
+
   views::View* view = new views::View();
   views::GridLayout* layout = new views::GridLayout(view);
   view->SetLayoutManager(layout);
@@ -433,9 +593,16 @@ views::View* TranslateBubbleView::CreateViewBeforeTranslate() {
   };
 
   views::ColumnSet* cs = layout->AddColumnSet(COLUMN_SET_ID_MESSAGE);
-  cs->AddColumn(GridLayout::LEADING, GridLayout::CENTER,
-                0, GridLayout::USE_PREF, 0, 0);
-  cs->AddPaddingColumn(0, views::kRelatedButtonHSpacing);
+  if (show_icon_) {
+    cs->AddColumn(GridLayout::LEADING, GridLayout::CENTER,
+                  0, GridLayout::USE_PREF, 0, 0);
+    cs->AddPaddingColumn(0, views::kRelatedButtonHSpacing);
+  }
+  if (!use_2016_q2_ui_) {
+    cs->AddColumn(GridLayout::LEADING, GridLayout::CENTER,
+                  0, GridLayout::USE_PREF, 0, 0);
+    cs->AddPaddingColumn(0, views::kRelatedButtonHSpacing);
+  }
   cs->AddColumn(GridLayout::LEADING, GridLayout::CENTER,
                 0, GridLayout::USE_PREF, 0, 0);
   cs->AddPaddingColumn(1, 0);
@@ -449,18 +616,41 @@ views::View* TranslateBubbleView::CreateViewBeforeTranslate() {
                 0, GridLayout::USE_PREF, 0, 0);
 
   layout->StartRow(0, COLUMN_SET_ID_MESSAGE);
-  layout->AddView(message_label);
-  layout->AddView(CreateLink(this,
-                             IDS_TRANSLATE_BUBBLE_ADVANCED,
-                             LINK_ID_ADVANCED));
+  if (show_icon_) {
+    views::ImageView* icon = new views::ImageView();
+    ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
+    icon->SetImage(
+          bundle.GetImageNamed(IDR_TRANSLATE_ICON_V2).ToImageSkia());
+    layout->AddView(icon);
+  }
+  if (use_2016_q2_ui_) {
+    layout->AddView(styled_label);

[groby-ooo-7-16, 2016/04/27 19:08:24]

styled_label leaks if we're  not using the new UI. Why not create it on demand
only?

[ftang, 2016/04/27 21:43:15]

Done.

+  } else {
+    layout->AddView(message_label);
+    auto link = CreateLink(this,
+                           IDS_TRANSLATE_BUBBLE_ADVANCED,
+                           LINK_ID_ADVANCED);
+    layout->AddView(link);
+  }
 
+
+  if (show_always_checkbox_ && !is_in_incognito_window_) {
+    layout->StartRow(0, COLUMN_SET_ID_MESSAGE);
+    layout->SkipColumns(1);
+    layout->AddView(always_translate_checkbox_);
+  }
   layout->AddPaddingRow(0, views::kUnrelatedControlVerticalSpacing);
 
   layout->StartRow(0, COLUMN_SET_ID_CONTENT);
-  views::LabelButton* accept_button = CreateLabelButton(
-      this,
-      l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_ACCEPT),
-      BUTTON_ID_TRANSLATE);
+  views::LabelButton* accept_button = use_blue_button_ ?
+      CreateBlueButton(
+          this,
+          l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_ACCEPT),
+          BUTTON_ID_TRANSLATE) :
+      CreateLabelButton(
+          this,
+          l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_ACCEPT),
+          BUTTON_ID_TRANSLATE);
   layout->AddView(accept_button);
   accept_button->SetIsDefault(true);
   layout->AddView(denial_combobox_);
@@ -486,8 +676,13 @@ views::View* TranslateBubbleView::CreateViewTranslating() {
   };
 
   views::ColumnSet* cs = layout->AddColumnSet(COLUMN_SET_ID_MESSAGE);
+  if (show_icon_) {
+    cs->AddColumn(GridLayout::LEADING, GridLayout::CENTER,
+                  0, views::GridLayout::USE_PREF, 0, 0);
+    cs->AddPaddingColumn(0, views::kRelatedButtonHSpacing);
+  }
   cs->AddColumn(GridLayout::LEADING, GridLayout::CENTER,
-                0, views::GridLayout::USE_PREF, 0, 0);
+                0, GridLayout::USE_PREF, 0, 0);
   cs->AddPaddingColumn(1, 0);
 
   cs = layout->AddColumnSet(COLUMN_SET_ID_CONTENT);
@@ -496,6 +691,13 @@ views::View* TranslateBubbleView::CreateViewTranslating() {
                 0, GridLayout::USE_PREF, 0, 0);
 
   layout->StartRow(0, COLUMN_SET_ID_MESSAGE);
+  if (show_icon_) {
+    views::ImageView* icon = new views::ImageView();
+    ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
+    icon->SetImage(
+          bundle.GetImageNamed(IDR_TRANSLATE_ICON_V2).ToImageSkia());
+    layout->AddView(icon);
+  }
   layout->AddView(label);
 
   layout->AddPaddingRow(0, views::kUnrelatedControlVerticalSpacing);
@@ -527,11 +729,16 @@ views::View* TranslateBubbleView::CreateViewAfterTranslate() {
   };
 
   views::ColumnSet* cs = layout->AddColumnSet(COLUMN_SET_ID_MESSAGE);
+  if (show_icon_) {
+    cs->AddColumn(GridLayout::LEADING, GridLayout::CENTER,
+                  0, views::GridLayout::USE_PREF, 0, 0);
+    cs->AddPaddingColumn(0, views::kRelatedButtonHSpacing);
+  }
   cs->AddColumn(GridLayout::LEADING, GridLayout::CENTER,
                 0, views::GridLayout::USE_PREF, 0, 0);
   cs->AddPaddingColumn(0, views::kRelatedButtonHSpacing);
   cs->AddColumn(GridLayout::LEADING, GridLayout::CENTER,
-                0, views::GridLayout::USE_PREF, 0, 0);
+                0, GridLayout::USE_PREF, 0, 0);
   cs->AddPaddingColumn(1, 0);
 
   cs = layout->AddColumnSet(COLUMN_SET_ID_CONTENT);
@@ -540,6 +747,13 @@ views::View* TranslateBubbleView::CreateViewAfterTranslate() {
                 0, GridLayout::USE_PREF, 0, 0);
 
   layout->StartRow(0, COLUMN_SET_ID_MESSAGE);
+  if (show_icon_) {
+    views::ImageView* icon = new views::ImageView();
+    ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
+    icon->SetImage(
+          bundle.GetImageNamed(IDR_TRANSLATE_ICON_V2).ToImageSkia());
+    layout->AddView(icon);
+  }
   layout->AddView(label);
   layout->AddView(CreateLink(this,
                              IDS_TRANSLATE_BUBBLE_ADVANCED,
@@ -572,6 +786,11 @@ views::View* TranslateBubbleView::CreateViewError() {
   };
 
   views::ColumnSet* cs = layout->AddColumnSet(COLUMN_SET_ID_MESSAGE);
+  if (show_icon_) {
+    cs->AddColumn(GridLayout::LEADING, GridLayout::CENTER,
+                  0, GridLayout::USE_PREF, 0, 0);
+    cs->AddPaddingColumn(0, views::kRelatedButtonHSpacing);
+  }
   cs->AddColumn(GridLayout::LEADING, GridLayout::CENTER,
                 0, GridLayout::USE_PREF, 0, 0);
   cs->AddPaddingColumn(0, views::kRelatedButtonHSpacing);
@@ -585,6 +804,13 @@ views::View* TranslateBubbleView::CreateViewError() {
                 0, GridLayout::USE_PREF, 0, 0);
 
   layout->StartRow(0, COLUMN_SET_ID_MESSAGE);
+  if (show_icon_) {

[groby-ooo-7-16, 2016/04/27 19:08:25]

Given that this is common code, I'm inclined to suggest factoring out.

[ftang, 2016/04/27 21:43:15]

Done.

+    views::ImageView* icon = new views::ImageView();
+    ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
+    icon->SetImage(
+        bundle.GetImageNamed(IDR_TRANSLATE_ICON_V2).ToImageSkia());
+    layout->AddView(icon);
+  }
   layout->AddView(label);
   layout->AddView(CreateLink(this,
                              IDS_TRANSLATE_BUBBLE_ADVANCED,
@@ -687,8 +913,11 @@ views::View* TranslateBubbleView::CreateViewAdvanced() {
   layout->AddView(CreateLink(this,
                              IDS_TRANSLATE_BUBBLE_LANGUAGE_SETTINGS,
                              LINK_ID_LANGUAGE_SETTINGS));
-  advanced_done_button_ = CreateLabelButton(
-      this, l10n_util::GetStringUTF16(IDS_DONE), BUTTON_ID_DONE);
+  advanced_done_button_ = use_blue_button_ ?
+      CreateBlueButton(
+          this, l10n_util::GetStringUTF16(IDS_DONE), BUTTON_ID_DONE) :
+      CreateLabelButton(
+          this, l10n_util::GetStringUTF16(IDS_DONE), BUTTON_ID_DONE);
   advanced_done_button_->SetIsDefault(true);
   advanced_cancel_button_ = CreateLabelButton(
       this, l10n_util::GetStringUTF16(IDS_CANCEL), BUTTON_ID_CANCEL);
@@ -734,6 +963,8 @@ void TranslateBubbleView::UpdateAdvancedView() {
     always_translate_checkbox_->SetText(
         l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_ALWAYS));
     always_translate_checkbox_->SetChecked(
+        use_2016_q2_ui_ ?
+        model_->GetAlwaysTranslateChecked() :
         model_->ShouldAlwaysTranslate());
   }