Checkbox alignment & bigger height for Uma opt-in view.

chrome/browser/ui/views/session_crashed_bubble_view.cc

Index: chrome/browser/ui/views/session_crashed_bubble_view.cc
diff --git a/chrome/browser/ui/views/session_crashed_bubble_view.cc b/chrome/browser/ui/views/session_crashed_bubble_view.cc
index ef43a73ed20883f69e3d5047ada5ae3ad06633d7..c6b01a96c87abb9e9d72dddc44f38ddb8b8a4c22 100644
--- a/chrome/browser/ui/views/session_crashed_bubble_view.cc
+++ b/chrome/browser/ui/views/session_crashed_bubble_view.cc
@@ -55,6 +55,9 @@ const int kWidthOfDescriptionText = 320;
 // Distance between checkbox and the text to the right of it.
 const int kCheckboxTextDistance = 4;
 
+// Bottom inset for UMA opt-in view in pixels.
+const int kUMAOptinViewBottomInset = 10;

[msw, 2014/09/18 23:14:48]

Move this down to where it's used and change the comment to explain why it's
necessary, the identifier already describes what it is.

[yao, 2014/09/19 15:12:27]

There is no particular reason, originally value 8 (from
views::kRelatedControlVerticalSpacing) was used, but the UI designer wants it to
be 10px now. 

+
 // The color of the text and background of the sub panel to offer UMA optin.
 // These values match the BookmarkSyncPromoView colors.
 const SkColor kBackgroundColor = SkColorSetRGB(245, 245, 245);
@@ -202,7 +205,7 @@ SessionCrashedBubbleView::SessionCrashedBubbleView(
       web_contents_(web_contents),
       restore_button_(NULL),
       uma_option_(NULL),
-      offer_uma_optin_(offer_uma_optin),
+      offer_uma_optin_(true),

[msw, 2014/09/18 23:14:48]

Why are you changing this? Was this intentional?

[yao, 2014/09/19 15:12:27]

Done.

       started_navigation_(false),
       restored_(false) {
   set_close_on_deactivate(false);
@@ -336,6 +339,9 @@ views::View* SessionCrashedBubbleView::CreateUMAOptinView() {
   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);
+  // Since the checkbox has a bigger height than text, moves it down by 1px to
+  // make them look aligned.
+  uma_label->SetBorder(views::Border::CreateEmptyBorder(1, 0, 0, 0));

[msw, 2014/09/18 23:14:48]

Why is this and the uma_layout inset change needed? Shouldn't one or the other
be sufficient?

[yao, 2014/09/19 15:12:26]

They are separate things. Changed the issue title to make this clearer.

This is for the checkbox alignment.

The inset change is to make the empty region at bottom of the UMA  opt-in view
higher (2 more pixels). 

 
   // Create a view to hold the checkbox and the text.
   views::View* uma_view = new views::View();
@@ -346,7 +352,7 @@ views::View* SessionCrashedBubbleView::CreateUMAOptinView() {
       views::Background::CreateSolidBackground(kBackgroundColor));
   int inset_left = GetBubbleFrameView()->GetTitleInsets().left();
   uma_layout->SetInsets(views::kRelatedControlVerticalSpacing, inset_left,
-                        views::kRelatedControlVerticalSpacing, inset_left);
+                        kUMAOptinViewBottomInset, inset_left);
 
   const int kReportColumnSetId = 0;
   views::ColumnSet* cs = uma_layout->AddColumnSet(kReportColumnSetId);