Fixed use-after-free in LoadCallback in bookmark_storage.cc

chrome/browser/bookmarks/chrome_bookmark_client.cc

Index: chrome/browser/bookmarks/chrome_bookmark_client.cc
diff --git a/chrome/browser/bookmarks/chrome_bookmark_client.cc b/chrome/browser/bookmarks/chrome_bookmark_client.cc
index 97740b742561f304db62d6546f4cf6b35d00c7e8..19aa1664d244951b7d54748ac2253f3d6c053a79 100644
--- a/chrome/browser/bookmarks/chrome_bookmark_client.cc
+++ b/chrome/browser/bookmarks/chrome_bookmark_client.cc
@@ -17,8 +17,6 @@
 #include "chrome/browser/policy/profile_policy_connector.h"
 #include "chrome/browser/policy/profile_policy_connector_factory.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/profiles/startup_task_runner_service.h"
-#include "chrome/browser/profiles/startup_task_runner_service_factory.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/browser/bookmark_node.h"
 #include "components/history/core/browser/url_database.h"
@@ -171,8 +169,6 @@ bookmarks::LoadExtraCallback ChromeBookmarkClient::GetLoadExtraNodesCallback() {
 
   return base::Bind(
       &ChromeBookmarkClient::LoadExtraNodes,
-      StartupTaskRunnerServiceFactory::GetForProfile(profile_)
-          ->GetBookmarkTaskRunner(),

[Joao da Silva, 2014/07/09 13:58:16]

This is what caused the leak:

- this callback is owned by BookmarkLoadDetails

- BookmarkLoadDetails is owned by the LoadCallback task, which is sent to this
task runner

- some tests delete their ProfileImpls before spinning the loops, so the Profile
that owns the StartupTaskRunnerService is deleted (and the service too), but now
the deferred_task_runner_ that was created by the service stays around because
it contains a task that references the runner itself

- that task never runs (and thus never releases its reference to the task
runner) because StartupTaskRunnerService::Start() is never called (this is
usually done by the profile_manager, but some tests create Profile directly and
bypass the manager)

       base::Passed(&managed),
       base::Passed(managed_bookmarks_tracker_->GetInitialManagedBookmarks()));
 }
@@ -238,11 +234,9 @@ void ChromeBookmarkClient::BookmarkModelLoaded(BookmarkModel* model,
 
 // static
 bookmarks::BookmarkPermanentNodeList ChromeBookmarkClient::LoadExtraNodes(
-    const scoped_refptr<base::DeferredSequencedTaskRunner>& profile_io_runner,
     scoped_ptr<BookmarkPermanentNode> managed_node,
     scoped_ptr<base::ListValue> initial_managed_bookmarks,
     int64* next_node_id) {
-  DCHECK(profile_io_runner->RunsTasksOnCurrentThread());
   // Load the initial contents of the |managed_node| now, and assign it an
   // unused ID.
   int64 managed_id = *next_node_id;