Fix dangerous pointer use in UserScriptMaster

chrome/browser/extensions/user_script_master.cc

Index: chrome/browser/extensions/user_script_master.cc
diff --git a/chrome/browser/extensions/user_script_master.cc b/chrome/browser/extensions/user_script_master.cc
index c24dd6c6aac5069f3817f6c3a1896c3a98082d55..6807ef620442b3e926d3c09e1a45974dd896c154 100644
--- a/chrome/browser/extensions/user_script_master.cc
+++ b/chrome/browser/extensions/user_script_master.cc
@@ -10,6 +10,7 @@
 #include "base/bind_helpers.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
+#include "base/memory/shared_memory.h"
 #include "base/version.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -33,157 +34,17 @@ using extensions::ExtensionsBrowserClient;
 
 namespace extensions {
 
-// Helper function to parse greasesmonkey headers
-static bool GetDeclarationValue(const base::StringPiece& line,
-                                const base::StringPiece& prefix,
-                                std::string* value) {
-  base::StringPiece::size_type index = line.find(prefix);
-  if (index == base::StringPiece::npos)
-    return false;
+namespace {
 
-  std::string temp(line.data() + index + prefix.length(),
-                   line.length() - index - prefix.length());
-
-  if (temp.empty() || !IsWhitespace(temp[0]))
-    return false;
+typedef base::Callback<void(scoped_ptr<UserScriptList>,
+                            scoped_ptr<base::SharedMemory>)>
+                                LoadScriptsCallback;
 
-  base::TrimWhitespaceASCII(temp, base::TRIM_ALL, value);
-  return true;
-}
-
-UserScriptMaster::ScriptReloader::ScriptReloader(UserScriptMaster* master)
-    : master_(master) {
-  CHECK(BrowserThread::GetCurrentThreadIdentifier(&master_thread_id_));
-}
-
-// static
-bool UserScriptMaster::ScriptReloader::ParseMetadataHeader(

[Devlin, 2014/07/07 16:07:27]

This function has not changed, but Rietveld's bad at displaying "moves".

-      const base::StringPiece& script_text, UserScript* script) {
-  // http://wiki.greasespot.net/Metadata_block
-  base::StringPiece line;
-  size_t line_start = 0;
-  size_t line_end = line_start;
-  bool in_metadata = false;
-
-  static const base::StringPiece kUserScriptBegin("// ==UserScript==");
-  static const base::StringPiece kUserScriptEng("// ==/UserScript==");
-  static const base::StringPiece kNamespaceDeclaration("// @namespace");
-  static const base::StringPiece kNameDeclaration("// @name");
-  static const base::StringPiece kVersionDeclaration("// @version");
-  static const base::StringPiece kDescriptionDeclaration("// @description");
-  static const base::StringPiece kIncludeDeclaration("// @include");
-  static const base::StringPiece kExcludeDeclaration("// @exclude");
-  static const base::StringPiece kMatchDeclaration("// @match");
-  static const base::StringPiece kExcludeMatchDeclaration("// @exclude_match");
-  static const base::StringPiece kRunAtDeclaration("// @run-at");
-  static const base::StringPiece kRunAtDocumentStartValue("document-start");
-  static const base::StringPiece kRunAtDocumentEndValue("document-end");
-  static const base::StringPiece kRunAtDocumentIdleValue("document-idle");
-
-  while (line_start < script_text.length()) {
-    line_end = script_text.find('\n', line_start);
-
-    // Handle the case where there is no trailing newline in the file.
-    if (line_end == std::string::npos)
-      line_end = script_text.length() - 1;
-
-    line.set(script_text.data() + line_start, line_end - line_start);
-
-    if (!in_metadata) {
-      if (line.starts_with(kUserScriptBegin))
-        in_metadata = true;
-    } else {
-      if (line.starts_with(kUserScriptEng))
-        break;
-
-      std::string value;
-      if (GetDeclarationValue(line, kIncludeDeclaration, &value)) {
-        // We escape some characters that MatchPattern() considers special.
-        ReplaceSubstringsAfterOffset(&value, 0, "\\", "\\\\");
-        ReplaceSubstringsAfterOffset(&value, 0, "?", "\\?");
-        script->add_glob(value);
-      } else if (GetDeclarationValue(line, kExcludeDeclaration, &value)) {
-        ReplaceSubstringsAfterOffset(&value, 0, "\\", "\\\\");
-        ReplaceSubstringsAfterOffset(&value, 0, "?", "\\?");
-        script->add_exclude_glob(value);
-      } else if (GetDeclarationValue(line, kNamespaceDeclaration, &value)) {
-        script->set_name_space(value);
-      } else if (GetDeclarationValue(line, kNameDeclaration, &value)) {
-        script->set_name(value);
-      } else if (GetDeclarationValue(line, kVersionDeclaration, &value)) {
-        Version version(value);
-        if (version.IsValid())
-          script->set_version(version.GetString());
-      } else if (GetDeclarationValue(line, kDescriptionDeclaration, &value)) {
-        script->set_description(value);
-      } else if (GetDeclarationValue(line, kMatchDeclaration, &value)) {
-        URLPattern pattern(UserScript::ValidUserScriptSchemes());
-        if (URLPattern::PARSE_SUCCESS != pattern.Parse(value))
-          return false;
-        script->add_url_pattern(pattern);
-      } else if (GetDeclarationValue(line, kExcludeMatchDeclaration, &value)) {
-        URLPattern exclude(UserScript::ValidUserScriptSchemes());
-        if (URLPattern::PARSE_SUCCESS != exclude.Parse(value))
-          return false;
-        script->add_exclude_url_pattern(exclude);
-      } else if (GetDeclarationValue(line, kRunAtDeclaration, &value)) {
-        if (value == kRunAtDocumentStartValue)
-          script->set_run_location(UserScript::DOCUMENT_START);
-        else if (value == kRunAtDocumentEndValue)
-          script->set_run_location(UserScript::DOCUMENT_END);
-        else if (value == kRunAtDocumentIdleValue)
-          script->set_run_location(UserScript::DOCUMENT_IDLE);
-        else
-          return false;
-      }
-
-      // TODO(aa): Handle more types of metadata.
-    }
-
-    line_start = line_end + 1;
-  }
-
-  // If no patterns were specified, default to @include *. This is what
-  // Greasemonkey does.
-  if (script->globs().empty() && script->url_patterns().is_empty())
-    script->add_glob("*");
-
-  return true;
-}
-
-void UserScriptMaster::ScriptReloader::StartLoad(
-    const UserScriptList& user_scripts,
-    const ExtensionsInfo& extensions_info) {
-  // Add a reference to ourselves to keep ourselves alive while we're running.
-  // Balanced by NotifyMaster().
-  AddRef();
-
-  verifier_ = master_->content_verifier();
-  this->extensions_info_ = extensions_info;
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(
-          &UserScriptMaster::ScriptReloader::RunLoad, this, user_scripts));
-}
-
-UserScriptMaster::ScriptReloader::~ScriptReloader() {}
-
-void UserScriptMaster::ScriptReloader::NotifyMaster(
-    scoped_ptr<base::SharedMemory> memory) {
-  // The master could go away
-  if (master_)
-    master_->NewScriptsAvailable(memory.Pass());
-
-  // Drop our self-reference.
-  // Balances StartLoad().
-  Release();
-}
-
-static void VerifyContent(ContentVerifier* verifier,
-                          const std::string& extension_id,
-                          const base::FilePath& extension_root,
-                          const base::FilePath& relative_path,
-                          const std::string& content) {
+void VerifyContent(scoped_refptr<ContentVerifier> verifier,
+                   const std::string& extension_id,
+                   const base::FilePath& extension_root,
+                   const base::FilePath& relative_path,
+                   const std::string& content) {
   scoped_refptr<ContentVerifyJob> job(
       verifier->CreateJobFor(extension_id, extension_root, relative_path));
   if (job.get()) {
@@ -193,10 +54,10 @@ static void VerifyContent(ContentVerifier* verifier,
   }
 }
 
-static bool LoadScriptContent(const std::string& extension_id,
-                              UserScript::File* script_file,
-                              const SubstitutionMap* localization_messages,
-                              ContentVerifier* verifier) {
+bool LoadScriptContent(const std::string& extension_id,
+                       UserScript::File* script_file,
+                       const SubstitutionMap* localization_messages,
+                       scoped_refptr<ContentVerifier> verifier) {
   std::string content;
   const base::FilePath& path = ExtensionResource::GetFilePath(
       script_file->extension_root(), script_file->relative_path(),
@@ -250,17 +111,28 @@ static bool LoadScriptContent(const std::string& extension_id,
   return true;
 }
 
-void UserScriptMaster::ScriptReloader::LoadUserScripts(
-    UserScriptList* user_scripts) {
+SubstitutionMap* GetLocalizationMessages(const ExtensionsInfo& extensions_info,
+                                         const std::string& extension_id) {
+  ExtensionsInfo::const_iterator iter = extensions_info.find(extension_id);
+  if (iter == extensions_info.end())
+    return NULL;
+  return file_util::LoadMessageBundleSubstitutionMap(iter->second.first,
+                                                     extension_id,
+                                                     iter->second.second);
+}
+
+void LoadUserScripts(UserScriptList* user_scripts,
+                     const ExtensionsInfo& extensions_info,
+                     ContentVerifier* verifier) {
   for (size_t i = 0; i < user_scripts->size(); ++i) {
     UserScript& script = user_scripts->at(i);
     scoped_ptr<SubstitutionMap> localization_messages(
-        GetLocalizationMessages(script.extension_id()));
+        GetLocalizationMessages(extensions_info, script.extension_id()));
     for (size_t k = 0; k < script.js_scripts().size(); ++k) {
       UserScript::File& script_file = script.js_scripts()[k];
       if (script_file.GetContent().empty())
         LoadScriptContent(
-            script.extension_id(), &script_file, NULL, verifier_.get());
+            script.extension_id(), &script_file, NULL, verifier);
     }
     for (size_t k = 0; k < script.css_scripts().size(); ++k) {
       UserScript::File& script_file = script.css_scripts()[k];
@@ -268,25 +140,13 @@ void UserScriptMaster::ScriptReloader::LoadUserScripts(
         LoadScriptContent(script.extension_id(),
                           &script_file,
                           localization_messages.get(),
-                          verifier_.get());
+                          verifier);
     }
   }
 }
 
-SubstitutionMap* UserScriptMaster::ScriptReloader::GetLocalizationMessages(
-    const std::string& extension_id) {
-  if (extensions_info_.find(extension_id) == extensions_info_.end()) {
-    return NULL;
-  }
-
-  return file_util::LoadMessageBundleSubstitutionMap(
-      extensions_info_[extension_id].first,
-      extension_id,
-      extensions_info_[extension_id].second);
-}
-
 // Pickle user scripts and return pointer to the shared memory.
-static scoped_ptr<base::SharedMemory> Serialize(const UserScriptList& scripts) {
+scoped_ptr<base::SharedMemory> Serialize(const UserScriptList& scripts) {
   Pickle pickle;
   pickle.WriteUInt64(scripts.size());
   for (size_t i = 0; i < scripts.size(); i++) {
@@ -331,26 +191,150 @@ static scoped_ptr<base::SharedMemory> Serialize(const UserScriptList& scripts) {
                                                 /*read_only=*/true));
 }
 
-// This method will be called on the file thread.
-void UserScriptMaster::ScriptReloader::RunLoad(
-    const UserScriptList& user_scripts) {
-  LoadUserScripts(const_cast<UserScriptList*>(&user_scripts));
-
-  // Scripts now contains list of up-to-date scripts. Load the content in the
-  // shared memory and let the master know it's ready. We need to post the task
-  // back even if no scripts ware found to balance the AddRef/Release calls.
-  BrowserThread::PostTask(master_thread_id_,
-                          FROM_HERE,
-                          base::Bind(&ScriptReloader::NotifyMaster,
-                                     this,
-                                     base::Passed(Serialize(user_scripts))));
+void LoadScriptsOnFileThread(scoped_ptr<UserScriptList> user_scripts,
+                             const ExtensionsInfo& extensions_info,
+                             scoped_refptr<ContentVerifier> verifier,
+                             LoadScriptsCallback callback) {
+  DCHECK(user_scripts.get());
+  LoadUserScripts(user_scripts.get(), extensions_info, verifier);
+  scoped_ptr<base::SharedMemory> memory = Serialize(*user_scripts);
+  BrowserThread::PostTask(
+      BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(callback,
+                 base::Passed(&user_scripts),
+                 base::Passed(&memory)));
+}
+
+// Helper function to parse greasesmonkey headers
+bool GetDeclarationValue(const base::StringPiece& line,
+                         const base::StringPiece& prefix,
+                         std::string* value) {
+  base::StringPiece::size_type index = line.find(prefix);
+  if (index == base::StringPiece::npos)
+    return false;
+
+  std::string temp(line.data() + index + prefix.length(),
+                   line.length() - index - prefix.length());
+
+  if (temp.empty() || !IsWhitespace(temp[0]))
+    return false;
+
+  base::TrimWhitespaceASCII(temp, base::TRIM_ALL, value);
+  return true;
+}
+
+}  // namespace
+
+// static
+bool UserScriptMaster::ParseMetadataHeader(
+      const base::StringPiece& script_text, UserScript* script) {
+  // http://wiki.greasespot.net/Metadata_block
+  base::StringPiece line;
+  size_t line_start = 0;
+  size_t line_end = line_start;
+  bool in_metadata = false;
+
+  static const base::StringPiece kUserScriptBegin("// ==UserScript==");
+  static const base::StringPiece kUserScriptEng("// ==/UserScript==");
+  static const base::StringPiece kNamespaceDeclaration("// @namespace");
+  static const base::StringPiece kNameDeclaration("// @name");
+  static const base::StringPiece kVersionDeclaration("// @version");
+  static const base::StringPiece kDescriptionDeclaration("// @description");
+  static const base::StringPiece kIncludeDeclaration("// @include");
+  static const base::StringPiece kExcludeDeclaration("// @exclude");
+  static const base::StringPiece kMatchDeclaration("// @match");
+  static const base::StringPiece kExcludeMatchDeclaration("// @exclude_match");
+  static const base::StringPiece kRunAtDeclaration("// @run-at");
+  static const base::StringPiece kRunAtDocumentStartValue("document-start");
+  static const base::StringPiece kRunAtDocumentEndValue("document-end");
+  static const base::StringPiece kRunAtDocumentIdleValue("document-idle");
+
+  while (line_start < script_text.length()) {
+    line_end = script_text.find('\n', line_start);
+
+    // Handle the case where there is no trailing newline in the file.
+    if (line_end == std::string::npos)
+      line_end = script_text.length() - 1;
+
+    line.set(script_text.data() + line_start, line_end - line_start);
+
+    if (!in_metadata) {
+      if (line.starts_with(kUserScriptBegin))
+        in_metadata = true;
+    } else {
+      if (line.starts_with(kUserScriptEng))
+        break;
+
+      std::string value;
+      if (GetDeclarationValue(line, kIncludeDeclaration, &value)) {
+        // We escape some characters that MatchPattern() considers special.
+        ReplaceSubstringsAfterOffset(&value, 0, "\\", "\\\\");
+        ReplaceSubstringsAfterOffset(&value, 0, "?", "\\?");
+        script->add_glob(value);
+      } else if (GetDeclarationValue(line, kExcludeDeclaration, &value)) {
+        ReplaceSubstringsAfterOffset(&value, 0, "\\", "\\\\");
+        ReplaceSubstringsAfterOffset(&value, 0, "?", "\\?");
+        script->add_exclude_glob(value);
+      } else if (GetDeclarationValue(line, kNamespaceDeclaration, &value)) {
+        script->set_name_space(value);
+      } else if (GetDeclarationValue(line, kNameDeclaration, &value)) {
+        script->set_name(value);
+      } else if (GetDeclarationValue(line, kVersionDeclaration, &value)) {
+        Version version(value);
+        if (version.IsValid())
+          script->set_version(version.GetString());
+      } else if (GetDeclarationValue(line, kDescriptionDeclaration, &value)) {
+        script->set_description(value);
+      } else if (GetDeclarationValue(line, kMatchDeclaration, &value)) {
+        URLPattern pattern(UserScript::ValidUserScriptSchemes());
+        if (URLPattern::PARSE_SUCCESS != pattern.Parse(value))
+          return false;
+        script->add_url_pattern(pattern);
+      } else if (GetDeclarationValue(line, kExcludeMatchDeclaration, &value)) {
+        URLPattern exclude(UserScript::ValidUserScriptSchemes());
+        if (URLPattern::PARSE_SUCCESS != exclude.Parse(value))
+          return false;
+        script->add_exclude_url_pattern(exclude);
+      } else if (GetDeclarationValue(line, kRunAtDeclaration, &value)) {
+        if (value == kRunAtDocumentStartValue)
+          script->set_run_location(UserScript::DOCUMENT_START);
+        else if (value == kRunAtDocumentEndValue)
+          script->set_run_location(UserScript::DOCUMENT_END);
+        else if (value == kRunAtDocumentIdleValue)
+          script->set_run_location(UserScript::DOCUMENT_IDLE);
+        else
+          return false;
+      }
+
+      // TODO(aa): Handle more types of metadata.
+    }
+
+    line_start = line_end + 1;
+  }
+
+  // If no patterns were specified, default to @include *. This is what
+  // Greasemonkey does.
+  if (script->globs().empty() && script->url_patterns().is_empty())
+    script->add_glob("*");
+
+  return true;
+}
+
+// static
+void UserScriptMaster::LoadScriptsForTest(UserScriptList* user_scripts) {
+  ExtensionsInfo info;
+  LoadUserScripts(user_scripts, info, NULL /* no verifier for testing */);
 }
 
 UserScriptMaster::UserScriptMaster(Profile* profile)
-    : extensions_service_ready_(false),
+    : user_scripts_(new UserScriptList()),
+      extensions_service_ready_(false),
       pending_load_(false),
+      is_loading_(false),
       profile_(profile),
-      extension_registry_observer_(this) {
+      extension_registry_observer_(this),
+      weak_factory_(this) {
   extension_registry_observer_.Add(ExtensionRegistry::Get(profile_));
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSIONS_READY,
                  content::Source<Profile>(profile_));
@@ -359,55 +343,50 @@ UserScriptMaster::UserScriptMaster(Profile* profile)
 }
 
 UserScriptMaster::~UserScriptMaster() {
-  if (script_reloader_.get())
-    script_reloader_->DisownMaster();
 }
 
-void UserScriptMaster::NewScriptsAvailable(
-    scoped_ptr<base::SharedMemory> handle) {
+void UserScriptMaster::OnScriptsLoaded(
+    scoped_ptr<UserScriptList> user_scripts,
+    scoped_ptr<base::SharedMemory> shared_memory) {
+  is_loading_ = false;
+
   if (pending_load_) {
-    // While we were loading, there were further changes.  Don't bother
+    // While we were loading, there were further changes. Don't bother
     // notifying about these scripts and instead just immediately reload.
     pending_load_ = false;
     StartLoad();
-  } else {
-    // We're no longer loading.
-    script_reloader_ = NULL;
-
-    if (handle == NULL) {
-      // This can happen if we run out of file descriptors.  In that case, we
-      // have a choice between silently omitting all user scripts for new tabs,
-      // by nulling out shared_memory_, or only silently omitting new ones by
-      // leaving the existing object in place. The second seems less bad, even
-      // though it removes the possibility that freeing the shared memory block
-      // would open up enough FDs for long enough for a retry to succeed.
-
-      // Pretend the extension change didn't happen.
-      return;
-    }
+    return;
+  }
 
-    // We've got scripts ready to go.
-    shared_memory_ = handle.Pass();
+  if (shared_memory.get() == NULL) {
+    // This can happen if we run out of file descriptors.  In that case, we
+    // have a choice between silently omitting all user scripts for new tabs,
+    // by nulling out shared_memory_, or only silently omitting new ones by
+    // leaving the existing object in place. The second seems less bad, even
+    // though it removes the possibility that freeing the shared memory block
+    // would open up enough FDs for long enough for a retry to succeed.
 
-    for (content::RenderProcessHost::iterator i(
-            content::RenderProcessHost::AllHostsIterator());
-         !i.IsAtEnd(); i.Advance()) {
-      SendUpdate(i.GetCurrentValue(),
-                 shared_memory_.get(),
-                 changed_extensions_);
-    }
-    changed_extensions_.clear();
+    // Pretend the extension change didn't happen.
+    return;
+  }
+
+  // We've got scripts ready to go.
+  shared_memory_.reset(shared_memory.release());
+  user_scripts_.reset(user_scripts.release());

[asargent_no_longer_on_chrome, 2014/07/08 20:59:13]

In the CL as is, you're passing a copy of user_scripts_ to
LoadScriptsOnFileThread inside StartLoad, and then take that copy back here and
replace the original contents of user_scripts_ with it. Can we just pass
ownership over to the file thread instead of making the copy? I guess you'd need
to slightly rework the OnExtensionLoaded / OnExtensionUnloaded so that instead
of modifying user_scripts_, you instead kept track of UserScript objects to add
to / remove from the list that will be coming back from the file thread. 

[Devlin, 2014/07/09 16:03:25]

Yeah, I was trying to keep the underlying mechanism relatively similar for
safety.  But let's see what happens if we tweak it a bit. =)

As you mentioned, this required updating the OnExtension[Un]loaded to track
which extensions were added/removed.  On the plus side, this allows for an extra
implementation speed-up - we can now only load the _new_ extensions, instead of
reloading them all.  An extra pair of eyes to make sure this is all still sound
would be appreciated. :)

 
-    content::NotificationService::current()->Notify(
-        chrome::NOTIFICATION_USER_SCRIPTS_UPDATED,
-        content::Source<Profile>(profile_),
-        content::Details<base::SharedMemory>(shared_memory_.get()));
+  for (content::RenderProcessHost::iterator i(
+          content::RenderProcessHost::AllHostsIterator());
+       !i.IsAtEnd(); i.Advance()) {
+    SendUpdate(i.GetCurrentValue(),
+               shared_memory_.get(),
+               changed_extensions_);
   }
-}
+  changed_extensions_.clear();
 
-ContentVerifier* UserScriptMaster::content_verifier() {
-  ExtensionSystem* system = ExtensionSystem::Get(profile_);
-  return system->content_verifier();
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_USER_SCRIPTS_UPDATED,
+      content::Source<Profile>(profile_),
+      content::Details<base::SharedMemory>(shared_memory_.get()));
 }
 
 void UserScriptMaster::OnExtensionLoaded(
@@ -423,16 +402,15 @@ void UserScriptMaster::OnExtensionLoaded(
   for (UserScriptList::const_iterator iter = scripts.begin();
        iter != scripts.end();
        ++iter) {
-    user_scripts_.push_back(*iter);
-    user_scripts_.back().set_incognito_enabled(incognito_enabled);
+    user_scripts_->push_back(*iter);
+    user_scripts_->back().set_incognito_enabled(incognito_enabled);
   }
   if (extensions_service_ready_) {
     changed_extensions_.insert(extension->id());
-    if (script_reloader_.get()) {
+    if (is_loading_)
       pending_load_ = true;
-    } else {
+    else
       StartLoad();
-    }
   }
 }
 
@@ -442,20 +420,18 @@ void UserScriptMaster::OnExtensionUnloaded(
     UnloadedExtensionInfo::Reason reason) {
   // Remove any content scripts.
   extensions_info_.erase(extension->id());
-  UserScriptList new_user_scripts;
-  for (UserScriptList::iterator iter = user_scripts_.begin();
-       iter != user_scripts_.end();
-       ++iter) {
-    if (iter->extension_id() != extension->id())
-      new_user_scripts.push_back(*iter);
+  for (UserScriptList::iterator iter = user_scripts_->begin();
+       iter != user_scripts_->end();) {
+    if (iter->extension_id() == extension->id())
+      iter = user_scripts_->erase(iter);
+    else
+      ++iter;
   }
-  user_scripts_ = new_user_scripts;
   changed_extensions_.insert(extension->id());
-  if (script_reloader_.get()) {
+  if (is_loading_)
     pending_load_ = true;
-  } else {
+  else
     StartLoad();
-  }
 }
 
 void UserScriptMaster::Observe(int type,
@@ -486,19 +462,30 @@ void UserScriptMaster::Observe(int type,
   }
 
   if (should_start_load) {
-    if (script_reloader_.get()) {
+    if (is_loading_)
       pending_load_ = true;
-    } else {
+    else
       StartLoad();
-    }
   }
 }
 
 void UserScriptMaster::StartLoad() {
-  if (!script_reloader_.get())
-    script_reloader_ = new ScriptReloader(this);
-
-  script_reloader_->StartLoad(user_scripts_, extensions_info_);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(!is_loading_);
+  is_loading_ = true;
+  scoped_ptr<UserScriptList> user_scripts_copy(
+      new UserScriptList(*user_scripts_));
+  DCHECK(user_scripts_copy.get());
+  BrowserThread::PostTask(
+      BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(&LoadScriptsOnFileThread,
+                 base::Passed(&user_scripts_copy),
+                 extensions_info_,
+                 make_scoped_refptr(
+                     ExtensionSystem::Get(profile_)->content_verifier()),
+                 base::Bind(&UserScriptMaster::OnScriptsLoaded,
+                            weak_factory_.GetWeakPtr())));
 }
 
 void UserScriptMaster::SendUpdate(