Use base::Passed(scoped_ptr) in policy/ code.

chrome/browser/policy/url_blacklist_manager.cc

 // Copyright (c) 2011 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/policy/url_blacklist_manager.h"
 
 #include "base/bind.h"
 #include "base/message_loop.h"
 #include "base/stl_util.h"
 #include "base/string_number_conversions.h"
@@ skipping 28 matching lines @@
   std::string s;
   for (base::ListValue::const_iterator it = list->begin();
        it != list->end() && vector->size() < kMaxFiltersPerPolicy; ++it) {
     if ((*it)->GetAsString(&s))
       vector->push_back(s);
   }
 
   return vector;
 }
 
-// A task that builds the blacklist on the FILE thread. Takes ownership
-// of |block| and |allow| but not of |blacklist|.
+// A task that builds the blacklist on the FILE thread.
 void BuildBlacklist(URLBlacklist* blacklist,
-                    StringVector* block,
-                    StringVector* allow) {
+                    scoped_ptr<StringVector> block,
+                    scoped_ptr<StringVector> allow) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-
-  scoped_ptr<StringVector> scoped_block(block);
-  scoped_ptr<StringVector> scoped_allow(allow);
-
   for (StringVector::iterator it = block->begin(); it != block->end(); ++it) {
     blacklist->Block(*it);
   }
   for (StringVector::iterator it = allow->begin(); it != allow->end(); ++it) {
     blacklist->Allow(*it);
   }
 }
 
-// A task that owns the URLBlacklist, and passes it to the URLBlacklistManager
-// on the IO thread, if the URLBlacklistManager still exists.
-void SetBlacklistOnIO(base::WeakPtr<URLBlacklistManager> blacklist_manager,
-                      URLBlacklist* blacklist) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  if (blacklist_manager) {
-    blacklist_manager->SetBlacklist(blacklist);
-  } else {
-    delete blacklist;
-  }
-}
-
 }  // namespace
 
 struct URLBlacklist::PathFilter {
   explicit PathFilter(const std::string& path, uint16 port, bool match)
       : path_prefix(path),
         port(port),
         blocked_schemes(0),
         allowed_schemes(0),
         match_subdomains(match) {}
 
@@ skipping 271 matching lines @@
   MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(&URLBlacklistManager::Update,
                  ui_weak_ptr_factory_.GetWeakPtr()));
 }
 
 void URLBlacklistManager::Update() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // The preferences can only be read on the UI thread.
-  StringVector* block =
-      ListValueToStringVector(pref_service_->GetList(prefs::kUrlBlacklist));
-  StringVector* allow =
-      ListValueToStringVector(pref_service_->GetList(prefs::kUrlWhitelist));
+  scoped_ptr<StringVector> block(
+      ListValueToStringVector(pref_service_->GetList(prefs::kUrlBlacklist)));
+  scoped_ptr<StringVector> allow(
+      ListValueToStringVector(pref_service_->GetList(prefs::kUrlWhitelist)));
 
   // Go through the IO thread to grab a WeakPtr to |this|. This is safe from
   // here, since this task will always execute before a potential deletion of
   // ProfileIOData on IO.
   BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
                           base::Bind(&URLBlacklistManager::UpdateOnIO,
-                                     base::Unretained(this), block, allow));
+                                     base::Unretained(this),
+                                     base::Passed(&block),
+                                     base::Passed(&allow)));
 }
 
-void URLBlacklistManager::UpdateOnIO(StringVector* block, StringVector* allow) {
+void URLBlacklistManager::UpdateOnIO(scoped_ptr<StringVector> block,
+                                     scoped_ptr<StringVector> allow) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  URLBlacklist* blacklist = new URLBlacklist;
+  scoped_ptr<URLBlacklist> blacklist(new URLBlacklist);
+  // Get a raw pointer now, since |blacklist.get()| might be evaluated after
+  // |Passed(&blacklist) below.
+  URLBlacklist* raw_blacklist = blacklist.get();

[Joao da Silva, 2011/12/10 14:11:54]

Only found this after the unit_tests crashed. Having "blacklist.get()" instead
of "raw_blacklist" in the PostTaskAndReply call was returning NULL, since
base::Passed() resets the scoped_ptr.

[awong, 2011/12/12 22:01:26]

Oh interesting.  I had not thought about that. Yeah, this seems like the right
workaround.

   // The URLBlacklist is built on the FILE thread. Once it's ready, it is passed
   // to the URLBlacklistManager on IO.
-  // |blacklist|, |block| and |allow| can leak on the unlikely event of a
-  // policy update and shutdown happening at the same time.
   BrowserThread::PostTaskAndReply(BrowserThread::FILE, FROM_HERE,
                                   base::Bind(&BuildBlacklist,
-                                             blacklist, block, allow),
-                                  base::Bind(&SetBlacklistOnIO,
+                                             raw_blacklist,
+                                             base::Passed(&block),
+                                             base::Passed(&allow)),
+                                  base::Bind(&URLBlacklistManager::SetBlacklist,
                                              io_weak_ptr_factory_.GetWeakPtr(),
-                                             blacklist));
+                                             base::Passed(&blacklist)));
 }
 
-void URLBlacklistManager::SetBlacklist(URLBlacklist* blacklist) {
+void URLBlacklistManager::SetBlacklist(scoped_ptr<URLBlacklist> blacklist) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  blacklist_.reset(blacklist);
+  blacklist_.swap(blacklist);
 }
 
 bool URLBlacklistManager::IsURLBlocked(const GURL& url) const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   return blacklist_->IsURLBlocked(url);
 }
 
 // static
 void URLBlacklistManager::RegisterPrefs(PrefService* pref_service) {
   pref_service->RegisterListPref(prefs::kUrlBlacklist,
                                  PrefService::UNSYNCABLE_PREF);
   pref_service->RegisterListPref(prefs::kUrlWhitelist,
                                  PrefService::UNSYNCABLE_PREF);
 }
 
 }  // namespace policy