Extend --load-extension commandline to accept multiple (comma-separated) extensions.

Extend --load-extension commandline to accept multiple (comma-separated) extensions.


R=asargent@chromium.org,mirandac@chromium.org
BUG=107397
TEST=ExtensionsLoadMultipleTest

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114478

[jvoung - send to chromium..., 2011-12-14 00:19:35]

+aa and mirandac, since you looked at a similar change and at least mirandac is
an owner =)

The similar change:
http://codereview.chromium.org/8659002

[Aaron Boodman, 2011-12-14 00:30:44]

lgtm

http://codereview.chromium.org/8898025/diff/1/chrome/browser/profiles/profile...
File chrome/browser/profiles/profile_impl.cc (right):

http://codereview.chromium.org/8898025/diff/1/chrome/browser/profiles/profile...
chrome/browser/profiles/profile_impl.cc:472:
extensions::UnpackedInstaller::Create(extension_service_.get())->
You could create one instance above the beginning of the while loop and use it
repeatedly to make this more readable.

[asargent_no_longer_on_chrome, 2011-12-14 00:37:44]

It would be nice to add a test for this. Probably the right thing to do is add 

chrome/browser/extensions/extension_load_browsertest.cc

and have that class inhert from InProcessBrowserTest, and then override
SetUpCommandLine to pass the flag with a path to two test extensions. The test
extensions should live in chrome/test/data/extensions (you might be able to just
re-use some ones we already have in there). 

See chrome/browser/extensions/extension_browsertest.cc - it is a common base
class for other extension browser_tests, and it already has a SetUpCommandLine
function that shows how to append switches and fine the path to the data dir.

Then in the body of the test, call browser()->profile()->GetExtensionService()
and use that to make sure that both extensions are installed (there are some
methods for getting the list of extensions).

http://codereview.chromium.org/8898025/diff/1/chrome/browser/profiles/profile...
File chrome/browser/profiles/profile_impl.cc (right):

http://codereview.chromium.org/8898025/diff/1/chrome/browser/profiles/profile...
chrome/browser/profiles/profile_impl.cc:470: FILE_PATH_LITERAL(","));
I guess if people have commas in their filenames they're out of luck? That's
probably rare enough that we don't care.

[asargent_no_longer_on_chrome, 2011-12-14 00:38:58]

Oh and BTW, you'll need to add the new test class to chrome/chrome_tests.gypi
(search for other extension browsertests in there) and re-run build/gyp_chromium
to get it to compile.

[jvoung - send to chromium..., 2011-12-14 03:00:43]

Thanks for the pointer, I found a similar test that loaded one extension in
"extension_startup_browsertest" and extended that to try loading 4.

http://codereview.chromium.org/8898025/diff/1/chrome/browser/profiles/profile...
File chrome/browser/profiles/profile_impl.cc (right):

http://codereview.chromium.org/8898025/diff/1/chrome/browser/profiles/profile...
chrome/browser/profiles/profile_impl.cc:470: FILE_PATH_LITERAL(","));
On 2011/12/14 00:37:45, Antony Sargent wrote:
> I guess if people have commas in their filenames they're out of luck? That's
> probably rare enough that we don't care.

Yeah, I'm not sure what would be a better character...

http://codereview.chromium.org/8898025/diff/1/chrome/browser/profiles/profile...
chrome/browser/profiles/profile_impl.cc:472:
extensions::UnpackedInstaller::Create(extension_service_.get())->
On 2011/12/14 00:30:44, Aaron Boodman wrote:
> You could create one instance above the beginning of the while loop and use it
> repeatedly to make this more readable.

Done.

[Miranda Callahan, 2011-12-14 08:48:31]

On 2011/12/14 03:00:43, jvoung wrote:
> Thanks for the pointer, I found a similar test that loaded one extension in
> "extension_startup_browsertest" and extended that to try loading 4.
> 
>
http://codereview.chromium.org/8898025/diff/1/chrome/browser/profiles/profile...
> File chrome/browser/profiles/profile_impl.cc (right):
> 
>
http://codereview.chromium.org/8898025/diff/1/chrome/browser/profiles/profile...
> chrome/browser/profiles/profile_impl.cc:470: FILE_PATH_LITERAL(","));
> On 2011/12/14 00:37:45, Antony Sargent wrote:
> > I guess if people have commas in their filenames they're out of luck? That's
> > probably rare enough that we don't care.
> 
> Yeah, I'm not sure what would be a better character...
> 
>
http://codereview.chromium.org/8898025/diff/1/chrome/browser/profiles/profile...
> chrome/browser/profiles/profile_impl.cc:472:
> extensions::UnpackedInstaller::Create(extension_service_.get())->
> On 2011/12/14 00:30:44, Aaron Boodman wrote:
> > You could create one instance above the beginning of the while loop and use
it
> > repeatedly to make this more readable.
> 
> Done.

OWNER's LGTM for profile_impl.

[asargent_no_longer_on_chrome, 2011-12-14 17:52:53]

LGTM

http://codereview.chromium.org/8898025/diff/6001/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_startup_browsertest.cc (right):

http://codereview.chromium.org/8898025/diff/6001/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_startup_browsertest.cc:230: 
nit: remove space here

[commit-bot: I haz the power, 2011-12-14 18:26:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jvoung@google.com/8898025/11001

[commit-bot: I haz the power, 2011-12-14 19:40:06]

Change committed as 114478