Removal of chrome.experimental.popup set of APIs

chrome/browser/ui/views/extensions/extension_popup.cc

Index: chrome/browser/ui/views/extensions/extension_popup.cc
===================================================================
--- chrome/browser/ui/views/extensions/extension_popup.cc	(revision 73621)
+++ chrome/browser/ui/views/extensions/extension_popup.cc	(working copy)
@@ -60,31 +60,22 @@
                                views::Widget* frame,
                                const gfx::Rect& relative_to,
                                BubbleBorder::ArrowLocation arrow_location,
-                               bool activate_on_show,
                                bool inspect_with_devtools,
-                               PopupChrome chrome,
                                Observer* observer)
     : BrowserBubble(host->view(),
                     frame,
-                    gfx::Point(),
-                    RECTANGLE_CHROME == chrome),  // If no bubble chrome is to
-                                                  // be displayed, then enable a
-                                                  // drop-shadow on the bubble
-                                                  // widget.
+                    gfx::Point()),
       relative_to_(relative_to),
       extension_host_(host),
-      activate_on_show_(activate_on_show),
       inspect_with_devtools_(inspect_with_devtools),
       close_on_lost_focus_(true),
       closing_(false),
       border_widget_(NULL),
       border_(NULL),
       border_view_(NULL),
-      popup_chrome_(chrome),
       max_size_(kMaxWidth, kMaxHeight),

[rafaelw, 2011/02/09 22:08:28]

Can we revert the effect of http://codereview.chromium.org/5254007

[Jeff Timanus, 2011/02/10 00:17:49]

Done.

       observer_(observer),
-      anchor_position_(arrow_location),
-      instance_lifetime_(new InternalRefCounter()){
+      anchor_position_(arrow_location) {
   AddRef();  // Balanced in Close();
   set_delegate(this);
   host->view()->SetContainer(this);
@@ -108,51 +99,42 @@
   relative_to_.set_origin(origin);
 
   // The bubble chrome requires a separate window, so construct it here.
-  if (BUBBLE_CHROME == popup_chrome_) {
-    gfx::NativeView native_window = frame->GetNativeView();
+  gfx::NativeView native_window = frame->GetNativeView();
 #if defined(OS_LINUX)
-    border_widget_ = new views::WidgetGtk(views::WidgetGtk::TYPE_WINDOW);
-    static_cast<views::WidgetGtk*>(border_widget_)->MakeTransparent();
-    static_cast<views::WidgetGtk*>(border_widget_)->make_transient_to_parent();
+  border_widget_ = new views::WidgetGtk(views::WidgetGtk::TYPE_WINDOW);
+  static_cast<views::WidgetGtk*>(border_widget_)->MakeTransparent();
+  static_cast<views::WidgetGtk*>(border_widget_)->make_transient_to_parent();
 #else
-    border_widget_ = Widget::CreatePopupWidget(Widget::Transparent,
-                                               Widget::NotAcceptEvents,
-                                               Widget::DeleteOnDestroy,
-                                               Widget::MirrorOriginInRTL);
+  border_widget_ = Widget::CreatePopupWidget(Widget::Transparent,
+                                             Widget::NotAcceptEvents,
+                                             Widget::DeleteOnDestroy,
+                                             Widget::MirrorOriginInRTL);
 #endif
-    border_widget_->Init(native_window, bounds());
+  border_widget_->Init(native_window, bounds());
 #if defined(OS_CHROMEOS)
-    {
-      vector<int> params;
-      params.push_back(0);  // don't show while screen is locked
-      chromeos::WmIpc::instance()->SetWindowType(
-          border_widget_->GetNativeView(),
-          chromeos::WM_IPC_WINDOW_CHROME_INFO_BUBBLE,
-          &params);
-    }
+  {
+    vector<int> params;
+    params.push_back(0);  // don't show while screen is locked
+    chromeos::WmIpc::instance()->SetWindowType(
+        border_widget_->GetNativeView(),
+        chromeos::WM_IPC_WINDOW_CHROME_INFO_BUBBLE,
+        &params);
+  }
 #endif
-    border_ = new BubbleBorder(arrow_location);
-    border_view_ = new views::View;
-    border_view_->set_background(new BubbleBackground(border_));
+  border_ = new BubbleBorder(arrow_location);
+  border_view_ = new views::View;
+  border_view_->set_background(new BubbleBackground(border_));
 
-    border_view_->set_border(border_);
-    border_widget_->SetContentsView(border_view_);
-    // Ensure that the popup contents are always displayed ontop of the border
-    // widget.
-    border_widget_->MoveAbove(popup_);
-  } else {
-    // Otherwise simply set a black-border on the view containing the popup
-    // extension view.
-    views::Border* border = views::Border::CreateSolidBorder(kPopupBorderWidth,
-                                                             SK_ColorBLACK);
-    view()->set_border(border);
-  }
+  border_view_->set_border(border_);
+  border_widget_->SetContentsView(border_view_);
+  // Ensure that the popup contents are always displayed ontop of the border
+  // widget.
+  border_widget_->MoveAbove(popup_);

[rafaelw, 2011/02/09 22:08:28]

Are you sure this is still needed? 

http://codereview.chromium.org/454019/diff/8013/chrome/browser/views/extensio...

[Jeff Timanus, 2011/02/10 00:17:49]

It's not absolutely needed, because I've changed the code back to always
activate the popup on show.  However, the code is a little fragile without this
z-reordering.  I think it is better to keep the explicit z-reorder here, so that
we are not indirectly depending on activation.

 }
 
 ExtensionPopup::~ExtensionPopup() {
   // The widget is set to delete on destroy, so no leak here.
-  if (border_widget_)
-    border_widget_->Close();
+  border_widget_->Close();
 }
 
 void ExtensionPopup::SetArrowPosition(

[rafaelw, 2011/02/09 22:08:28]

I don't think this is needed anymore.

http://codereview.chromium.org/1774012/

[Jeff Timanus, 2011/02/10 00:17:49]

Done.

@@ -167,8 +149,7 @@
 
 void ExtensionPopup::Hide() {
   BrowserBubble::Hide();
-  if (border_widget_)
-    border_widget_->Hide();
+  border_widget_->Hide();
 }
 
 void ExtensionPopup::Show(bool activate) {
@@ -183,39 +164,31 @@
   ResizeToView();
 
   // Show the border first, then the popup overlaid on top.
-  if (border_widget_)
-    border_widget_->Show();
+  border_widget_->Show();
   BrowserBubble::Show(activate);
 }
 
 void ExtensionPopup::ResizeToView() {
-  if (observer_)
-    observer_->ExtensionPopupResized(this);
-
   gfx::Rect rect = GetOuterBounds();

[rafaelw, 2011/02/09 22:08:28]

Please consider putting this back the way it was:

http://codereview.chromium.org/577015/diff/14036/chrome/browser/views/extensi...

Including inlining GetOuterBounds. It was simpler that way.

[Jeff Timanus, 2011/02/10 00:17:49]

Done.

 
   gfx::Point origin = rect.origin();
   views::View::ConvertPointToView(NULL, frame_->GetRootView(), &origin);
 
-  if (border_widget_) {
-    // Set the bubble-chrome widget according to the outer bounds of the entire
-    // popup.
-    border_widget_->SetBounds(rect);
+  // Set the bubble-chrome widget according to the outer bounds of the entire
+  // popup.
+  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);
+  // 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);
 
-    origin.set_x(origin.x() + border_insets.left() + kPopupBubbleCornerRadius);
-    origin.set_y(origin.y() + border_insets.top() + kPopupBubbleCornerRadius);
+  origin.set_x(origin.x() + border_insets.left() + kPopupBubbleCornerRadius);
+  origin.set_y(origin.y() + border_insets.top() + kPopupBubbleCornerRadius);
 
-    gfx::Size new_size = view()->size();
-    SetBounds(origin.x(), origin.y(), new_size.width(), new_size.height());
-  } else {
-    SetBounds(origin.x(), origin.y(), rect.width(), rect.height());
-  }
+  gfx::Size new_size = view()->size();
+  SetBounds(origin.x(), origin.y(), new_size.width(), new_size.height());
 }
 
 void ExtensionPopup::BubbleBrowserWindowMoved(BrowserBubble* bubble) {
@@ -259,7 +232,7 @@
       // Once we receive did stop loading, the content will be complete and
       // the width will have been computed.  Now it's safe to show.
       if (extension_host_.get() == Details<ExtensionHost>(details).ptr()) {
-        Show(activate_on_show_);
+        Show(true);
 
         if (inspect_with_devtools_) {
           // Listen for the the devtools window closing.
@@ -313,20 +286,6 @@
       std::max(kMinWidth, std::min(max_width, sz.width())),
       std::max(kMinHeight, std::min(max_height, sz.height())));
 
-  // If popup_chrome_ == RECTANGLE_CHROME, the border is drawn in the client
-  // area of the ExtensionView, rather than in a window which sits behind it.
-  // In this case, the actual size of the view must be enlarged so that the
-  // web contents portion of the view gets its full PreferredSize area.
-  if (view->border()) {
-    gfx::Insets border_insets;
-    view->border()->GetInsets(&border_insets);
-
-    gfx::Rect bounds(view->bounds());
-    gfx::Size size(bounds.size());
-    size.Enlarge(border_insets.width(), border_insets.height());
-    view->SetBounds(bounds.x(), bounds.y(), size.width(), size.height());
-  }
-
   ResizeToView();
 }
 
@@ -338,35 +297,13 @@
 
   gfx::Size contents_size = view()->size();
 
-  // If the popup has a bubble-chrome, then let the BubbleBorder compute
-  // the bounds.
-  if (BUBBLE_CHROME == popup_chrome_) {
-    // 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.
-    contents_size.Enlarge(2 * kPopupBubbleCornerRadius,
-                          2 * kPopupBubbleCornerRadius);
-    return border_->GetBounds(relative_rect, contents_size);
-  }
-
-  // Position the bounds according to the location of the |anchor_position_|.
-  int y;
-  if (BubbleBorder::is_arrow_on_top(anchor_position_))
-    y = relative_rect.bottom();
-  else
-    y = relative_rect.y() - contents_size.height();
-
-  int x;
-  if (BubbleBorder::is_arrow_on_left(anchor_position_))
-    x = relative_rect.x();
-  else
-    // Note that if the arrow is on the right, that the x position of the popup
-    // is assigned so that the rightmost edge of the popup is aligned with the
-    // rightmost edge of the relative region.
-    x = relative_rect.right() - contents_size.width();
-
-  return gfx::Rect(x, y, contents_size.width(), contents_size.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.
+  contents_size.Enlarge(2 * kPopupBubbleCornerRadius,
+                        2 * kPopupBubbleCornerRadius);
+  return border_->GetBounds(relative_rect, contents_size);
 }
 
 // static
@@ -377,9 +314,7 @@
     gfx::NativeWindow frame_window,
     const gfx::Rect& relative_to,
     BubbleBorder::ArrowLocation arrow_location,
-    bool activate_on_show,
     bool inspect_with_devtools,
-    PopupChrome chrome,
     Observer* observer) {
   DCHECK(profile);
   DCHECK(frame_window);
@@ -399,21 +334,14 @@
     return NULL;
 
   ExtensionHost* host = manager->CreatePopup(url, browser);
-  if (observer)
-    observer->ExtensionHostCreated(host);
-
   ExtensionPopup* popup = new ExtensionPopup(host, frame_widget, relative_to,
-                                             arrow_location, activate_on_show,
-                                             inspect_with_devtools, chrome,
-                                             observer);
+                                             arrow_location,
+                                             inspect_with_devtools, observer);
 
-  if (observer)
-    observer->ExtensionPopupCreated(popup);
-
   // If the host had somehow finished loading, then we'd miss the notification
   // and not show.  This seems to happen in single-process mode.
   if (host->did_stop_loading())
-    popup->Show(activate_on_show);
+    popup->Show(true);
 
   return popup;
 }
@@ -429,18 +357,3 @@
 
   Release();  // Balanced in ctor.
 }
-
-void ExtensionPopup::Release() {
-  bool final_release = instance_lifetime_->HasOneRef();
-  instance_lifetime_->Release();
-  if (final_release) {
-    DCHECK(closing_) << "ExtensionPopup to be destroyed before being closed.";
-    ExtensionPopup::Observer* observer = observer_;
-    delete this;
-
-    // |this| is passed only as a 'cookie'. The observer API explicitly takes a
-    // void* argument to emphasize this.
-    if (observer)
-      observer->ExtensionPopupClosed(this);
-  }
-}