Support for chrome.experimental.popup API in ExternalTabContainer views

chrome/browser/tab_contents/tab_contents_delegate.h

Index: chrome/browser/tab_contents/tab_contents_delegate.h
===================================================================
--- chrome/browser/tab_contents/tab_contents_delegate.h	(revision 34039)
+++ chrome/browser/tab_contents/tab_contents_delegate.h	(working copy)
@@ -16,6 +16,7 @@
 #include "webkit/glue/context_menu.h"
 #include "webkit/glue/window_open_disposition.h"
 
+class Browser;
 class DownloadItem;
 class ExtensionFunctionDispatcher;
 class GURL;
@@ -251,6 +252,12 @@
                                        int32 page_id) {
   }
 
+  // Returns the browser in which the tab contents is being displayed.
+  virtual Browser* GetBrowser() { return NULL; }

[Ben Goodger (Google), 2010/06/03 23:21:18]

This is a layering violation and is just about the worst thing you can do in
Chrome code short of shooting a kitten.

We need to correct this since it looks like people (e.g. dhg@) are already
beginning to use this for other purposes.

TabContents should never rely on Browser, it is at a lower level of abstraction.

[Jeff Timanus, 2010/06/04 18:16:15]

Yikes!  And I love kittens!  (We should be able to resolve this issue - some
changes will be needed in the ExtensionPopup::Show and associated routines.)
I added this method because of the need for these parameters during display of
an ExtensionPopup view.  (See browser\views\extensions\extension_popup.{h|cc}) 
The experimental extension popup API only has access to an
ExtensionFunctionDispatcher, and needed a means to gain access to these values. 
See browser\extensions\extension_pop_api.cc.

The TabContentsDelegate is used to implement the
ExtensionFunctionDispatcher::Delegate interface in both ExtensionDOMUI, and
ExtensionHost.  This is because popups can be launched both from ExtensionHosts
(toolstrips), or from a TabContents view that has been navigated to an extension
url.

Looking at the use of the Browser in the construction of the ExtensionPopup, it
seems to be required during the construction of the ExtensionHost for use within
the popup.  See ExtensionProcessManager::CreateView.  Perhaps it would be
possible to decouple these functions from the browser, but further inspection
will be required.

The native window that frames the view that is launching the popup is used to
find the corresponding widget.  This is subsequently used during positioning of
the popup (which is a top-level window).  The widget is used to convert the
coordinates of the position of the popup, which are in the coordinate frame of
the frame to coordinates on the screen.

I hope that makes some sense.  Please let me know if you need more
clarification.  I was not aware, at the time, of the abstraction boundary
between the TabContentsDelegate and Browser classes.  

I'll investigate if there is a means to have the ExtensionFunctionDispatcher
(which I presume can know all about the Browser) provide a different access
point for the browser and framing window.

+
+  // Returns the widget framing the view containing the tab contents.
+  virtual gfx::NativeWindow GetFrameNativeWindow() { return NULL; }

[Ben Goodger (Google), 2010/06/03 23:21:18]

See the above comment. I am not clear on the need for there to be assumption in
the TabContents API of such a relationship (presence of a "frame native
window").

+
  protected:
   ~TabContentsDelegate() {}
 };