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

chrome/browser/ui/browser.cc

 // Copyright 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 "chrome/browser/ui/browser.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #include <shellapi.h>
 #endif  // defined(OS_WIN)
@@ skipping 134 matching lines @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/custom_handlers/protocol_handler.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/profiling.h"
 #include "chrome/common/search_types.h"
 #include "chrome/common/url_constants.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/browser/bookmark_utils.h"
 #include "components/google/core/browser/google_url_tracker.h"
 #include "components/startup_metric_utils/startup_metric_utils.h"
+#include "components/web_modal/popup_manager.h"
 #include "components/web_modal/web_contents_modal_dialog_manager.h"
 #include "content/public/browser/devtools_manager.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/download_manager.h"
 #include "content/public/browser/interstitial_page.h"
 #include "content/public/browser/invalidate_type.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
@@ skipping 272 matching lines @@
   // TODO(beng): move to ChromeBrowserMain:
   if (first_run::ShouldDoPersonalDataManagerFirstRun()) {
 #if defined(OS_WIN)
     // Notify PDM that this is a first run.
     ImportAutofillDataWin(
         autofill::PersonalDataManagerFactory::GetForProfile(profile_));
 #endif  // defined(OS_WIN)
   }
 
   fullscreen_controller_.reset(new FullscreenController(this));
+
+  // Must be initialized after window_.
+  // This would DCHECK on presence of the dialog host but tests
+  // may not implement, and currently Cocoa does not either.

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

Nit: I completely fail to understand this second sentence.  Are you trying to
say something like this?:

Ideally we would DCHECK that GetWebContentsModalDialogHost() returned non-NULL
here, but on Mac or in certain tests that will fail.

(And if you are, _why_ would we ideally DCHECK this?  How can it be important if
Mac doesn't do it?)

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

I can remove it if its puzzling. I found a DCHECK here to validate the ordering
to be a very inviting idea, and surprising that it won't work, hence the
comment.

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

I'm not sure you need to remove the comment, but it needs to spell things out
explicitly for readers like me who don't know why one might expect this function
to return non-NULL to begin with.  Part of why the current comment is confusing
is that it assumes the reader understands why one would want this DCHECK in the
first place.

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

Done.

+  popup_manager_.reset(new web_modal::PopupManager(
+      GetWebContentsModalDialogHost()));
 }
 
 Browser::~Browser() {
   // Stop observing notifications before continuing with destruction. Profile
   // destruction will unload extensions and reentrant calls to Browser:: should
   // be avoided while it is being torn down.
   registrar_.RemoveAll();
 
   // The tab strip should not have any tabs at this point.
   DCHECK(tab_strip_model_->empty());
@@ skipping 428 matching lines @@
   return OpenURLFromTab(NULL, params);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // Browser, TabStripModelObserver implementation:
 
 void Browser::TabInsertedAt(WebContents* contents,
                             int index,
                             bool foreground) {
   SetAsDelegate(contents, true);
+
+  if (popup_manager_.get())

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

Nit: Remove get() (not necessary to NULL-check a scoped_ptr anymore) (3 places)

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

Done.

+    popup_manager_->RegisterWith(contents);
+
   SessionTabHelper* session_tab_helper =
       SessionTabHelper::FromWebContents(contents);
   session_tab_helper->SetWindowID(session_id());
 
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_TAB_PARENTED,
       content::Source<content::WebContents>(contents),
       content::NotificationService::NoDetails());
 
   SyncHistoryWithTabs(index);
@@ skipping 21 matching lines @@
   fullscreen_controller_->OnTabClosing(contents);
   SessionService* session_service =
       SessionServiceFactory::GetForProfile(profile_);
   if (session_service)
     session_service->TabClosing(contents);
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_TAB_CLOSING,
       content::Source<NavigationController>(&contents->GetController()),
       content::NotificationService::NoDetails());
 
+  if (popup_manager_.get())
+    popup_manager_->UnregisterWith(contents);
+
   // Sever the WebContents' connection back to us.
   SetAsDelegate(contents, false);
 }
 
 void Browser::TabDetachedAt(WebContents* contents, int index) {
   // TabDetachedAt is called before TabStripModel has updated the
   // active index.
   int old_active_index = tab_strip_model_->active_index();
   if (index < old_active_index && !tab_strip_model_->closing_all()) {
     SessionService* session_service =
         SessionServiceFactory::GetForProfileIfExisting(profile_);
     if (session_service)
       session_service->SetSelectedTabInWindow(session_id(),
                                               old_active_index - 1);
   }
+
+  if (popup_manager_.get())
+    popup_manager_->UnregisterWith(contents);
+
   TabDetachedAtImpl(contents, index, DETACH_TYPE_DETACH);
 }
 
 void Browser::TabDeactivated(WebContents* contents) {
   fullscreen_controller_->OnTabDeactivated(contents);
   search_delegate_->OnTabDeactivated(contents);
   SearchTabHelper::FromWebContents(contents)->OnTabDeactivated();
 
   // Save what the user's currently typing, so it can be restored when we
   // switch back to this tab.
@@ skipping 1461 matching lines @@
   if (contents && !allow_js_access) {
     contents->web_contents()->GetController().LoadURL(
         target_url,
         content::Referrer(),
         content::PAGE_TRANSITION_LINK,
         std::string());  // No extra headers.
   }
 
   return contents != NULL;
 }