Remove render process and WebContents notifications from background printing.

chrome/browser/printing/background_printing_manager.cc

Index: chrome/browser/printing/background_printing_manager.cc
diff --git a/chrome/browser/printing/background_printing_manager.cc b/chrome/browser/printing/background_printing_manager.cc
index f0aa96a0022fe09eb4226bdc1d77a630dac23c66..e4147e3e6216b33111555f0e929b4b09abf52827 100644
--- a/chrome/browser/printing/background_printing_manager.cc
+++ b/chrome/browser/printing/background_printing_manager.cc
@@ -14,12 +14,40 @@
 #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));
 }
@@ -27,9 +55,11 @@ BackgroundPrintingManager::BackgroundPrintingManager() {
 BackgroundPrintingManager::~BackgroundPrintingManager() {
   DCHECK(CalledOnValidThread());
   // The might be some WebContentses still in |printing_contents_set_| at this

[Lei Zhang, 2014/01/11 02:35:22]

|printing_contents_set_| -> |printing_contents_map_|

[Avi (use Gerrit), 2014/01/11 19:12:49]

Done.

-  // 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.
+  // 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,14 @@ void BackgroundPrintingManager::OwnPrintPreviewDialog(
   DCHECK(PrintPreviewDialogController::IsPrintPreviewDialog(preview_dialog));
   CHECK(!HasPrintPreviewDialog(preview_dialog));
 
-  printing_contents_set_.insert(preview_dialog);
+  Observer* observer = new Observer(this, preview_dialog);

[Lei Zhang, 2014/01/11 02:35:22]

nit: no need for the |observer| variable

[Avi (use Gerrit), 2014/01/11 19:12:49]

Done.

+  printing_contents_map_[preview_dialog] = observer;
 
-  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 +92,43 @@ 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();
+  if (!ContainsKey(printing_contents_map_, preview_contents)) {

[Lei Zhang, 2014/01/11 02:35:22]

If you save the iterator here instead, you can re-use the iterator on lines 111
and 112.

[Avi (use Gerrit), 2014/01/11 19:12:49]

Done.

+    // This is a reasonable case, hit when the release of the print job races
+    // deletion of the render process. In this case, everything is already torn
+    // down and 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 = printing_contents_map_[preview_contents];
+  printing_contents_map_.erase(preview_contents);
+  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