Revert 111252 - Switch MHTMLGenerationManager to use a Callback.

MHTMLGenerationManager now uses a callback instead of a notification.

BUG=None
TEST=MHTML generation (via extension API should still work).

Review URL: http://codereview.chromium.org/8566016

TBR=jcivelli@chromium.org
Review URL: http://codereview.chromium.org/8672001

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

6 files changed, 76 insertions, 52 deletions

diff --git a/chrome/browser/extensions/extension_save_page_api.cc b/chrome/browser/extensions/extension_save_page_api.cc
index 6a8bde6..e9c0386 100644
--- a/chrome/browser/extensions/extension_save_page_api.cc
+++ b/chrome/browser/extensions/extension_save_page_api.cc
@@ -4,7 +4,6 @@
 
 #include "chrome/browser/extensions/extension_save_page_api.h"
 
-#include "base/bind.h"
 #include "base/file_util.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
@@ -106,27 +105,46 @@ void SavePageAsMHTMLFunction::TemporaryFileCreated(bool success) {
     return;
   }
 
-  MHTMLGenerationManager::GenerateMHTMLCallback callback =
-      base::Bind(&SavePageAsMHTMLFunction::MHTMLGenerated, this);
-
+  registrar_.Add(
+      this, content::NOTIFICATION_MHTML_GENERATED,
+      content::Source<RenderViewHost>(tab_contents->render_view_host()));
+  // TODO(jcivelli): we should listen for navigation in the tab, tab closed,
+  //                 renderer died.
   g_browser_process->mhtml_generation_manager()->GenerateMHTML(
-      tab_contents, mhtml_path_, callback);
+      tab_contents, mhtml_path_);
 }
 
-void SavePageAsMHTMLFunction::MHTMLGenerated(const FilePath& file_path,
-                                             int64 mhtml_file_size) {
-  DCHECK(mhtml_path_ == file_path);
-  if (mhtml_file_size <= 0) {
+void SavePageAsMHTMLFunction::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  DCHECK(type == content::NOTIFICATION_MHTML_GENERATED);
+
+  const MHTMLGenerationManager::NotificationDetails* save_details =
+      content::Details<MHTMLGenerationManager::NotificationDetails>(details).
+          ptr();
+
+  if (mhtml_path_ != save_details->file_path) {
+    // This could happen if there are concurrent MHTML generations going on for
+    // the same tab.
+    LOG(WARNING) << "Received a notification that MHTML was generated but for a"
+        " different file.";
+    return;
+  }
+
+  registrar_.RemoveAll();
+
+  if (save_details->file_size <= 0) {
     ReturnFailure(kMHTMLGenerationFailedError);
     return;
   }
 
-  if (mhtml_file_size > std::numeric_limits<int>::max()) {
+  if (save_details->file_size > std::numeric_limits<int>::max()) {
     ReturnFailure(kFileTooBigError);
     return;
   }
 
-  ReturnSuccess(mhtml_file_size);
+  ReturnSuccess(save_details->file_size);
 }
 
 void SavePageAsMHTMLFunction::ReturnFailure(const std::string& error) {
diff --git a/chrome/browser/extensions/extension_save_page_api.h b/chrome/browser/extensions/extension_save_page_api.h
index fc7ce9a..106cd5a 100644
--- a/chrome/browser/extensions/extension_save_page_api.h
+++ b/chrome/browser/extensions/extension_save_page_api.h
@@ -10,11 +10,12 @@
 #include "base/memory/ref_counted.h"
 #include "chrome/browser/extensions/extension_function.h"
 #include "content/browser/tab_contents/tab_contents_observer.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
 #include "webkit/blob/deletable_file_reference.h"
 
-class FilePath;
-
-class SavePageAsMHTMLFunction : public AsyncExtensionFunction {
+class SavePageAsMHTMLFunction : public AsyncExtensionFunction,
+                                public content::NotificationObserver {
  public:
   SavePageAsMHTMLFunction();
 
@@ -30,6 +31,9 @@ class SavePageAsMHTMLFunction : public AsyncExtensionFunction {
  private:
   virtual ~SavePageAsMHTMLFunction();
   virtual bool RunImpl() OVERRIDE;
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
   virtual bool OnMessageReceivedFromRenderView(
       const IPC::Message& message) OVERRIDE;
 
@@ -41,9 +45,6 @@ class SavePageAsMHTMLFunction : public AsyncExtensionFunction {
   void ReturnFailure(const std::string& error);
   void ReturnSuccess(int64 file_size);
 
-  // Callback called once the MHTML generation is done.
-  void MHTMLGenerated(const FilePath& file_path, int64 mhtml_file_size);
-
   // Returns the TabContents we are associated with, NULL if it's been closed.
   TabContents* GetTabContents();
 
@@ -55,6 +56,8 @@ class SavePageAsMHTMLFunction : public AsyncExtensionFunction {
   // The file containing the MHTML.
   scoped_refptr<webkit_blob::DeletableFileReference> mhtml_file_;
 
+  content::NotificationRegistrar registrar_;
+
   DECLARE_EXTENSION_FUNCTION_NAME("experimental.savePage.saveAsMHTML")
 };
 
diff --git a/content/browser/download/mhtml_generation_browsertest.cc b/content/browser/download/mhtml_generation_browsertest.cc
index 4b81e35..8e19f19 100644
--- a/content/browser/download/mhtml_generation_browsertest.cc
+++ b/content/browser/download/mhtml_generation_browsertest.cc
@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/bind.h"
 #include "base/file_path.h"
 #include "base/scoped_temp_dir.h"
 #include "chrome/browser/ui/browser.h"
@@ -11,6 +10,7 @@
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/browser/download/mhtml_generation_manager.h"
 #include "content/browser/tab_contents/tab_contents.h"
+#include "content/public/browser/notification_types.h"
 #include "net/test/test_server.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -18,13 +18,7 @@ namespace {
 
 class MHTMLGenerationTest : public InProcessBrowserTest {
  public:
-  MHTMLGenerationTest() : mhtml_generated_(false), file_size_(0) {}
-
-  void MHTMLGenerated(const FilePath& path, int64 size) {
-    mhtml_generated_ = true;
-    file_size_ = size;
-    MessageLoopForUI::current()->Quit();
-  }
+  MHTMLGenerationTest() {}
 
  protected:
   virtual void SetUp() {
@@ -32,14 +26,7 @@ class MHTMLGenerationTest : public InProcessBrowserTest {
     InProcessBrowserTest::SetUp();
   }
 
-  bool mhtml_generated() const { return mhtml_generated_; }
-  int64 file_size() const { return file_size_; }
-
   ScopedTempDir temp_dir_;
-
- private:
-  bool mhtml_generated_;
-  int64 file_size_;
 };
 
 // Tests that generating a MHTML does create contents.
@@ -59,14 +46,16 @@ IN_PROC_BROWSER_TEST_F(MHTMLGenerationTest, GenerateMHTML) {
   MHTMLGenerationManager* mhtml_generation_manager =
       g_browser_process->mhtml_generation_manager();
 
-  mhtml_generation_manager->GenerateMHTML(tab, path,
-      base::Bind(&MHTMLGenerationTest::MHTMLGenerated, this));
-
-  // Block until the MHTML is generated.
-  ui_test_utils::RunMessageLoop();
+  content::Source<RenderViewHost> source(tab->render_view_host());
+  ui_test_utils::WindowedNotificationObserverWithDetails<
+      MHTMLGenerationManager::NotificationDetails> signal(
+          content::NOTIFICATION_MHTML_GENERATED, source);
+  mhtml_generation_manager->GenerateMHTML(tab, path);
+  signal.Wait();
 
-  EXPECT_TRUE(mhtml_generated());
-  EXPECT_GT(file_size(), 0);
+  MHTMLGenerationManager::NotificationDetails details;
+  ASSERT_TRUE(signal.GetDetailsFor(source.map_key(), &details));
+  ASSERT_GT(details.file_size, 0);
 
   // Make sure the actual generated file has some contents.
   int64 file_size;
diff --git a/content/browser/download/mhtml_generation_manager.cc b/content/browser/download/mhtml_generation_manager.cc
index 82948735..7a63009 100644
--- a/content/browser/download/mhtml_generation_manager.cc
+++ b/content/browser/download/mhtml_generation_manager.cc
@@ -29,8 +29,7 @@ MHTMLGenerationManager::~MHTMLGenerationManager() {
 }
 
 void MHTMLGenerationManager::GenerateMHTML(TabContents* tab_contents,
-    const FilePath& file,
-    const GenerateMHTMLCallback& callback) {
+                                           const FilePath& file) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   static int id_counter = 0;
 
@@ -39,7 +38,6 @@ void MHTMLGenerationManager::GenerateMHTML(TabContents* tab_contents,
   job.file_path = file;
   job.process_id = tab_contents->GetRenderProcessHost()->GetID();
   job.routing_id = tab_contents->render_view_host()->routing_id();
-  job.callback = callback;
   id_to_job_[job_id] = job;
 
   base::ProcessHandle renderer_process =
@@ -111,7 +109,18 @@ void MHTMLGenerationManager::JobFinished(int job_id, int64 file_size) {
   }
 
   Job& job = iter->second;
-  job.callback.Run(job.file_path, file_size);
+
+  RenderViewHost* rvh = RenderViewHost::FromID(job.process_id, job.routing_id);
+  if (rvh) {
+    NotificationDetails details;
+    details.file_path = job.file_path;
+    details.file_size = file_size;
+
+    content::NotificationService::current()->Notify(
+        content::NOTIFICATION_MHTML_GENERATED,
+        content::Source<RenderViewHost>(rvh),
+        content::Details<NotificationDetails>(&details));
+  }
 
   BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
       base::Bind(&MHTMLGenerationManager::CloseFile, this, job.browser_file));
diff --git a/content/browser/download/mhtml_generation_manager.h b/content/browser/download/mhtml_generation_manager.h
index 9c186d9..eeb6b04 100644
--- a/content/browser/download/mhtml_generation_manager.h
+++ b/content/browser/download/mhtml_generation_manager.h
@@ -24,20 +24,21 @@ class CONTENT_EXPORT MHTMLGenerationManager
   MHTMLGenerationManager();
   ~MHTMLGenerationManager();
 
-  typedef base::Callback<void(const FilePath& /* path to the MHTML file */,
-      int64 /* size of the file */)> GenerateMHTMLCallback;
-
   // Instructs the render view to generate a MHTML representation of the current
   // page for |tab_contents|.
-  void GenerateMHTML(TabContents* tab_contents,
-                     const FilePath& file,
-                     const GenerateMHTMLCallback& callback);
+  void GenerateMHTML(TabContents* tab_contents, const FilePath& file);
 
   // Notification from the renderer that the MHTML generation finished.
   // |mhtml_data_size| contains the size in bytes of the generated MHTML data,
   // or -1 in case of failure.
   void MHTMLGenerated(int job_id, int64 mhtml_data_size);
 
+  // The details sent along with the MHTML_GENERATED notification.
+  struct NotificationDetails {
+    FilePath file_path;
+    int64 file_size;
+  };
+
  private:
   struct Job{
     Job();
@@ -52,9 +53,6 @@ class CONTENT_EXPORT MHTMLGenerationManager
     // The IDs mapping to a specific tab.
     int process_id;
     int routing_id;
-
-    // The callback to call once generation is complete.
-    GenerateMHTMLCallback callback;
   };
 
   // Called on the file thread to create |file|.
diff --git a/content/public/browser/notification_types.h b/content/public/browser/notification_types.h
index 8acc092..67549aa 100644
--- a/content/public/browser/notification_types.h
+++ b/content/public/browser/notification_types.h
@@ -408,6 +408,13 @@ enum NotificationType {
   // in a Details<ChildProcessInfo>.
   NOTIFICATION_CHILD_INSTANCE_CREATED,
 
+  // Download Notifications --------------------------------------------------
+
+  // Sent when a page generation to MHTML has finished.
+  // The source is the corresponding RenderViewHost. The details is a
+  // MHTMLGenerationManager::NotificationDetails.
+  NOTIFICATION_MHTML_GENERATED,
+
   // Saved Pages -------------------------------------------------------------
 
   // Sent when a SavePackage finishes successfully. The source is the