Make UserScript non-copyable.

extensions/renderer/user_script_injector.cc

Index: extensions/renderer/user_script_injector.cc
diff --git a/extensions/renderer/user_script_injector.cc b/extensions/renderer/user_script_injector.cc
index 21a59eac30fa4bd3e03c928090db5f13e6189ed9..0a13e2283aaa6830eac1d1ce9688d2c19881c313 100644
--- a/extensions/renderer/user_script_injector.cc
+++ b/extensions/renderer/user_script_injector.cc
@@ -101,7 +101,7 @@ UserScriptInjector::~UserScriptInjector() {
 
 void UserScriptInjector::OnUserScriptsUpdated(
     const std::set<HostID>& changed_hosts,
-    const std::vector<std::unique_ptr<UserScript>>& scripts) {
+    const UserScriptList& scripts) {
   // If the host causing this injection changed, then this injection
   // will be removed, and there's no guarantee the backing script still exists.
   if (changed_hosts.count(host_id_) > 0) {
@@ -202,13 +202,8 @@ std::vector<blink::WebScriptSource> UserScriptInjector::GetJsSources(
 
   DCHECK_EQ(script_->run_location(), run_location);
 
-  const UserScript::FileList& js_scripts = script_->js_scripts();
-
-  for (UserScript::FileList::const_iterator iter = js_scripts.begin();
-       iter != js_scripts.end();
-       ++iter) {
+  for (const std::unique_ptr<UserScript::File>& iter : script_->js_scripts()) {

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

s/iter/file

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

Done.

     std::string content = iter->GetContent().as_string();

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

wait.  Doesn't this mean that we're performing *another* copy here?

Yuck.  This is used to construct the WebString that's part of the source that we
inject.  WebString (and WTF::String) allow for refcounting so that they don't
have to copy around all the underlying data, which is great.  Unfortunately, I
don't know if we can leverage that here, since this string wouldn't be
refcounted and the data goes away if the extension is updated (and js scripts
can execute async - yay complications!).

For now, I think a TODO to address this is fine.

One thought of how we could at least make this better would be to have a single
WebString member for each script somewhere in the renderer (strawman that we
shouldn't do but as an example: make each instance a std::pair<UserScript,
WebString>).  Then, we could use that WebString here, which wouldn't perform a
copy of the data - it would just refcount the inner storage.  We still have to
do this copy once per user script per renderer process, but at least we're not
doing it once per script per frame per process.

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

As discussed offline, there're actually two copies. I've added a todo and will
follow up.

-
     // We add this dumb function wrapper for user scripts to emulate what
     // Greasemonkey does.
     if (script_->emulate_greasemonkey()) {
@@ -235,12 +230,8 @@ std::vector<std::string> UserScriptInjector::GetCssSources(
   if (!script_)
     return sources;
 
-  const UserScript::FileList& css_scripts = script_->css_scripts();
-  for (UserScript::FileList::const_iterator iter = css_scripts.begin();
-       iter != css_scripts.end();
-       ++iter) {
+  for (const std::unique_ptr<UserScript::File>& iter : script_->css_scripts())
     sources.push_back(iter->GetContent().as_string());

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

ditto :(

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

Added TODO

-  }
   return sources;
 }
 
@@ -253,9 +244,7 @@ void UserScriptInjector::GetRunInfo(
   if (ShouldInjectJs(run_location)) {
     const UserScript::FileList& js_scripts = script_->js_scripts();
     scripts_run_info->num_js += js_scripts.size();
-    for (UserScript::FileList::const_iterator iter = js_scripts.begin();
-         iter != js_scripts.end();
-         ++iter) {
+    for (const std::unique_ptr<UserScript::File>& iter : js_scripts) {
       scripts_run_info->executing_scripts[host_id_.id()].insert(
           iter->url().path());
     }