Fix URLFetcher leaks in ChromeToMobileService

chrome/browser/chrome_to_mobile_service.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/chrome_to_mobile_service.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/guid.h"
@@ skipping 234 matching lines @@
   data.mobile_os = (mobile_os.compare(kTypeAndroid) == 0) ? ANDROID : IOS;
   if (!mobile->GetString("id", &data.mobile_id))
     NOTREACHED();
   content::WebContents* web_contents = chrome::GetActiveWebContents(browser);
   data.url = web_contents->GetURL();
   data.title = web_contents->GetTitle();
   data.snapshot = snapshot;
   data.snapshot_id = base::GenerateGUID();
   data.type = !snapshot.empty() ? DELAYED_SNAPSHOT : URL;
 
-  net::URLFetcher* submit_url = CreateRequest();
-  request_observer_map_[submit_url] = observer;
-  SendRequest(submit_url, data);
+  submit_url_.reset(CreateRequest());

[msw, 2012/09/07 02:01:30]

This won't work, as there may be multiple simultaneous send requests. Can we
change |request_observer_map_| to be an std::map<scoped_ptr<net::URLFetcher>,
base::WeakPtr<Observer> > and let the requests be owned by the map?

+  request_observer_map_[submit_url_.get()] = observer;
+  SendRequest(submit_url_.get(), data);
 
   if (data.type == DELAYED_SNAPSHOT) {
     LogMetric(SENDING_SNAPSHOT);
 
     data.type = SNAPSHOT;
-    net::URLFetcher* submit_snapshot = CreateRequest();
-    request_observer_map_[submit_snapshot] = observer;
+    submit_snapshot_.reset(CreateRequest());
+    request_observer_map_[submit_snapshot_.get()] = observer;
     if (!content::BrowserThread::PostBlockingPoolSequencedTask(
             data.snapshot.AsUTF8Unsafe(), FROM_HERE,
             base::Bind(&ChromeToMobileService::SendRequest,
                        weak_ptr_factory_.GetWeakPtr(),
-                       submit_snapshot, data))) {
+                       submit_snapshot_.get(), data))) {
       NOTREACHED();
     }
   }
 }
 
 void ChromeToMobileService::DeleteSnapshot(const FilePath& snapshot) {
   DCHECK(snapshot.empty() || snapshots_.find(snapshot) != snapshots_.end());
   if (snapshots_.find(snapshot) != snapshots_.end()) {
     if (!snapshot.empty()) {
       if (!content::BrowserThread::PostBlockingPoolSequencedTask(
@@ skipping 224 matching lines @@
   if (access_token_.empty()) {
     // Enqueue this task to perform after obtaining an access token.
     task_queue_.push(base::Bind(&ChromeToMobileService::RequestDeviceSearch,
                                 weak_ptr_factory_.GetWeakPtr()));
     RequestAccessToken();
     return;
   }
 
   LogMetric(DEVICES_REQUESTED);
 
-  net::URLFetcher* search_request = net::URLFetcher::Create(
-      GetSearchURL(cloud_print_url_), net::URLFetcher::GET, this);
-  InitRequest(search_request);
-  search_request->Start();
+  search_request_.reset(net::URLFetcher::Create(

[msw, 2012/09/07 02:01:30]

Similarly, this won't work for multiple simultaneous search requests. Could we
instead maintain a ScopedVector<URLFetcher> for the search requests?

+      GetSearchURL(cloud_print_url_), net::URLFetcher::GET, this));
+  InitRequest(search_request_.get());
+  search_request_->Start();
 }
 
 void ChromeToMobileService::HandleSearchResponse(
     const net::URLFetcher* source) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK_EQ(source->GetURL(), GetSearchURL(cloud_print_url_));
 
   std::string data;
   ListValue* list = NULL;
   DictionaryValue* dictionary = NULL;
@@ skipping 64 matching lines @@
 
       // Report failure below and ignore the second response.
       request_observer_map_.erase(other);
       break;
     }
   }
 
   if (observer.get())
     observer->OnSendComplete(success);
 }