Remove render process and WebContents notifications from background printing.

BUG=170921
TEST=everything still works, bug 100806 doesn't regress

Review URL: https://codereview.chromium.org/133013002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@244391 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 80 insertions, 147 deletions

diff --git a/chrome/browser/printing/background_printing_manager.cc b/chrome/browser/printing/background_printing_manager.cc
index f0aa96a..e619fc9 100644
--- a/chrome/browser/printing/background_printing_manager.cc
+++ b/chrome/browser/printing/background_printing_manager.cc
@@ -14,22 +14,52 @@
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_delegate.h"
+#include "content/public/browser/web_contents_observer.h"
 
 using content::BrowserThread;
 using content::WebContents;
 
 namespace printing {
 
+class BackgroundPrintingManager::Observer
+    : public content::WebContentsObserver {
+ public:
+  Observer(BackgroundPrintingManager* manager, WebContents* web_contents);
+
+ private:
+  virtual void RenderProcessGone(base::TerminationStatus status) OVERRIDE;
+  virtual void WebContentsDestroyed(WebContents* web_contents) OVERRIDE;
+
+  BackgroundPrintingManager* manager_;
+};
+
+BackgroundPrintingManager::Observer::Observer(
+    BackgroundPrintingManager* manager, WebContents* web_contents)
+    : content::WebContentsObserver(web_contents),
+      manager_(manager) {
+}
+
+void BackgroundPrintingManager::Observer::RenderProcessGone(
+    base::TerminationStatus status) {
+  manager_->DeletePreviewContents(web_contents());
+}
+void BackgroundPrintingManager::Observer::WebContentsDestroyed(
+    WebContents* web_contents) {
+  manager_->DeletePreviewContents(web_contents);
+}
+
 BackgroundPrintingManager::BackgroundPrintingManager() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 }
 
 BackgroundPrintingManager::~BackgroundPrintingManager() {
   DCHECK(CalledOnValidThread());
-  // The might be some WebContentses still in |printing_contents_set_| at this
-  // point. E.g. when the last remaining tab closes and there is still a print
-  // preview WebContents trying to print. In which case it will fail to print.
+  // The might be some WebContentses still in |printing_contents_map_| at this
+  // point (e.g. when the last remaining tab closes and there is still a print
+  // preview WebContents trying to print). In such a case it will fail to print,
+  // but we should at least clean up the observers.
   // TODO(thestig): Handle this case better.
+  STLDeleteValues(&printing_contents_map_);
 }
 
 void BackgroundPrintingManager::OwnPrintPreviewDialog(
@@ -38,30 +68,13 @@ void BackgroundPrintingManager::OwnPrintPreviewDialog(
   DCHECK(PrintPreviewDialogController::IsPrintPreviewDialog(preview_dialog));
   CHECK(!HasPrintPreviewDialog(preview_dialog));
 
-  printing_contents_set_.insert(preview_dialog);
+  printing_contents_map_[preview_dialog] = new Observer(this, preview_dialog);
 
-  content::Source<WebContents> preview_source(preview_dialog);
-  registrar_.Add(this, chrome::NOTIFICATION_PRINT_JOB_RELEASED, preview_source);
-
-  // OwnInitiatorWebContents() may have already added this notification.
-  if (!registrar_.IsRegistered(this,
-      content::NOTIFICATION_WEB_CONTENTS_DESTROYED, preview_source)) {
-    registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
-                   preview_source);
-  }
-
-  // If a WebContents that is printing crashes, the user cannot destroy it since
-  // it is hidden. Thus listen for crashes here and delete it.
-  //
-  // Multiple sites may share the same RenderProcessHost, so check if this
-  // notification has already been added.
-  content::Source<content::RenderProcessHost> rph_source(
-      preview_dialog->GetRenderProcessHost());
-  if (!registrar_.IsRegistered(this,
-      content::NOTIFICATION_RENDERER_PROCESS_CLOSED, rph_source)) {
-    registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
-                   rph_source);
-  }
+  // Watch for print jobs finishing. Everything else is watched for by the
+  // Observer. TODO(avi, cait): finish the job of removing this last
+  // notification.
+  registrar_.Add(this, chrome::NOTIFICATION_PRINT_JOB_RELEASED,
+                 content::Source<WebContents>(preview_dialog));
 
   // Activate the initiator.
   PrintPreviewDialogController* dialog_controller =
@@ -78,110 +91,45 @@ void BackgroundPrintingManager::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
-  if (type == content::NOTIFICATION_RENDERER_PROCESS_CLOSED) {
-    OnRendererProcessClosed(
-        content::Source<content::RenderProcessHost>(source).ptr());
-  } else if (type == chrome::NOTIFICATION_PRINT_JOB_RELEASED) {
-    OnPrintJobReleased(content::Source<WebContents>(source).ptr());
-  } else {
-    DCHECK_EQ(content::NOTIFICATION_WEB_CONTENTS_DESTROYED, type);
-    OnWebContentsDestroyed(content::Source<WebContents>(source).ptr());
-  }
-}
-
-void BackgroundPrintingManager::OnRendererProcessClosed(
-    content::RenderProcessHost* rph) {
-  WebContentsSet printing_contents_pending_deletion_set;
-  WebContentsSet::const_iterator it;
-  for (it = begin(); it != end(); ++it) {
-    WebContents* preview_contents = *it;
-    if (preview_contents->GetRenderProcessHost() == rph) {
-      printing_contents_pending_deletion_set.insert(preview_contents);
-    }
-  }
-  for (it = printing_contents_pending_deletion_set.begin();
-       it != printing_contents_pending_deletion_set.end();
-       ++it) {
-    DeletePreviewContents(*it);
-  }
+  DCHECK_EQ(chrome::NOTIFICATION_PRINT_JOB_RELEASED, type);
+  DeletePreviewContents(content::Source<WebContents>(source).ptr());
 }
 
-void BackgroundPrintingManager::OnPrintJobReleased(
-    WebContents* preview_contents) {
-  DeletePreviewContents(preview_contents);
-}
-
-void BackgroundPrintingManager::OnWebContentsDestroyed(
+void BackgroundPrintingManager::DeletePreviewContents(
     WebContents* preview_contents) {
-  // Always need to remove this notification since the WebContents is gone.
-  content::Source<WebContents> preview_source(preview_contents);
-  registrar_.Remove(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
-                    preview_source);
-
-  if (!HasPrintPreviewDialog(preview_contents)) {
-    NOTREACHED();
+  WebContentsObserverMap::iterator i =
+      printing_contents_map_.find(preview_contents);
+  if (i == printing_contents_map_.end()) {
+    // Everyone is racing to be the first to delete the |preview_contents|. If
+    // this case is hit, someone else won the race, so there is no need to
+    // continue. <http://crbug.com/100806>
     return;
   }
 
-  // Remove NOTIFICATION_RENDERER_PROCESS_CLOSED if |preview_contents| is the
-  // last WebContents associated with |rph|.
-  bool shared_rph =
-      (HasSharedRenderProcessHost(printing_contents_set_, preview_contents) ||
-       HasSharedRenderProcessHost(printing_contents_pending_deletion_set_,
-                                  preview_contents));
-  if (!shared_rph) {
-    content::RenderProcessHost* rph = preview_contents->GetRenderProcessHost();
-    registrar_.Remove(this, content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
-                      content::Source<content::RenderProcessHost>(rph));
-  }
-
-  // Remove other notifications and remove the WebContents from its
-  // WebContentsSet.
-  if (printing_contents_set_.erase(preview_contents) == 1) {
-    registrar_.Remove(this, chrome::NOTIFICATION_PRINT_JOB_RELEASED,
-                      preview_source);
-  } else {
-    // DeletePreviewContents already deleted the notification.
-    printing_contents_pending_deletion_set_.erase(preview_contents);
-  }
-}
-
-void BackgroundPrintingManager::DeletePreviewContents(
-    WebContents* preview_contents) {
+  // Stop all observation ...
   registrar_.Remove(this, chrome::NOTIFICATION_PRINT_JOB_RELEASED,
                     content::Source<WebContents>(preview_contents));
-  printing_contents_set_.erase(preview_contents);
-  printing_contents_pending_deletion_set_.insert(preview_contents);
+  Observer* observer = i->second;
+  printing_contents_map_.erase(i);
+  delete observer;
+
+  // ... and mortally wound the contents. (Deletion immediately is not a good
+  // idea in case this was called from RenderViewGone.)
   base::MessageLoop::current()->DeleteSoon(FROM_HERE, preview_contents);
 }
 
-bool BackgroundPrintingManager::HasSharedRenderProcessHost(
-    const WebContentsSet& set, WebContents* preview_contents) {
-  content::RenderProcessHost* rph = preview_contents->GetRenderProcessHost();
-  for (WebContentsSet::const_iterator it = set.begin(); it != set.end(); ++it) {
-    WebContents* iter_contents = *it;
-    if (iter_contents == preview_contents)
-      continue;
-    if (iter_contents->GetRenderProcessHost() == rph)
-      return true;
+std::set<content::WebContents*> BackgroundPrintingManager::CurrentContentSet() {
+  std::set<content::WebContents*> result;
+  for (WebContentsObserverMap::iterator i = printing_contents_map_.begin();
+       i != printing_contents_map_.end(); ++i) {
+    result.insert(i->first);
   }
-  return false;
-}
-
-BackgroundPrintingManager::WebContentsSet::const_iterator
-    BackgroundPrintingManager::begin() {
-  return printing_contents_set_.begin();
-}
-
-BackgroundPrintingManager::WebContentsSet::const_iterator
-    BackgroundPrintingManager::end() {
-  return printing_contents_set_.end();
+  return result;
 }
 
 bool BackgroundPrintingManager::HasPrintPreviewDialog(
     WebContents* preview_dialog) {
-  return (ContainsKey(printing_contents_set_, preview_dialog) ||
-          ContainsKey(printing_contents_pending_deletion_set_, preview_dialog));
+  return ContainsKey(printing_contents_map_, preview_dialog);
 }
 
 }  // namespace printing
diff --git a/chrome/browser/printing/background_printing_manager.h b/chrome/browser/printing/background_printing_manager.h
index 4987152..bb3414e 100644
--- a/chrome/browser/printing/background_printing_manager.h
+++ b/chrome/browser/printing/background_printing_manager.h
@@ -27,7 +27,8 @@ namespace printing {
 class BackgroundPrintingManager : public base::NonThreadSafe,
                                   public content::NotificationObserver {
  public:
-  typedef std::set<content::WebContents*> WebContentsSet;
+  class Observer;
+  typedef std::map<content::WebContents*, Observer*> WebContentsObserverMap;
 
   BackgroundPrintingManager();
   virtual ~BackgroundPrintingManager();
@@ -37,12 +38,11 @@ class BackgroundPrintingManager : public base::NonThreadSafe,
   // and hides it from the user.
   void OwnPrintPreviewDialog(content::WebContents* preview_dialog);
 
-  // Returns true if |printing_contents_set_| contains |preview_dialog|.
+  // Returns true if |printing_contents_map_| contains |preview_dialog|.
   bool HasPrintPreviewDialog(content::WebContents* preview_dialog);
 
-  // Let others iterate over the list of background printing contents.
-  WebContentsSet::const_iterator begin();
-  WebContentsSet::const_iterator end();
+  // Let others see the list of background printing contents.
+  std::set<content::WebContents*> CurrentContentSet();
 
  private:
   // content::NotificationObserver overrides:
@@ -50,26 +50,12 @@ class BackgroundPrintingManager : public base::NonThreadSafe,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
-  // Notifications handlers.
-  void OnRendererProcessClosed(content::RenderProcessHost* rph);
-  void OnPrintJobReleased(content::WebContents* preview_contents);
-  void OnWebContentsDestroyed(content::WebContents* preview_contents);
-
-  // Add |preview_contents| to the pending deletion set and schedule deletion.
+  // Schedule deletion of |preview_contents|.
   void DeletePreviewContents(content::WebContents* preview_contents);
 
-  // Check if any of the WebContentses in |set| share a RenderProcessHost
-  // with |tab|, excluding |tab|.
-  bool HasSharedRenderProcessHost(const WebContentsSet& set,
-                                  content::WebContents* preview_contents);
-
-  // The set of print preview WebContentses managed by
-  // BackgroundPrintingManager.
-  WebContentsSet printing_contents_set_;
-
-  // The set of print preview Webcontents managed by BackgroundPrintingManager
-  // that are pending deletion.
-  WebContentsSet printing_contents_pending_deletion_set_;
+  // A map from print preview WebContentses (managed by
+  // BackgroundPrintingManager) to the Observers that observe them.
+  WebContentsObserverMap printing_contents_map_;
 
   content::NotificationRegistrar registrar_;
 
diff --git a/chrome/browser/task_manager/tab_contents_resource_provider.cc b/chrome/browser/task_manager/tab_contents_resource_provider.cc
index 9b79cc1..8bf392a 100644
--- a/chrome/browser/task_manager/tab_contents_resource_provider.cc
+++ b/chrome/browser/task_manager/tab_contents_resource_provider.cc
@@ -249,9 +249,11 @@ void TabContentsResourceProvider::StartUpdating() {
   // Add all the pages being background printed.
   printing::BackgroundPrintingManager* printing_manager =
       g_browser_process->background_printing_manager();
-  for (printing::BackgroundPrintingManager::WebContentsSet::iterator i =
-           printing_manager->begin();
-       i != printing_manager->end(); ++i) {
+  std::set<content::WebContents*> printing_contents =
+      printing_manager->CurrentContentSet();
+  for (std::set<content::WebContents*>::iterator i =
+           printing_contents.begin();
+       i != printing_contents.end(); ++i) {
     Add(*i);
   }
 #endif
diff --git a/chrome/browser/ui/webui/memory_internals/memory_internals_proxy.cc b/chrome/browser/ui/webui/memory_internals/memory_internals_proxy.cc
index 2c49eba..d30dcf8 100644
--- a/chrome/browser/ui/webui/memory_internals/memory_internals_proxy.cc
+++ b/chrome/browser/ui/webui/memory_internals/memory_internals_proxy.cc
@@ -107,8 +107,7 @@ void GetAllWebContents(std::set<content::WebContents*>* web_contents) {
       continue;
     const std::vector<content::WebContents*> contentses =
         prerender_manager->GetAllPrerenderingContents();
-    for (size_t j = 0; j < contentses.size(); ++j)
-      web_contents->insert(contentses[j]);
+    web_contents->insert(contentses.begin(), contentses.end());
   }
   // Add all the Instant Extended prerendered NTPs.
   for (size_t i = 0; i < profiles.size(); ++i) {
@@ -121,11 +120,9 @@ void GetAllWebContents(std::set<content::WebContents*>* web_contents) {
   // Add all the pages being background printed.
   printing::BackgroundPrintingManager* printing_manager =
       g_browser_process->background_printing_manager();
-  for (printing::BackgroundPrintingManager::WebContentsSet::const_iterator
-           iter = printing_manager->begin();
-       iter != printing_manager->end(); ++iter) {
-    web_contents->insert(*iter);
-  }
+  std::set<content::WebContents*> printing_contents =
+      printing_manager->CurrentContentSet();
+  web_contents->insert(printing_contents.begin(), printing_contents.end());
 #endif
 }