Make UserScript non-copyable.

extensions/browser/web_ui_user_script_loader.cc

Index: extensions/browser/web_ui_user_script_loader.cc
diff --git a/extensions/browser/web_ui_user_script_loader.cc b/extensions/browser/web_ui_user_script_loader.cc
index 2180e8ef7f954ba563c0411f28f9e146542a5ba0..ad13adaaee946c240bf1f7d1baa5acd1c0169c78 100644
--- a/extensions/browser/web_ui_user_script_loader.cc
+++ b/extensions/browser/web_ui_user_script_loader.cc
@@ -48,14 +48,13 @@ WebUIUserScriptLoader::WebUIUserScriptLoader(
 WebUIUserScriptLoader::~WebUIUserScriptLoader() {
 }
 
-void WebUIUserScriptLoader::AddScripts(
-    const extensions::UserScriptList& scripts,
-    int render_process_id,
-    int render_frame_id) {
+void WebUIUserScriptLoader::AddScripts(extensions::UserScriptList& scripts,
+                                       int render_process_id,
+                                       int render_frame_id) {
   UserScriptRenderInfo info(render_process_id, render_frame_id);
-  for (const extensions::UserScript& script : scripts) {
+  for (const std::unique_ptr<extensions::UserScript>& script : scripts) {
     script_render_info_map_.insert(
-        std::pair<int, UserScriptRenderInfo>(script.id(), info));
+        std::pair<int, UserScriptRenderInfo>(script->id(), info));
   }
 
   extensions::UserScriptLoader::AddScripts(scripts);
@@ -75,11 +74,12 @@ void WebUIUserScriptLoader::LoadScripts(
   // fetch tasks.
   DCHECK_EQ(0u, complete_fetchers_);
 
-  for (extensions::UserScript& script : *user_scripts_cache_) {
-    if (added_script_ids.count(script.id()) == 0)
+  for (const std::unique_ptr<extensions::UserScript>& script :
+       *user_scripts_cache_) {
+    if (added_script_ids.count(script->id()) == 0)
       continue;
 
-    auto iter = script_render_info_map_.find(script.id());
+    auto iter = script_render_info_map_.find(script->id());
     DCHECK(iter != script_render_info_map_.end());
     int render_process_id = iter->second.render_process_id;
     int render_frame_id = iter->second.render_frame_id;
@@ -88,12 +88,12 @@ void WebUIUserScriptLoader::LoadScripts(
         content::RenderProcessHost::FromID(render_process_id)
             ->GetBrowserContext();
 
-    CreateWebUIURLFetchers(&script.js_scripts(), browser_context,
+    CreateWebUIURLFetchers(&script->js_scripts(), browser_context,
                            render_process_id, render_frame_id);
-    CreateWebUIURLFetchers(&script.css_scripts(), browser_context,
+    CreateWebUIURLFetchers(&script->css_scripts(), browser_context,
                            render_process_id, render_frame_id);
 
-    script_render_info_map_.erase(script.id());
+    script_render_info_map_.erase(script->id());
   }
 
   // If no fetch is needed, call OnWebUIURLFetchComplete directly.
@@ -110,15 +110,17 @@ void WebUIUserScriptLoader::CreateWebUIURLFetchers(
     content::BrowserContext* browser_context,
     int render_process_id,
     int render_frame_id) {
-  for (extensions::UserScript::File& file : *script_files) {
-    if (file.GetContent().empty()) {
+  for (const std::unique_ptr<extensions::UserScript::File>& script_file :
+       *script_files) {
+    if (script_file->GetContent().empty()) {
       // The WebUIUserScriptLoader owns these WebUIURLFetchers. Once the
       // loader is destroyed, all the fetchers will be destroyed. Therefore,
       // we are sure it is safe to use base::Unretained(this) here.

[Devlin, 2016/08/17 16:39:31]

This explains why base::Unretained(this) is safe, but doesn't explain why using
the raw file is safe.  If LoadScripts() were ever to be called before the
previous load had finished, this would break, because the scripts would be
deleted.

It looks like this can't happen because UserScriptLoader should only have one
load running at a time, but it'd be nice to maybe DCHECK in LoadScripts that
user_script_cache_ is empty (and reset it in OnWebUIUrlFetchComplete).

[lazyboy, 2016/08/17 18:55:52]

Added note here.
but it'd be nice to maybe DCHECK in LoadScripts that
Added DCHECK
(and reset it in OnWebUIUrlFetchComplete).
The reset should already happen because of base::Passed in
OnSingleWebUIURLFetchComplete, right?

[Devlin, 2016/08/17 20:40:39]

In practice, yes, I think so.  But AFAIK, base::Passed is basically a wrapper
around std::move, and the semantics std::move and move ctors typically state
that the object can be left in an undefined state.  reset() would ensure it gets
back into a sane, empty state, despite any crazy optimizations that C++32
decides to make. :)

[lazyboy, 2016/08/17 21:34:20]

LOL
Added explicit reset().

       std::unique_ptr<WebUIURLFetcher> fetcher(new WebUIURLFetcher(
-          browser_context, render_process_id, render_frame_id, file.url(),
+          browser_context, render_process_id, render_frame_id,
+          script_file->url(),
           base::Bind(&WebUIUserScriptLoader::OnSingleWebUIURLFetchComplete,
-                     base::Unretained(this), &file)));
+                     base::Unretained(this), script_file.get())));
       fetchers_.push_back(std::move(fetcher));
     }
   }