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

components/web_modal/popup_manager.cc

Index: components/web_modal/popup_manager.cc
diff --git a/components/web_modal/popup_manager.cc b/components/web_modal/popup_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a74a336e0cc34577f9258ddc6e6dc5b5360ee75c
--- /dev/null
+++ b/components/web_modal/popup_manager.cc
@@ -0,0 +1,144 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/web_modal/popup_manager.h"
+
+#include "content/public/browser/navigation_details.h"
+#include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/web_contents.h"
+#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
+
+using content::WebContents;
+
+namespace web_modal {
+
+PopupManager::~PopupManager() {
+  DCHECK(child_dialogs_.empty());
+}
+
+void PopupManager::ShowPopup(scoped_ptr<SinglePopupManager> manager) {
+  child_dialogs_.push_back(new PopupState(manager->popup(), manager.Pass()));
+
+  if (child_dialogs_.size() == 1)
+    child_dialogs_.back()->manager->Show();
+}
+
+bool PopupManager::IsPopupActiveInCurrentWebContents() const {
+  return !child_dialogs_.empty();
+}
+
+void PopupManager::FocusTopmostPopup() {

[Finnur, 2014/05/16 12:38:14]

Topmost isn't clear. How about FocusNextPopup?

+  DCHECK(!child_dialogs_.empty());
+  child_dialogs_.front()->manager->Focus();
+}
+
+void PopupManager::WillClose(NativePopup popup) {
+  PopupList::iterator dlg = FindPopupState(popup);

[Finnur, 2014/05/16 12:38:14]

nit: Conflating dialogs and popups again. :)

+
+  // The Views tab contents modal dialog calls WillClose twice.  Ignore the
+  // second invocation.
+  if (dlg == child_dialogs_.end())
+    return;

[Finnur, 2014/05/16 12:38:14]

This not the first time I've seen code trying to deal with WillClose being sent
twice. I would have thought that to be a bug, no? 

(Yes, I know it is not your fault and that it is out of scope for this, I'm kind
of posing it as a question for the entire group, not you specifically).

[Mike Wittman, 2014/05/16 20:35:40]

This was my comment from investigating this for the WCMD case. As I recall the
problem is a mismatch between the views::DialogDelegate interface, which expects
Cancel/Accept to take action *without* closing the window, and the usage of the
TabModalConfirmDialog interface, which expects Cancel/Accept to close the
dialog.

This could be rationalized but it touches a lot of dialogs across both Views and
Cocoa.

+
+  bool removed_topmost_dialog = dlg == child_dialogs_.begin();
+  scoped_ptr<PopupState> deleter(*dlg);
+  child_dialogs_.erase(dlg);

[Finnur, 2014/05/16 12:38:14]

You don't erase in CloseAllPopups. Is that intended?

[Mike Wittman, 2014/05/16 20:35:40]

Speaking based on the WCMDM implementation, yes. All dialogs invoke this
function on closing and the dialog gets erased here, including those closed via
CloseAllPopups.

+  if (!child_dialogs_.empty() && removed_topmost_dialog &&
+      !closing_all_popups_) {
+    child_dialogs_.front()->manager->Show();
+  }
+}
+
+PopupManager::PopupManager()
+    : closing_all_popups_(false) {
+}
+
+PopupManager::PopupState::PopupState(
+    NativePopup popup,
+    scoped_ptr<SinglePopupManager> mgr)
+    : popup(popup),
+      manager(mgr.release()),

[Finnur, 2014/05/16 12:38:14]

Can this be .Pass() instead of .release()?

+#if defined(USE_AURA)
+      close_on_interstitial_webui(true)
+#else
+      // TODO(wittman): Test that closing on interstitial webui works properly
+      // on Mac and use the true default for all platforms.
+      close_on_interstitial_webui(false)
+#endif
+                                         {
+}
+
+PopupManager::PopupState::~PopupState() {}
+
+PopupManager::PopupList::iterator PopupManager::FindPopupState(
+    NativePopup popup) {
+  PopupList::iterator i;
+  for (i = child_dialogs_.begin(); i != child_dialogs_.end(); ++i) {
+    if ((*i)->popup == popup)
+      break;
+  }
+
+  return i;
+}
+
+void PopupManager::CloseAllPopups() {
+  closing_all_popups_ = true;
+
+  // Clear out any dialogs since we are leaving this page entirely.

[Finnur, 2014/05/16 12:38:14]

Will this function only be called when leaving a page?

Also, is it correct to close all popup types upon leaving the page? (not saying
it is not, just thinking out loud).

[Greg Billock, 2014/05/16 15:49:56]

I only really changed the header file to try to reflect the new language. My
expectation is this file will end up needing a lot of work to implement whatever
policy we put in it. I maybe should just strip off the .cc files for this cl,
since the intention is to focus more on making sure the API is something we're
happy with.

[Mike Wittman, 2014/05/16 20:35:40]

I believe this function would be WCMD-specific.

+  while (!child_dialogs_.empty()) {
+    child_dialogs_.front()->manager->Close();
+  }

[Finnur, 2014/05/16 12:38:14]

nit: No braces.

+
+  closing_all_popups_ = false;
+}
+
+void PopupManager::DidNavigateMainFrame(
+    content::WebContents* web_contents,
+    const content::LoadCommittedDetails& details,
+    const content::FrameNavigateParams& params) {
+  // Close constrained windows if necessary.

[Finnur, 2014/05/16 12:38:14]

I need to brush up on terminology. :) Are all popup types considered constrained
windows?

+  if (!net::registry_controlled_domains::SameDomainOrHost(
+          details.previous_url, details.entry->GetURL(),
+          net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES))
+    CloseAllPopups();

[Finnur, 2014/05/16 12:38:14]

Don't you need to check here to see whether this is the right WebContents?
Similar question -- is it right to close them all at this point?

[Mike Wittman, 2014/05/16 20:35:40]

This is also WMCD-specific, and probably doesn't belong here anyway... Not
closing on navigation to the same site is the right behavior for things like the
cookies and certificate information dialogs, but not for e.g. the confirm form
resubmission dialog.

+}
+
+void PopupManager::DidGetIgnoredUIEvent(content::WebContents* web_contents) {
+  if (!child_dialogs_.empty()) {
+    child_dialogs_.front()->manager->Focus();
+  }

[Finnur, 2014/05/16 12:38:14]

nit: No braces.

+}
+
+void PopupManager::WasShown(content::WebContents* web_contents) {
+  if (!child_dialogs_.empty())
+    child_dialogs_.front()->manager->Show();
+}
+
+void PopupManager::WasHidden(content::WebContents* web_contents) {
+  if (!child_dialogs_.empty())
+    child_dialogs_.front()->manager->Hide();
+}
+
+void PopupManager::WebContentsDestroyed(content::WebContents* web_contents) {
+  // First cleanly close all child dialogs.
+  // TODO(mpcomplete): handle case if MaybeCloseChildWindows() already asked
+  // some of these to close.  CloseAllDialogs is async, so it might get called

[Finnur, 2014/05/16 12:38:14]

CloseAllDialogs?

+  // twice before it runs.
+  CloseAllPopups();
+}
+
+void PopupManager::DidAttachInterstitialPage(
+    content::WebContents* web_contents) {
+  // Copy the dialogs so we can close and remove them while iterating over the
+  // list.
+  PopupList dialogs(child_dialogs_);
+  for (PopupList::iterator it = dialogs.begin();
+       it != dialogs.end(); ++it) {
+    if ((*it)->close_on_interstitial_webui)
+      (*it)->manager->Close();
+  }
+}
+
+}  // namespace web_modal