Introduce ScriptInjection in extensions/renderer to inject UserScripts

Introduce ScriptInjection in extensions/renderer to inject UserScripts

UserScriptSlave was responsible for too much, and the logic was getting
cluttered (which also made changes in, e.g.
https://codereview.chromium.org/288053002/ difficult to read, write, and
review). Refactor the logic to inject the scripts into a new class, so that
UserScriptSlave is pretty much just responsible for containing them.

BUG=373961
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271532

[Devlin, 2014-05-15 21:53:11]

As requested.

[not at google - send to devlin, 2014-05-15 23:23:20]

lgtm

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/script_i...
File extensions/renderer/script_injection.cc (right):

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/script_i...
extensions/renderer/script_injection.cc:90: 
dd

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/script_i...
extensions/renderer/script_injection.cc:138: script_->run_location() ==
UserScript::DOCUMENT_START;
I think you want |run_location| == DOCUMENT_START not script_->run_location() ==
DOCUMENT_START.

which makes me realise, despite my previous comment, that we should be safe to
delay CSS injection without a page refresh (unlike JS).

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/script_i...
File extensions/renderer/script_injection.h (right):

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/script_i...
extensions/renderer/script_injection.h:66: // Injects the script into the given
|frame|, and stores the information about
s/stores/updates

or something to imply that this is an additive operation, not a destructive one,
which wasn't clear tome.

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/user_scr...
File extensions/renderer/user_script_slave.cc (right):

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/user_scr...
extensions/renderer/user_script_slave.cc:185: 
I note you removed your TODO here :)

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/user_scr...
File extensions/renderer/user_script_slave.h (right):

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/user_scr...
extensions/renderer/user_script_slave.h:80: std::vector<ScriptInjection*>
script_injections_;
mind changing this to a ScopedVector?

[Devlin, 2014-05-16 22:32:20]

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/script_i...
File extensions/renderer/script_injection.cc (right):

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/script_i...
extensions/renderer/script_injection.cc:90: 
On 2014/05/15 23:23:21, kalman wrote:
> dd

Done.

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/script_i...
extensions/renderer/script_injection.cc:138: script_->run_location() ==
UserScript::DOCUMENT_START;
On 2014/05/15 23:23:21, kalman wrote:
> I think you want |run_location| == DOCUMENT_START not script_->run_location()
==
> DOCUMENT_START.
> 
> which makes me realise, despite my previous comment, that we should be safe to
> delay CSS injection without a page refresh (unlike JS).

Whoops, good catch - yes, meant |run_location|.

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/script_i...
File extensions/renderer/script_injection.h (right):

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/script_i...
extensions/renderer/script_injection.h:66: // Injects the script into the given
|frame|, and stores the information about
On 2014/05/15 23:23:21, kalman wrote:
> s/stores/updates
> 
> or something to imply that this is an additive operation, not a destructive
one,
> which wasn't clear tome.

Done.

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/user_scr...
File extensions/renderer/user_script_slave.cc (right):

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/user_scr...
extensions/renderer/user_script_slave.cc:185: 
On 2014/05/15 23:23:21, kalman wrote:
> I note you removed your TODO here :)

Whoops!  That was actually just because this was based off the content scripts
patch, in which the TODO *is* resolved. :)  Added back.

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/user_scr...
File extensions/renderer/user_script_slave.h (right):

https://codereview.chromium.org/284153006/diff/1/extensions/renderer/user_scr...
extensions/renderer/user_script_slave.h:80: std::vector<ScriptInjection*>
script_injections_;
On 2014/05/15 23:23:21, kalman wrote:
> mind changing this to a ScopedVector?

Sure.

[Devlin, 2014-05-19 19:43:58]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2014-05-19 19:44:25]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/284153006/...

[commit-bot: I haz the power, 2014-05-19 23:53:13]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-20 01:59:49]

Change committed as 271532