Add an BubbleBorder to BrowserAction popups and fix positioning of the

popup.

BUG=23833,23835
TEST=none

Review URL: http://codereview.chromium.org/259065

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@28187 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 108 insertions, 32 deletions

diff --git a/chrome/browser/views/browser_actions_container.cc b/chrome/browser/views/browser_actions_container.cc
index e97e053..ae68c37 100644
--- a/chrome/browser/views/browser_actions_container.cc
+++ b/chrome/browser/views/browser_actions_container.cc
@@ -334,8 +334,8 @@ void BrowserActionsContainer::OnBrowserActionExecuted(
       return;
 
     gfx::Point origin;
-    View::ConvertPointToWidget(button, &origin);
-    gfx::Rect rect = bounds();
+    View::ConvertPointToScreen(button, &origin);
+    gfx::Rect rect = button->bounds();
     rect.set_x(origin.x());
     rect.set_y(origin.y());
     popup_ = ExtensionPopup::Show(browser_action.popup_url(),
diff --git a/chrome/browser/views/browser_bubble.h b/chrome/browser/views/browser_bubble.h
index 327d4fd..36accbd 100644
--- a/chrome/browser/views/browser_bubble.h
+++ b/chrome/browser/views/browser_bubble.h
@@ -88,17 +88,17 @@ class BrowserBubble {
   // Move the popup to an absolute position.
   void MovePopup(int x, int y, int w, int h);
 
- private:
+  // The widget that this bubble is in.
+  views::Widget* popup_;
+
   // The frame that this bubble is attached to.
   views::Widget* frame_;
   gfx::NativeView frame_native_view_;
 
+ private:
   // The view that is displayed in this bubble.
   views::View* view_;
 
-  // The widget that this bubble is in.
-  views::Widget* popup_;
-
   // The bounds relative to the frame.
   gfx::Rect bounds_;
 
diff --git a/chrome/browser/views/browser_bubble_win.cc b/chrome/browser/views/browser_bubble_win.cc
index 17ab8ca..3a5bca1 100644
--- a/chrome/browser/views/browser_bubble_win.cc
+++ b/chrome/browser/views/browser_bubble_win.cc
@@ -62,24 +62,10 @@ private:
 };
 
 void BrowserBubble::InitPopup() {
-  gfx::NativeWindow native_window = frame_->GetWindow()->GetNativeWindow();
-
   // popup_ is a Widget, but we need to do some WidgetWin stuff first, then
   // we'll assign it into popup_.
   views::WidgetWin* pop = new BubbleWidget(this);
   pop->set_window_style(WS_POPUP);
-
-#if 0
-  // TODO(erikkay) Layered windows don't draw child windows.
-  // Apparently there's some tricks you can do to handle that.
-  // Do the research so we can use this.
-  pop->set_window_ex_style(WS_EX_LAYERED |
-                           l10n_util::GetExtendedTooltipStyles());
-  pop->SetOpacity(0xFF);
-#endif
-
-  // A focus manager is necessary if you want to be able to handle various
-  // mouse events properly.
   pop->Init(frame_native_view_, bounds_);
   pop->SetContentsView(view_);
 
diff --git a/chrome/browser/views/bubble_border.cc b/chrome/browser/views/bubble_border.cc
index 9796a46..3c18983 100644
--- a/chrome/browser/views/bubble_border.cc
+++ b/chrome/browser/views/bubble_border.cc
@@ -265,3 +265,22 @@ void BubbleBorder::Paint(const views::View& view, gfx::Canvas* canvas) const {
                          width - bl_width - br_width, b_height);
   }
 }
+
+/////////////////////////
+
+void BubbleBackground::Paint(gfx::Canvas* canvas, views::View* view) const {
+  // The border of this view creates an anti-aliased round-rect region for the
+  // contents, which we need to fill with the background color.
+  SkPaint paint;
+  paint.setAntiAlias(true);
+  paint.setStyle(SkPaint::kFill_Style);
+  paint.setColor(border_->background_color());
+  gfx::Path path;
+  gfx::Rect bounds(view->GetLocalBounds(false));
+  SkRect rect;
+  rect.set(SkIntToScalar(bounds.x()), SkIntToScalar(bounds.y()),
+           SkIntToScalar(bounds.right()), SkIntToScalar(bounds.bottom()));
+  SkScalar radius = SkIntToScalar(BubbleBorder::GetCornerRadius());
+  path.addRoundRect(rect, radius, radius);
+  canvas->drawPath(path, paint);
+}
diff --git a/chrome/browser/views/bubble_border.h b/chrome/browser/views/bubble_border.h
index 75f7a02..f7af358 100644
--- a/chrome/browser/views/bubble_border.h
+++ b/chrome/browser/views/bubble_border.h
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_VIEWS_BUBBLE_BORDER_H_
 
 #include "third_party/skia/include/core/SkColor.h"
+#include "views/background.h"
 #include "views/border.h"
 
 class SkBitmap;
@@ -46,6 +47,7 @@ class BubbleBorder : public views::Border {
   void set_background_color(SkColor background_color) {
     background_color_ = background_color;
   }
+  SkColor background_color() const { return background_color_; }
 
   // For borders with an arrow, gives the desired bounds (in screen coordinates)
   // given the rect to point to and the size of the contained contents.  This
@@ -99,4 +101,19 @@ class BubbleBorder : public views::Border {
   DISALLOW_COPY_AND_ASSIGN(BubbleBorder);
 };
 
+// A Background that clips itself to the specified BubbleBorder and uses
+// the background color of the BubbleBorder.
+class BubbleBackground : public views::Background {
+public:
+  BubbleBackground(BubbleBorder* border) : border_(border) {}
+
+  // Background overrides.
+  virtual void Paint(gfx::Canvas* canvas, views::View* view) const;
+
+private:
+  BubbleBorder* border_;
+
+  DISALLOW_COPY_AND_ASSIGN(BubbleBackground);
+};
+
 #endif  // #ifndef CHROME_BROWSER_VIEWS_BUBBLE_BORDER_H_
diff --git a/chrome/browser/views/extensions/extension_popup.cc b/chrome/browser/views/extensions/extension_popup.cc
index b938b27..c1174fb 100644
--- a/chrome/browser/views/extensions/extension_popup.cc
+++ b/chrome/browser/views/extensions/extension_popup.cc
@@ -14,27 +14,65 @@
 #include "chrome/common/notification_source.h"
 #include "chrome/common/notification_type.h"
 
+#include "views/widget/root_view.h"
+
 ExtensionPopup::ExtensionPopup(ExtensionHost* host,
                                views::Widget* frame,
                                const gfx::Rect& relative_to)
-    : BrowserBubble(host->view(),
-      frame,
-      gfx::Point()),
+    : BrowserBubble(host->view(), frame, gfx::Point()),
       relative_to_(relative_to),
       extension_host_(host) {
   registrar_.Add(this, NotificationType::EXTENSION_HOST_DID_STOP_LOADING,
                  Source<Profile>(host->profile()));
+
+  // TODO(erikkay) Some of this border code is derived from InfoBubble.
+  // We should see if we can unify these classes.
+  gfx::NativeView native_window = frame->GetNativeView();
+  border_widget_.reset(views::Widget::CreateTransparentPopupWidget(true));
+  border_widget_->Init(native_window, bounds());
+  border_ = new BubbleBorder;
+  border_->set_arrow_location(BubbleBorder::TOP_RIGHT);
+  border_view_ = new views::View;
+  border_view_->set_background(new BubbleBackground(border_));
+  border_view_->set_border(border_);
+  border_widget_->SetContentsView(border_view_);
 }
 
 ExtensionPopup::~ExtensionPopup() {
+  border_widget_->Close();
+}
+
+void ExtensionPopup::Hide() {
+  BrowserBubble::Hide();
+  border_widget_->Hide();
 }
 
 void ExtensionPopup::Show() {
   ResizeToView();
 
-  // Anchor on the lower right corner and extend to the left.
-  SetBounds(relative_to_.right() - width(), relative_to_.bottom(),
-            width(), height());
+  // The rounded corners cut off more of the view than the border insets claim.
+  // Since we can't clip the ExtensionView's corners, we need to increase the
+  // inset by half the corner radius as well as lying about the size of the
+  // contents size to compensate.
+  int corner_inset = BubbleBorder::GetCornerRadius() / 2;
+  gfx::Size adjusted_size = bounds().size();
+  adjusted_size.Enlarge(2 * corner_inset, 2 * corner_inset);
+  gfx::Rect rect = border_->GetBounds(relative_to_, adjusted_size);
+  border_widget_->SetBounds(rect);
+
+  // Now calculate the inner bounds.  This is a bit more convoluted than
+  // it should be because BrowserBubble coordinates are in Browser coordinates
+  // while |rect| is in screen coordinates.
+  gfx::Insets border_insets;
+  border_->GetInsets(&border_insets);
+  gfx::Point origin = rect.origin();
+  views::View::ConvertPointToView(NULL, frame_->GetRootView(), &origin);
+  origin.set_x(origin.x() + border_insets.left() + corner_inset);
+  origin.set_y(origin.y() + border_insets.top() + corner_inset);
+  MoveTo(origin.x(), origin.y());
+
+  // Show the border first, then the popup overlaid on top.
+  border_widget_->Show();
   BrowserBubble::Show(true);
 }
 
diff --git a/chrome/browser/views/extensions/extension_popup.h b/chrome/browser/views/extensions/extension_popup.h
index cdefc8e..ec0ba93 100644
--- a/chrome/browser/views/extensions/extension_popup.h
+++ b/chrome/browser/views/extensions/extension_popup.h
@@ -8,6 +8,7 @@
 #include "googleurl/src/gurl.h"
 #include "chrome/browser/extensions/extension_host.h"
 #include "chrome/browser/views/browser_bubble.h"
+#include "chrome/browser/views/bubble_border.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
 
@@ -20,8 +21,8 @@ class ExtensionPopup : public BrowserBubble,
   virtual ~ExtensionPopup();
 
   // Create and show a popup with |url| positioned below |relative_to| in
-  // |browser| coordinates. This is anchored to the lower right corner of the
-  // rect, extending to the left, just like the wrench and page menus.
+  // screen coordinates. This is anchored to the lower middle of the rect,
+  // extending to the left, just like the wrench and page menus.
   //
   // The actual display of the popup is delayed until the page contents
   // finish loading in order to minimize UI flashing and resizing.
@@ -33,6 +34,7 @@ class ExtensionPopup : public BrowserBubble,
 
   // BrowserBubble overrides.
   virtual void Show();
+  virtual void Hide();
 
   // NotificationObserver overrides.
   virtual void Observe(NotificationType type,
@@ -45,13 +47,21 @@ class ExtensionPopup : public BrowserBubble,
                  const gfx::Rect& relative_to);
 
   // The area on the screen that the popup should be positioned relative to.
-  const gfx::Rect relative_to_;
+  gfx::Rect relative_to_;
 
   // The contained host for the view.
   scoped_ptr<ExtensionHost> extension_host_;
 
   NotificationRegistrar registrar_;
 
+  // A separate widget and associated pieces to draw a border behind the
+  // popup.  This has to be a separate window in order to support transparency.
+  // Layered windows can't contain native child windows, so we wouldn't be
+  // able to have the ExtensionView child.
+  scoped_ptr<views::Widget> border_widget_;
+  BubbleBorder* border_;
+  views::View* border_view_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionPopup);
 };
 
diff --git a/chrome/test/data/extensions/samples/set_page_color/manifest.json b/chrome/test/data/extensions/samples/set_page_color/manifest.json
index 081681a..f4ad151 100755
--- a/chrome/test/data/extensions/samples/set_page_color/manifest.json
+++ b/chrome/test/data/extensions/samples/set_page_color/manifest.json
@@ -7,6 +7,6 @@
   "browser_action": {
       "name": "Set this page's color.",
       "icons": ["icon.png"],
-      "popup": { "path": "popup.html", "height": 78 }
+      "popup": { "path": "popup.html", "height": 79 }
   }
 }
\ No newline at end of file
diff --git a/chrome/test/data/extensions/samples/set_page_color/popup.html b/chrome/test/data/extensions/samples/set_page_color/popup.html
index 3de4f36..62de0ed 100755
--- a/chrome/test/data/extensions/samples/set_page_color/popup.html
+++ b/chrome/test/data/extensions/samples/set_page_color/popup.html
@@ -3,15 +3,21 @@ body {
   overflow: hidden;

   margin: 0px;

   padding: 0px;

-  background: #cccccc;

-  border: 1px solid black;

+  background: white;

+}

+

+div:first-child {

+  margin-top: 0px;

 }

+

 div {

   cursor: pointer;

   text-align: center;

   padding: 1px 3px;

   font: menu;

   width: 100px;

+  margin-top: 1px;

+  background: #cccccc;

 }

 div:hover {

   background: #aaaaaa;