UI polish for certificate viewer

chrome/browser/chromeos/frame/bubble_frame_view.cc

 // Copyright (c) 2011 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/chromeos/frame/bubble_frame_view.h"
 
 #include <algorithm>
 
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/chromeos/frame/bubble_window.h"
@@ skipping 130 matching lines @@
 
 void BubbleFrameView::UpdateWindowIcon() {
 }
 
 gfx::Insets BubbleFrameView::GetInsets() const {
   return (style_ & STYLE_FLUSH) ?
          gfx::Insets() :
          gfx::Insets(kTitleTopPadding,
                      kHorizontalPadding,
                      0,
                      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());
   return frame_->non_client_view()->GetWindowBoundsForClientBounds(
       bounds).size();
 }
 
 void BubbleFrameView::Layout() {
   gfx::Insets insets = GetInsets();
 
   gfx::Size title_size;
   if (title_)
     title_size = title_->GetPreferredSize();
   gfx::Size close_button_size;
   if (close_button_)
     close_button_size = close_button_->GetPreferredSize();
   gfx::Size throbber_size;
   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());
   }
 
   int top_height = insets.top();
   if (title_size.height() > 0 ||
       close_button_size.height() > 0 ||
       throbber_size.height() > 0) {
     top_height += kTitleContentPadding + std::max(
         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) {
   SkPaint paint;
   paint.setStyle(SkPaint::kFill_Style);
   paint.setColor(kBubbleWindowBackgroundColor);
   gfx::Path path;
   gfx::Rect bounds(GetContentsBounds());
   SkRect rect;
   rect.set(SkIntToScalar(bounds.x()), SkIntToScalar(bounds.y()),
            SkIntToScalar(bounds.right()), SkIntToScalar(bounds.bottom()));
   path.addRect(rect);
   canvas->GetSkCanvas()->drawPath(path, paint);
 }
 
 void BubbleFrameView::ButtonPressed(views::Button* sender,
                                     const views::Event& event) {
   if (close_button_ != NULL && sender == close_button_)
     frame_->Close();
 }
 
 }  // namespace chromeos