Fix crash when a download is requested for an outdated plugin.

chrome/browser/plugins/plugin_installer.cc

 // Copyright (c) 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/plugins/plugin_installer.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/file_util.h"
 #include "base/process.h"
@@ skipping 19 matching lines @@
 
 using content::BrowserContext;
 using content::BrowserThread;
 using content::DownloadItem;
 using content::DownloadManager;
 using content::ResourceDispatcherHost;
 
 namespace {
 
 void BeginDownload(
-    const GURL& url,
+    GURL url,
     content::ResourceContext* resource_context,
     int render_process_host_id,
     int render_view_host_routing_id,
     const ResourceDispatcherHost::DownloadStartedCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   ResourceDispatcherHost* rdh = ResourceDispatcherHost::Get();
   scoped_ptr<net::URLRequest> request(
       resource_context->GetRequestContext()->CreateRequest(url, NULL));
   net::Error error = rdh->BeginDownload(
@@ skipping 71 matching lines @@
 
 void PluginInstaller::AddWeakObserver(WeakPluginInstallerObserver* observer) {
   weak_observers_.AddObserver(observer);
 }
 
 void PluginInstaller::RemoveWeakObserver(
     WeakPluginInstallerObserver* observer) {
   weak_observers_.RemoveObserver(observer);
 }
 
-void PluginInstaller::StartInstalling(bool url_for_display,
-                                      const GURL& plugin_url,
+void PluginInstaller::StartInstalling(GURL plugin_url,
                                       TabContents* tab_contents) {
   DCHECK_EQ(INSTALLER_STATE_IDLE, state_);
-  DCHECK(url_for_display);
   state_ = INSTALLER_STATE_DOWNLOADING;
   FOR_EACH_OBSERVER(PluginInstallerObserver, observers_, DownloadStarted());
   content::WebContents* web_contents = tab_contents->web_contents();
   DownloadManager* download_manager =
       BrowserContext::GetDownloadManager(tab_contents->profile());
   download_util::RecordDownloadSource(
       download_util::INITIATED_BY_PLUGIN_INSTALLER);
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&BeginDownload,
@@ skipping 20 matching lines @@
   // TODO(benjhayden): DCHECK(item && item->IsInProgress()) after figuring out
   // why DownloadStarted may get net:OK but an invalid id.
   if (!download_item) {
     DownloadError("Download not found");
     return;
   }
   download_item->SetOpenWhenComplete(true);
   download_item->AddObserver(this);
 }
 
-void PluginInstaller::OpenDownloadURL(bool url_for_display,
-                                      const GURL& plugin_url,
+void PluginInstaller::OpenDownloadURL(GURL plugin_url,

[ibraaaa, 2012/10/05 11:14:07]

Here a copy is made directly in |OpenURLParams()| but I passed it by value for
consistency.

[Bernhard Bauer, 2012/10/05 11:16:30]

Consistency with what? Note that we usually pass objects by const-reference.

[ibraaaa, 2012/10/05 11:23:36]

Well I thought this way. If a refactor is done to this method that involves a
change similar to the one that caused the bug above then this may lead to
another bug. You can always correct me.

On 2012/10/05 11:16:30, Bernhard Bauer wrote:

[Bernhard Bauer, 2012/10/05 12:01:57]

Wait, do we know that this fixes the crash? It seems like we're passing a
reference to something that has gone away, so we crash when trying to
dereference it for the first time (i.e. copying it into the base::Bind closure).
Passing it by value would only make it crash sooner in that case.

                                       content::WebContents* web_contents) {
   DCHECK_EQ(INSTALLER_STATE_IDLE, state_);
-  DCHECK(url_for_display);
   web_contents->OpenURL(content::OpenURLParams(
       plugin_url,
       content::Referrer(web_contents->GetURL(),
                         WebKit::WebReferrerPolicyDefault),
       NEW_FOREGROUND_TAB, content::PAGE_TRANSITION_TYPED, false));
   FOR_EACH_OBSERVER(PluginInstallerObserver, observers_, DownloadFinished());
 }
 
 void PluginInstaller::DownloadError(const std::string& msg) {
   DCHECK_EQ(INSTALLER_STATE_DOWNLOADING, state_);
   state_ = INSTALLER_STATE_IDLE;
   FOR_EACH_OBSERVER(PluginInstallerObserver, observers_, DownloadError(msg));
 }
 
 void PluginInstaller::DownloadCancelled() {
   DCHECK_EQ(INSTALLER_STATE_DOWNLOADING, state_);
   state_ = INSTALLER_STATE_IDLE;
   FOR_EACH_OBSERVER(PluginInstallerObserver, observers_, DownloadCancelled());
 }