Addition of 'maxSize' to experimental popup extension API.

chrome/browser/extensions/extension_popup_api.cc

Index: chrome/browser/extensions/extension_popup_api.cc
===================================================================
--- chrome/browser/extensions/extension_popup_api.cc	(revision 67311)
+++ chrome/browser/extensions/extension_popup_api.cc	(working copy)
@@ -57,6 +57,7 @@
 const char kGiveFocusKey[] = "giveFocus";
 const char kDomAnchorKey[] = "domAnchor";
 const char kBorderStyleKey[] = "borderStyle";
+const char kMaxSizeKey[] = "maxSize";
 
 // chrome enumeration values
 const char kRectangleChrome[] = "rectangle";
@@ -107,8 +108,9 @@
                            public base::RefCounted<ExtensionPopupHost>,
                            public NotificationObserver {
  public:
-  explicit ExtensionPopupHost(ExtensionFunctionDispatcher* dispatcher)
-      : dispatcher_(dispatcher), popup_(NULL) {
+  explicit ExtensionPopupHost(ExtensionFunctionDispatcher* dispatcher,
+                              const gfx::Size& max_popup_size)

[Jói, 2010/11/25 13:50:30]

would be good to document that setting a dimension in max_popup_size to 0 makes
the popup host use the default maximum.

[Jeff Timanus, 2010/11/25 23:35:30]

Good point.  Done.

+      : dispatcher_(dispatcher), popup_(NULL), max_popup_size_(max_popup_size) {
     AddRef();  // Balanced in DispatchPopupClosedEvent().
     views::FocusManager::GetWidgetFocusManager()->AddFocusChangeListener(this);
   }
@@ -177,6 +179,16 @@
     }
   }
 
+  virtual void ExtensionPopupCreated(ExtensionPopup* popup) {
+    // The popup has been created, but not yet displayed, so install the max
+    // size overrides before the first positioning.
+    if (max_popup_size_.width())
+      popup->set_max_width(max_popup_size_.width());
+
+    if (max_popup_size_.height())
+      popup->set_max_height(max_popup_size_.height());
+  }
+
   virtual void ExtensionPopupResized(ExtensionPopup* popup) {
     // Reposition the location of the arrow on the popup so that the popup
     // better fits on the working monitor.
@@ -216,16 +228,6 @@
     }
   }
 
-  virtual void DispatchPopupClosedEvent() {
-    if (dispatcher_) {
-      PopupEventRouter::OnPopupClosed(
-          dispatcher_->profile(),
-          dispatcher_->render_view_host()->routing_id());
-      dispatcher_ = NULL;
-    }
-    Release();  // Balanced in ctor.
-  }
-
   // Overridden from views::WidgetFocusChangeListener
   virtual void NativeFocusWillChange(gfx::NativeView focused_before,
                                      gfx::NativeView focused_now) {
@@ -323,6 +325,16 @@
     return delegate ? delegate->GetAutomationResourceRoutingDelegate() : NULL;
   }
 
+  void DispatchPopupClosedEvent() {
+    if (dispatcher_) {
+      PopupEventRouter::OnPopupClosed(
+          dispatcher_->profile(),
+          dispatcher_->render_view_host()->routing_id());
+      dispatcher_ = NULL;
+    }
+    Release();  // Balanced in ctor.
+  }
+
   // A pointer to the dispatcher that handled the request that opened this
   // popup view.
   ExtensionFunctionDispatcher* dispatcher_;
@@ -330,6 +342,9 @@
   // A pointer to the popup.
   ExtensionPopup* popup_;
 
+  // The maximal size to which the popup is permitted to expand.
+  gfx::Size max_popup_size_;
+
   NotificationRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionPopupHost);
@@ -406,6 +421,21 @@
                                                          &give_focus));
   }
 
+  int max_width = 0;
+  int max_height = 0;
+  if (show_details->HasKey(kMaxSizeKey)) {
+    DictionaryValue* max_size = NULL;
+    EXTENSION_FUNCTION_VALIDATE(show_details->GetDictionary(kMaxSizeKey,
+                                                            &max_size));
+
+    if (max_size->HasKey(kWidthKey))
+      EXTENSION_FUNCTION_VALIDATE(max_size->GetInteger(kWidthKey, &max_width));

[Jói, 2010/11/25 13:50:30]

Should perhaps validate (here and below) that the width/height are positive, and
within the minimum/maximum bounds described in the API definition (or hmm, I
guess that's already done somewhere based on the definition - ignore this
comment :)

[Jeff Timanus, 2010/11/25 23:35:30]

Yup!  I resisted the urge to double validate - I recalled that all parameters
are validated on the JS side, before the message is sent to the extension
function dispatcher.

+
+    if (max_size->HasKey(kHeightKey))
+      EXTENSION_FUNCTION_VALIDATE(max_size->GetInteger(kHeightKey,
+                                                       &max_height));
+  }
+
 #if defined(TOOLKIT_VIEWS)
   // The default behaviour is to provide the bubble-chrome to the popup.
   ExtensionPopup::PopupChrome chrome = ExtensionPopup::BUBBLE_CHROME;
@@ -456,7 +486,8 @@
   BubbleBorder::ArrowLocation arrow_location = BubbleBorder::TOP_LEFT;
 
   // ExtensionPopupHost manages it's own lifetime.
-  ExtensionPopupHost* popup_host = new ExtensionPopupHost(dispatcher());
+  ExtensionPopupHost* popup_host =
+      new ExtensionPopupHost(dispatcher(), gfx::Size(max_width, max_height));
   popup_ = ExtensionPopup::Show(url,
                                 GetCurrentBrowser(),
                                 dispatcher()->profile(),