Make UserScript non-copyable.

extensions/browser/web_ui_user_script_loader.cc

 // Copyright 2015 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 "extensions/browser/web_ui_user_script_loader.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/strings/string_util.h"
@@ skipping 30 matching lines @@
 WebUIUserScriptLoader::WebUIUserScriptLoader(
     content::BrowserContext* browser_context,
     const HostID& host_id)
     : UserScriptLoader(browser_context, host_id), complete_fetchers_(0) {
   SetReady(true);
 }
 
 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);
 }
 
 void WebUIUserScriptLoader::LoadScripts(
     std::unique_ptr<extensions::UserScriptList> user_scripts,
     const std::set<HostID>& changed_hosts,
     const std::set<int>& added_script_ids,
     LoadScriptsCallback callback) {
   user_scripts_cache_.swap(user_scripts);
   scripts_loaded_callback_ = callback;
 
   // The total number of the tasks is used to trace whether all the fetches
   // are complete. Therefore, we store all the fetcher pointers in |fetchers_|
   // before we get theis number. Once we get the total number, start each
   // 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;
 
     content::BrowserContext* browser_context =
         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.
   if (fetchers_.empty()) {
     OnWebUIURLFetchComplete();
     return;
   }
   for (const auto& fetcher : fetchers_)
     fetcher->Start();
 }
 
 void WebUIUserScriptLoader::CreateWebUIURLFetchers(
     extensions::UserScript::FileList* script_files,

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

this should pass by const &

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

Done.

     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));
     }
   }
 }
 
 void WebUIUserScriptLoader::OnSingleWebUIURLFetchComplete(
     extensions::UserScript::File* script_file,
     bool success,
     const std::string& data) {
   if (success) {
@@ skipping 14 matching lines @@
   }
 }
 
 void WebUIUserScriptLoader::OnWebUIURLFetchComplete() {
   content::BrowserThread::PostTask(
       content::BrowserThread::FILE, FROM_HERE,
       base::Bind(&SerializeOnFileThread, base::Passed(&user_scripts_cache_),
                  scripts_loaded_callback_));
   scripts_loaded_callback_.Reset();
 }