UI polish for certificate viewer

chrome/browser/chromeos/frame/bubble_frame_view.cc

Index: chrome/browser/chromeos/frame/bubble_frame_view.cc
diff --git a/chrome/browser/chromeos/frame/bubble_frame_view.cc b/chrome/browser/chromeos/frame/bubble_frame_view.cc
index dd418520ad36b2a5a7929352a1ec61a823bf645f..de3e805f61e105f4ccfcc1d3727d5e0a2b9159a8 100644
--- a/chrome/browser/chromeos/frame/bubble_frame_view.cc
+++ b/chrome/browser/chromeos/frame/bubble_frame_view.cc
@@ -151,6 +151,15 @@ gfx::Insets BubbleFrameView::GetInsets() const {
                      kHorizontalPadding);

[flackr, 2011/11/21 20:43:31]

I think the Insets you return here should not include the horizontal padding.
According to views/view.h this returns the insets of the current border. In the
case of FLUSH_CONTENT the overall border does not include the horizontal
padding.

[bshe, 2011/11/22 16:26:24]

I was thinking that I can use offset_insets to offset the horizontal padding of
content. So that I just need to change the code in one place(content) instead of
three places(title, x_button, throbber). But you are right. I should conform
with the description of this function in views/view.h. Less hacky.

 }
 
+gfx::Insets BubbleFrameView::GetOffsetInsets() const {
+  return (style_ & STYLE_FLUSH_CONTENT) ?
+         gfx::Insets(0,
+                     -kHorizontalPadding,
+                     0,
+                     -kHorizontalPadding) :
+         gfx::Insets();
+}
+
 gfx::Size BubbleFrameView::GetPreferredSize() {
   gfx::Size pref = frame_->client_view()->GetPreferredSize();
   gfx::Rect bounds(0, 0, pref.width(), pref.height());
@@ -171,22 +180,33 @@ void BubbleFrameView::Layout() {
   if (throbber_)
     throbber_size = throbber_->GetPreferredSize();
 
+  // Need to center elements which are shorter.
+  int max_height = std::max(title_size.height(),
+                            std::max(close_button_size.height(),
+                                     throbber_size.height()));
+
   if (title_) {
     title_->SetBounds(
-        insets.left(), insets.top(),
+        insets.left(),
+        insets.top() + (static_cast<int>(

[flackr, 2011/11/21 20:43:31]

Remove static_cast, integer division results in an integer.

[bshe, 2011/11/22 16:26:24]

Done. 
A stupid mistake...

+            (max_height - title_size.height()) / 2)), // Center
         std::max(0, width() - insets.width() - close_button_size.width()),
         title_size.height());
   }
 
   if (close_button_) {
     close_button_->SetBounds(
-        width() - insets.right() - close_button_size.width(), insets.top(),
+        width() - insets.right() - close_button_size.width(),
+        insets.top() + (static_cast<int>(

[flackr, 2011/11/21 20:43:31]

ditto

[bshe, 2011/11/22 16:26:24]

Done.

+            (max_height - close_button_size.height()) / 2)),
         close_button_size.width(), close_button_size.height());
   }
 
   if (throbber_) {
     throbber_->SetBounds(
-        insets.left(), insets.top(),
+        insets.left(),
+        insets.top() + (static_cast<int> (

[flackr, 2011/11/21 20:43:31]

ditto

[bshe, 2011/11/22 16:26:24]

Done.

+            (max_height - throbber_size.height()) / 2)),
         std::min(throbber_size.width(), width()),
         throbber_size.height());
   }
@@ -199,9 +219,12 @@ void BubbleFrameView::Layout() {
         std::max(title_size.height(), close_button_size.height()),
         throbber_size.height());
   }
-  client_view_bounds_.SetRect(insets.left(), top_height,
-      std::max(0, width() - insets.width()),
-      std::max(0, height() - top_height - insets.bottom()));
+
+  gfx::Insets offset_insets = GetOffsetInsets();

[flackr, 2011/11/21 20:43:31]

As per my above comment, my preference would be if you return the insets without
horizontal padding from GetInsets, and then you can use a local variable to
customize the insets for the titlebar, close button, and throbber above.

[bshe, 2011/11/22 16:26:24]

Done.

+  client_view_bounds_.SetRect(insets.left() + offset_insets.left(), top_height,
+      std::max(0, width() - insets.width() - offset_insets.width()),
+      std::max(0, height() - top_height - insets.bottom()
+               - offset_insets.bottom()));
 }
 
 void BubbleFrameView::OnPaint(gfx::Canvas* canvas) {