[WebModals] New API for browser-scoped popup management.

chrome/browser/ui/browser.h

Index: chrome/browser/ui/browser.h
diff --git a/chrome/browser/ui/browser.h b/chrome/browser/ui/browser.h
index d5e9d7299bb6a6e7c485cfcb253e50d12ff54a5d..220e28731ca7ba7d3cad6155d6dca988cb6b7409 100644
--- a/chrome/browser/ui/browser.h
+++ b/chrome/browser/ui/browser.h
@@ -91,6 +91,7 @@ class WebDialogDelegate;
 }
 
 namespace web_modal {
+class PopupManager;
 class WebContentsModalDialogHost;
 }
 
@@ -242,6 +243,9 @@ class Browser : public TabStripModelObserver,
     toolbar_model->swap(toolbar_model_);
   }
 #endif
+  web_modal::PopupManager* popup_manager() const {

[Peter Kasting, 2014/07/08 19:13:54]

Const functions should not return non-const pointers.  Either return a const
pointer or make the function non-const.

(I'm aware that other code in this file violates this rule, but I don't want to
require you to fix those cases before checking in.)

[Greg Billock, 2014/07/09 19:28:12]

browser_commands uses Browser in a const context when calling this, and the
accessor can't return a const object. Similar problems are probably why other
code ignores this rule as well.

[Peter Kasting, 2014/07/09 19:41:41]

Then browser_commands needs to use a non-const Browser.  If this is too big a
change to fix here, do it as a separate CL before this one.

[Greg Billock, 2014/07/10 18:32:03]

Done.

+    return popup_manager_.get();
+  }
   TabStripModel* tab_strip_model() const { return tab_strip_model_.get(); }
   chrome::BrowserCommandController* command_controller() {
     return command_controller_.get();
@@ -819,6 +823,10 @@ class Browser : public TabStripModelObserver,
   // This Browser's window.
   BrowserWindow* window_;
 
+  // Manages popup windows (bubbles, tab-modals) visible overlapping this
+  // window.

[Peter Kasting, 2014/07/08 19:13:54]

Nit: What's the "overlapping this window" part mean?  Are there other popup
windows that come from this browser that don't overlap the window and aren't
managed here?

[Greg Billock, 2014/07/09 19:28:12]

Potentially, yes -- JS popups, for instance, which are a whole different bag of
hurt.

[Peter Kasting, 2014/07/09 19:41:41]

I suggest clarifying this in your comment.

[Greg Billock, 2014/07/10 18:32:03]

Done.

+  scoped_ptr<web_modal::PopupManager> popup_manager_;
+
   scoped_ptr<TabStripModelDelegate> tab_strip_model_delegate_;
   scoped_ptr<TabStripModel> tab_strip_model_;