Align the checkbox with the first line of the text.

chrome/browser/ui/views/session_crashed_bubble_view.cc

 // Copyright 2014 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/session_crashed_bubble_view.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 135 matching lines @@
                  base::Passed(&browser_observer)));
 #else
   SessionCrashedBubbleView::ShowForReal(browser_observer.Pass(), false);
 #endif  // defined(GOOGLE_CHROME_BUILD)
 }
 
 // static
 void SessionCrashedBubbleView::ShowForReal(
     scoped_ptr<BrowserRemovalObserver> browser_observer,
     bool uma_opted_in_already) {
-  // Determine whether or not the uma opt-in option should be offered. It is
+  // Determine whether or not the UMA opt-in option should be offered. It is
   // offered only when it is a Google chrome build, user hasn't opted in yet,
   // and the preference is modifiable by the user.
   bool offer_uma_optin = false;
 
 #if defined(GOOGLE_CHROME_BUILD)
   if (!uma_opted_in_already) {
     offer_uma_optin = g_browser_process->local_state()->FindPreference(
         prefs::kMetricsReportingEnabled)->IsUserModifiable();
   }
 #endif  // defined(GOOGLE_CHROME_BUILD)
@@ skipping 108 matching lines @@
   cs->AddPaddingColumn(0, GetBubbleFrameView()->GetTitleInsets().left());
 
   layout->StartRow(0, kTextColumnSetId);
   layout->AddView(text_label);
   layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
 
   layout->StartRow(0, kButtonColumnSetId);
   layout->AddView(restore_button_);
   layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
 
+  int bottom_margin = 1;
+
   // Metrics reporting option.
   if (offer_uma_optin_) {
-    CreateUmaOptinView(layout);
+    const int kUMAOptionColumnSetId = 2;

[msw, 2014/08/27 23:43:07]

nit: should this be "Optin" instead of "Option"?

[yao, 2014/08/28 00:13:36]

I wanted it to be Option, (The checkout box is named uma_option, I was thinking
it's an option to optin to UMA). If you think it makes more sense to be optin,
I'll change it. 

[msw, 2014/08/28 00:15:35]

It's fine as-is if that what you intended.

+    cs = layout->AddColumnSet(kUMAOptionColumnSetId);
+    cs->AddColumn(
+        GridLayout::FILL, GridLayout::FILL, 1, GridLayout::USE_PREF, 0, 0);
+    layout->StartRow(1, kUMAOptionColumnSetId);
+    layout->AddView(new views::Separator(views::Separator::HORIZONTAL));
+    layout->StartRow(1, kUMAOptionColumnSetId);
+    layout->AddView(CreateUMAOptinView());
+
+    // Since the UMA optin row has a different background than the default
+    // background color of bubbles, the bottom margin has to be 0 to make sure
+    // the background extends to the bottom edge of the bubble.
+    bottom_margin = 0;
+
     RecordBubbleHistogramValue(SESSION_CRASHED_BUBBLE_OPTIN_BAR_SHOWN);
   }
 
-  set_margins(gfx::Insets());
+  set_margins(gfx::Insets(1, 0, bottom_margin, 0));
   Layout();
 }
 
-void SessionCrashedBubbleView::CreateUmaOptinView(GridLayout* layout) {
+views::View* SessionCrashedBubbleView::CreateUMAOptinView() {
   // Checkbox for metric reporting setting.
   // Since the text to the right of the checkbox can't be a simple string (needs
   // a hyperlink in it), this checkbox contains an empty string as its label,
   // and the real text will be added as a separate view.
   uma_option_ = new views::Checkbox(base::string16());
   uma_option_->SetChecked(false);
-  uma_option_->set_background(
-      views::Background::CreateSolidBackground(kBackgroundColor));
 
   // The text to the right of the checkbox.
   size_t offset;
   base::string16 link_text =
       l10n_util::GetStringUTF16(IDS_SESSION_CRASHED_BUBBLE_UMA_LINK_TEXT);
   base::string16 uma_text = l10n_util::GetStringFUTF16(
       IDS_SESSION_CRASHED_VIEW_UMA_OPTIN,
       link_text,
       &offset);
   views::StyledLabel* uma_label = new views::StyledLabel(uma_text, this);
-  uma_label->set_background(
-      views::Background::CreateSolidBackground(kBackgroundColor));
   views::StyledLabel::RangeStyleInfo link_style =
       views::StyledLabel::RangeStyleInfo::CreateForLink();
   link_style.font_style = gfx::Font::NORMAL;
   uma_label->AddStyleRange(gfx::Range(offset, offset + link_text.length()),
                            link_style);
   views::StyledLabel::RangeStyleInfo uma_style;
   uma_style.color = kTextColor;
   gfx::Range before_link_range(0, offset);
   if (!before_link_range.is_empty())
     uma_label->AddStyleRange(before_link_range, uma_style);
   gfx::Range after_link_range(offset + link_text.length(), uma_text.length());
   if (!after_link_range.is_empty())
     uma_label->AddStyleRange(after_link_range, uma_style);
 
-  // We use a border instead of padding so that the background color reaches
-  // the edges of the bubble.
-  const gfx::Insets title_insets = GetBubbleFrameView()->GetTitleInsets();
-  uma_option_->SetBorder(views::Border::CreateSolidSidedBorder(
-      0, title_insets.left(), 0, 0, kBackgroundColor));
-  uma_label->SetBorder(views::Border::CreateSolidSidedBorder(
-      views::kRelatedControlVerticalSpacing, kCheckboxTextDistance,
-      views::kRelatedControlVerticalSpacing, title_insets.left(),
-      kBackgroundColor));
+  // Create a view to hold the checkbox and the text.
+  views::View* uma_view = new views::View();
+  GridLayout* uma_layout = new GridLayout(uma_view);
+  uma_view->SetLayoutManager(uma_layout);
 
-  // Separator.
-  const int kSeparatorColumnSetId = 2;
-  views::ColumnSet* cs = layout->AddColumnSet(kSeparatorColumnSetId);
-  cs->AddColumn(GridLayout::FILL, GridLayout::FILL, 1,
+  uma_view->set_background(
+      views::Background::CreateSolidBackground(kBackgroundColor));
+  int inset_left = GetBubbleFrameView()->GetTitleInsets().left();
+  uma_layout->SetInsets(views::kRelatedControlVerticalSpacing, inset_left,
+                        views::kRelatedControlVerticalSpacing, inset_left);
+
+  const int kReportColumnSetId = 0;
+  views::ColumnSet* cs = uma_layout->AddColumnSet(kReportColumnSetId);
+  cs->AddColumn(GridLayout::CENTER, GridLayout::LEADING, 0,
                 GridLayout::USE_PREF, 0, 0);
-
-  // Reporting row.
-  const int kReportColumnSetId = 3;
-  cs = layout->AddColumnSet(kReportColumnSetId);
-  cs->AddColumn(GridLayout::CENTER, GridLayout::FILL, 0,
-                GridLayout::USE_PREF, 0, 0);
+  cs->AddPaddingColumn(0, kCheckboxTextDistance);
   cs->AddColumn(GridLayout::FILL, GridLayout::FILL, 0,
                 GridLayout::FIXED, kWidthOfDescriptionText, 0);
 
-  layout->StartRow(0, kSeparatorColumnSetId);
-  layout->AddView(new views::Separator(views::Separator::HORIZONTAL));
-  layout->StartRow(0, kReportColumnSetId);
-  layout->AddView(uma_option_);
-  layout->AddView(uma_label);
+  uma_layout->StartRow(0, kReportColumnSetId);
+  uma_layout->AddView(uma_option_);
+  uma_layout->AddView(uma_label);
+
+  return uma_view;
 }
 
 void SessionCrashedBubbleView::ButtonPressed(views::Button* sender,
                                              const ui::Event& event) {
   DCHECK_EQ(sender, restore_button_);
   RestorePreviousSession(sender);
 }
 
 void SessionCrashedBubbleView::StyledLabelLinkClicked(const gfx::Range& range,
                                                       int event_flags) {
@@ skipping 63 matching lines @@
   GetWidget()->Close();
 }
 
 bool ShowSessionCrashedBubble(Browser* browser) {
   if (IsBubbleUIEnabled()) {
     SessionCrashedBubbleView::Show(browser);
     return true;
   }
   return false;
 }