Add first-class support for user scripts

Add first-class support for user scripts by automatically converting them into extensions on install.

BUG=22103
TEST=Install a user script (such as from userscripts.org). You should get the extension install UI and the script should show up in the extension management UI. It should also work, though some scripts use Firefox-specific APIs and those won't work in Chromium.

[omattos, 2009-11-02 21:21:16]

On 2009/11/02 06:44:45, Aaron Boodman wrote:
> though some scripts use Firefox-specific APIs

Can we do a simple string search on the user script and fail the installation
with a warning if firefox api's are found?  This heuristic probably won't pick
everything up, but will catch the majority of cases.

[Aaron Boodman, 2009-11-02 22:08:54]

On 2009/11/02 21:21:16, omattos wrote:
> On 2009/11/02 06:44:45, Aaron Boodman wrote:
> > though some scripts use Firefox-specific APIs
> 
> Can we do a simple string search on the user script and fail the installation
> with a warning if firefox api's are found?  This heuristic probably won't pick
> everything up, but will catch the majority of cases.

I was just guessing that this would be the case; I'm not aware of any
Firefox-specific APIs off-hand.

The thing I'm mainly worried about is DOM/CSS/JavaScript APIs that work slightly
differently in WebKit than in Gecko, or proprietary/experimental features of
Gecko that haven't been adopted by WebKit.

[Matt Perry, 2009-11-02 23:15:04]

very awesome. mostly LGTM

http://codereview.chromium.org/340057/diff/51/1030
File chrome/browser/download/download_manager.cc (right):

http://codereview.chromium.org/340057/diff/51/1030#newcode1250
Line 1250: if (Extension::HasExtensionFileExtension(full_path)) {
It looks like the way you did the logic in here and in IsExtensionInstall is
different, so that a file served with the extension mimetype but ending in .foo
will be treated as a user script.

http://codereview.chromium.org/340057/diff/51/1031
File chrome/browser/extensions/convert_user_script.cc (right):

http://codereview.chromium.org/340057/diff/51/1031#newcode92
Line 92: matches->Append(Value::CreateStringValue("http://*/*"));
this will cause the script to apply to every page, since we take the union of
@match and @include patterns.

http://codereview.chromium.org/340057/diff/51/1031#newcode130
Line 130: std::string error;
reuse the error param

http://codereview.chromium.org/340057/diff/51/1034
File chrome/browser/extensions/crx_installer.cc (right):

http://codereview.chromium.org/340057/diff/51/1034#newcode97
Line 97: void CrxInstaller::StartCrx() {
nit: the only caller is Start, so you could just move this code there

http://codereview.chromium.org/340057/diff/51/1040
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/340057/diff/51/1040#newcode143
Line 143: static FilePath extension;
can't you just do 'static FilePath extension(kFileExtension)' ?

http://codereview.chromium.org/340057/diff/51/1040#newcode250
Line 250: error, &UserScript::add_glob, result)) {
a slightly more c++ way of doing this would be to change the last 2 params to:
  NewCallback(result, &UserScript::add_glob)
with the param type as
  Callback1<std::string>::Type
but if you think it makes the code uglier, this way is fine.

http://codereview.chromium.org/340057/diff/51/1046
File chrome/common/extensions/user_script.cc (right):

http://codereview.chromium.org/340057/diff/51/1046#newcode17
Line 17: static FilePath extension;
same comment about static init

http://codereview.chromium.org/340057/diff/51/1047
File chrome/common/extensions/user_script.h (right):

http://codereview.chromium.org/340057/diff/51/1047#newcode104
Line 104: std::string name_space() const { return name_space_; }
return const ref

http://codereview.chromium.org/340057/diff/51/1047#newcode109
Line 109: std::string name() const { return name_; }
ditto

http://codereview.chromium.org/340057/diff/51/1047#newcode112
Line 112: std::string description() const { return description_; }
ditto

http://codereview.chromium.org/340057/diff/51/1049
File chrome/renderer/user_script_slave.cc (right):

http://codereview.chromium.org/340057/diff/51/1049#newcode177
Line 177: if (!script->extension_id().empty()) {
does this mean user scripts as extensions get access to the content script APIs?

[Aaron Boodman, 2009-11-03 20:08:45]

Ok, ready for another look.

Also added some stubs functions for unimplemented Greasemonkey APIs.

http://codereview.chromium.org/340057/diff/51/1030
File chrome/browser/download/download_manager.cc (right):

http://codereview.chromium.org/340057/diff/51/1030#newcode1250
Line 1250: if (Extension::HasExtensionFileExtension(full_path)) {
On 2009/11/02 23:15:04, Matt Perry wrote:
> It looks like the way you did the logic in here and in IsExtensionInstall is
> different, so that a file served with the extension mimetype but ending in
.foo
> will be treated as a user script.

Ah, good catch. Thanks. Fixed.

http://codereview.chromium.org/340057/diff/51/1031
File chrome/browser/extensions/convert_user_script.cc (right):

http://codereview.chromium.org/340057/diff/51/1031#newcode92
Line 92: matches->Append(Value::CreateStringValue("http://*/*"));
On 2009/11/02 23:15:04, Matt Perry wrote:
> this will cause the script to apply to every page, since we take the union of
> @match and @include patterns.

Great catch. I have changed this to do an intersection of the sets of exclude,
include, and url patterns instead.

http://codereview.chromium.org/340057/diff/51/1031#newcode130
Line 130: std::string error;
On 2009/11/02 23:15:04, Matt Perry wrote:
> reuse the error param

Done.

http://codereview.chromium.org/340057/diff/51/1034
File chrome/browser/extensions/crx_installer.cc (right):

http://codereview.chromium.org/340057/diff/51/1034#newcode97
Line 97: void CrxInstaller::StartCrx() {
On 2009/11/02 23:15:04, Matt Perry wrote:
> nit: the only caller is Start, so you could just move this code there

Done.

http://codereview.chromium.org/340057/diff/51/1040
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/340057/diff/51/1040#newcode143
Line 143: static FilePath extension;
On 2009/11/02 23:15:04, Matt Perry wrote:
> can't you just do 'static FilePath extension(kFileExtension)' ?

This code is gone per another one of your comments, but good point -- I've fixed
the same code in the UserScript class.

http://codereview.chromium.org/340057/diff/51/1040#newcode250
Line 250: error, &UserScript::add_glob, result)) {
On 2009/11/02 23:15:04, Matt Perry wrote:
> a slightly more c++ way of doing this would be to change the last 2 params to:
>   NewCallback(result, &UserScript::add_glob)
> with the param type as
>   Callback1<std::string>::Type
> but if you think it makes the code uglier, this way is fine.

I tried doing this, but got stuck on template errors and am not feeling
motivated enough to fix.

http://codereview.chromium.org/340057/diff/51/1046
File chrome/common/extensions/user_script.cc (right):

http://codereview.chromium.org/340057/diff/51/1046#newcode17
Line 17: static FilePath extension;
On 2009/11/02 23:15:04, Matt Perry wrote:
> same comment about static init

Done.

http://codereview.chromium.org/340057/diff/51/1049
File chrome/renderer/user_script_slave.cc (right):

http://codereview.chromium.org/340057/diff/51/1049#newcode177
Line 177: if (!script->extension_id().empty()) {
On 2009/11/02 23:15:04, Matt Perry wrote:
> does this mean user scripts as extensions get access to the content script
APIs?

Yes, which seems ok to me (thinking ahead to a feature in the developer tools to
convert a user script to an extension).

[Matt Perry, 2009-11-03 20:21:29]

LGTM

http://codereview.chromium.org/340057/diff/4002/6023
File chrome/renderer/resources/greasemonkey_api.js (right):

http://codereview.chromium.org/340057/diff/4002/6023#newcode72
Line 72: console.log("adding apis");
remove log statements?