Avoid having SavePackageFilePicker keep a reference to SavePackage, to avoid interfering with its lifetime.

BUG=82782
Review URL: http://codereview.chromium.org/7398003

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

9 files changed, 32 insertions, 27 deletions

diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index 7cce656..ca207f2 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -712,7 +712,7 @@ void ChromeContentBrowserClient::ClearCookies(RenderViewHost* rvh) {
 }
 
 void ChromeContentBrowserClient::ChooseSavePath(
-    SavePackage* save_package,
+    const base::WeakPtr<SavePackage>& save_package,
     const FilePath& suggested_path,
     bool can_save_as_complete) {
   // Deletes itself.
diff --git a/chrome/browser/chrome_content_browser_client.h b/chrome/browser/chrome_content_browser_client.h
index a55cfa6..01c808e 100644
--- a/chrome/browser/chrome_content_browser_client.h
+++ b/chrome/browser/chrome_content_browser_client.h
@@ -102,7 +102,7 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
   virtual void BrowserURLHandlerCreated(BrowserURLHandler* handler) OVERRIDE;
   virtual void ClearCache(RenderViewHost* rvh) OVERRIDE;
   virtual void ClearCookies(RenderViewHost* rvh) OVERRIDE;
-  virtual void ChooseSavePath(SavePackage* save_package,
+  virtual void ChooseSavePath(const base::WeakPtr<SavePackage>& save_package,
                               const FilePath& suggested_path,
                               bool can_save_as_complete) OVERRIDE;
 
diff --git a/chrome/browser/download/save_package_file_picker.cc b/chrome/browser/download/save_package_file_picker.cc
index 1c32aec..ee57adc 100644
--- a/chrome/browser/download/save_package_file_picker.cc
+++ b/chrome/browser/download/save_package_file_picker.cc
@@ -47,21 +47,13 @@ const int kIndexToIDS[] = {
 
 }
 
-SavePackageFilePicker::SavePackageFilePicker(SavePackage* save_package,
-                                             const FilePath& suggested_path,
-                                             bool can_save_as_complete)
+SavePackageFilePicker::SavePackageFilePicker(
+    const base::WeakPtr<SavePackage>& save_package,
+    const FilePath& suggested_path,
+    bool can_save_as_complete)
     : save_package_(save_package) {
-  // The TabContents which owns this SavePackage may have disappeared during
-  // the UI->FILE->UI thread hop of
-  // GetSaveInfo->CreateDirectoryOnFileThread->ContinueGetSaveInfo.
-  TabContents* tab_contents = save_package->tab_contents();
-  if (!tab_contents) {
-    delete this;
-    return;
-  }
-
-  DownloadPrefs* download_prefs =
-      tab_contents->profile()->GetDownloadManager()->download_prefs();
+  DownloadPrefs* download_prefs = save_package->tab_contents()->profile()->
+      GetDownloadManager()->download_prefs();
   int file_type_index = SavePackageTypeToIndex(
       static_cast<SavePackage::SavePackageType>(
           download_prefs->save_file_type()));
@@ -125,6 +117,7 @@ SavePackageFilePicker::SavePackageFilePicker(SavePackage* save_package,
 
   if (g_should_prompt_for_filename) {
     select_file_dialog_ = SelectFileDialog::Create(this);
+    TabContents* tab_contents = save_package_->tab_contents();
     select_file_dialog_->SelectFile(SelectFileDialog::SELECT_SAVEAS_FILE,
                                     string16(),
                                     suggested_path,
@@ -156,7 +149,8 @@ void SavePackageFilePicker::FileSelected(const FilePath& path,
   DCHECK(index >= kSelectFileHtmlOnlyIndex &&
          index <= kSelectFileCompleteIndex);
 
-  save_package_->OnPathPicked(path, kIndexToSaveType[index]);
+  if (save_package_)
+    save_package_->OnPathPicked(path, kIndexToSaveType[index]);
   delete this;
 }
 
diff --git a/chrome/browser/download/save_package_file_picker.h b/chrome/browser/download/save_package_file_picker.h
index db0da64..aa0c830 100644
--- a/chrome/browser/download/save_package_file_picker.h
+++ b/chrome/browser/download/save_package_file_picker.h
@@ -7,6 +7,7 @@
 #pragma once
 
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "chrome/browser/ui/shell_dialogs.h"
 
 class FilePath;
@@ -15,7 +16,7 @@ class SavePackage;
 // Handles showing a dialog to the user to ask for the filename to save a page.
 class SavePackageFilePicker : public SelectFileDialog::Listener {
  public:
-  SavePackageFilePicker(SavePackage* save_package,
+  SavePackageFilePicker(const base::WeakPtr<SavePackage>& save_package,
                         const FilePath& suggested_path,
                         bool can_save_as_complete);
   virtual ~SavePackageFilePicker();
@@ -29,7 +30,7 @@ class SavePackageFilePicker : public SelectFileDialog::Listener {
   virtual void FileSelected(const FilePath& path, int index, void* params);
   virtual void FileSelectionCanceled(void* params);
 
-  scoped_refptr<SavePackage> save_package_;
+  base::WeakPtr<SavePackage> save_package_;
 
   // For managing select file dialogs.
   scoped_refptr<SelectFileDialog> select_file_dialog_;
diff --git a/content/browser/content_browser_client.h b/content/browser/content_browser_client.h
index c5cedc2..3cd4729 100644
--- a/content/browser/content_browser_client.h
+++ b/content/browser/content_browser_client.h
@@ -9,6 +9,7 @@
 #include <string>
 
 #include "base/callback_old.h"
+#include "base/memory/weak_ptr.h"
 #include "content/common/content_client.h"
 #include "content/common/window_container_type.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebNotificationPresenter.h"
@@ -255,9 +256,9 @@ class ContentBrowserClient {
   // Clears browser cookies.
   virtual void ClearCookies(RenderViewHost* rvh) = 0;
 
-  // Asks the user for the path to save a page. The embedder calls
+  // Asks the user for the path to save a page. The embedder calls the tab's
   // SavePackage::OnPathPicked to give the answer.
-  virtual void ChooseSavePath(SavePackage* save_package,
+  virtual void ChooseSavePath(const base::WeakPtr<SavePackage>& save_package,
                               const FilePath& suggested_path,
                               bool can_save_as_complete) = 0;
 
diff --git a/content/browser/download/save_package.cc b/content/browser/download/save_package.cc
index 6420268..6c2d3ea 100644
--- a/content/browser/download/save_package.cc
+++ b/content/browser/download/save_package.cc
@@ -1263,8 +1263,14 @@ void SavePackage::CreateDirectoryOnFileThread(
 
 void SavePackage::ContinueGetSaveInfo(const FilePath& suggested_path,
                                       bool can_save_as_complete) {
+  // The TabContents which owns this SavePackage may have disappeared during
+  // the UI->FILE->UI thread hop of
+  // GetSaveInfo->CreateDirectoryOnFileThread->ContinueGetSaveInfo.
+  if (!tab_contents())
+    return;
+
   content::GetContentClient()->browser()->ChooseSavePath(
-      this, suggested_path, can_save_as_complete); 
+      AsWeakPtr(), suggested_path, can_save_as_complete);
 }
 
 // Called after the save file dialog box returns.
diff --git a/content/browser/download/save_package.h b/content/browser/download/save_package.h
index e346467..7a7e87b 100644
--- a/content/browser/download/save_package.h
+++ b/content/browser/download/save_package.h
@@ -15,6 +15,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/hash_tables.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "base/task.h"
 #include "content/browser/tab_contents/tab_contents_observer.h"
 #include "googleurl/src/gurl.h"
@@ -51,7 +52,8 @@ class Time;
 // by the SavePackage. SaveItems are created when a user initiates a page
 // saving job, and exist for the duration of one tab's life time.
 class SavePackage : public base::RefCountedThreadSafe<SavePackage>,
-                    public TabContentsObserver {
+                    public TabContentsObserver,
+                    public base::SupportsWeakPtr<SavePackage> {
  public:
   enum SavePackageType {
     // The value of the save type before its set by the user.
diff --git a/content/browser/mock_content_browser_client.cc b/content/browser/mock_content_browser_client.cc
index 0dbb5fc..2a41bbf 100644
--- a/content/browser/mock_content_browser_client.cc
+++ b/content/browser/mock_content_browser_client.cc
@@ -206,9 +206,10 @@ void MockContentBrowserClient::ClearCache(RenderViewHost* rvh) {
 void MockContentBrowserClient::ClearCookies(RenderViewHost* rvh) {
 }
 
-void MockContentBrowserClient::ChooseSavePath(SavePackage* save_package,
-                                              const FilePath& suggested_path,
-                                              bool can_save_as_complete) {
+void MockContentBrowserClient::ChooseSavePath(
+    const base::WeakPtr<SavePackage>& save_package,
+    const FilePath& suggested_path,
+    bool can_save_as_complete) {
 }
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX)
diff --git a/content/browser/mock_content_browser_client.h b/content/browser/mock_content_browser_client.h
index c3ccb2f..cbda376 100644
--- a/content/browser/mock_content_browser_client.h
+++ b/content/browser/mock_content_browser_client.h
@@ -102,7 +102,7 @@ class MockContentBrowserClient : public ContentBrowserClient {
   virtual void BrowserURLHandlerCreated(BrowserURLHandler* handler) OVERRIDE;
   virtual void ClearCache(RenderViewHost* rvh)  OVERRIDE;
   virtual void ClearCookies(RenderViewHost* rvh)  OVERRIDE;
-  virtual void ChooseSavePath(SavePackage* save_package,
+  virtual void ChooseSavePath(const base::WeakPtr<SavePackage>& save_package,
                               const FilePath& suggested_path,
                               bool can_save_as_complete) OVERRIDE;