hterm: Load component-extension version of crosh+hterm (chromeos only).

hterm: Load component-extension version of crosh+hterm (chromeos only).

This adds the crosh_builtin component extension provided by
https://gerrit.chromium.org/gerrit/18899 to chromeos.

BUG=chromium-os:27663, chromium-os:27630
TEST=Test that ctrl-alt-t opens crosh even with no extensions
installed.
TEST=Install hterm, test that ctrl-alt-t opens hterm instead of
component extension.
TEST=Disable hterm, ensure that ctrl-alt-t opens component
extension again.

Note: Use document.location.href from the js console to determine
which extension is launched for ctrl-alt-t.


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

[rginda, 2012-03-22 23:27:33]

win_rel failure looks unrelated (can't find a bluetooth.h file), resubmitting
the try job.

[zel, 2012-03-22 23:35:08]

lgtm

[tonibarzic, 2012-03-22 23:53:55]

https://chromiumcodereview.appspot.com/9826032/diff/2001/chrome/browser/exten...
File chrome/browser/extensions/api/terminal/terminal_extension_helper.cc
(right):

https://chromiumcodereview.appspot.com/9826032/diff/2001/chrome/browser/exten...
chrome/browser/extensions/api/terminal/terminal_extension_helper.cc:31: if
(extension && service->IsExtensionEnabled(kPossibleAppIds[x]))
false in GetExtensionById(id, false) is for "include_disabled", 
so you probably don't need IsExtensionEnabled check

[rginda, 2012-03-23 00:00:48]

On 2012/03/22 23:53:55, tonibarzic wrote:
>
https://chromiumcodereview.appspot.com/9826032/diff/2001/chrome/browser/exten...
> File chrome/browser/extensions/api/terminal/terminal_extension_helper.cc
> (right):
> 
>
https://chromiumcodereview.appspot.com/9826032/diff/2001/chrome/browser/exten...
> chrome/browser/extensions/api/terminal/terminal_extension_helper.cc:31: if
> (extension && service->IsExtensionEnabled(kPossibleAppIds[x]))
> false in GetExtensionById(id, false) is for "include_disabled", 
> so you probably don't need IsExtensionEnabled check

I looked into the IsExtensionEnabled code and it appeared as though it would
return true for an unknown extension, under the assumption that it just hasn't
been loaded yet.

[rginda, 2012-03-23 00:01:20]

On 2012/03/23 00:00:48, rginda wrote:
> On 2012/03/22 23:53:55, tonibarzic wrote:
> >
>
https://chromiumcodereview.appspot.com/9826032/diff/2001/chrome/browser/exten...
> > File chrome/browser/extensions/api/terminal/terminal_extension_helper.cc
> > (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/9826032/diff/2001/chrome/browser/exten...
> > chrome/browser/extensions/api/terminal/terminal_extension_helper.cc:31: if
> > (extension && service->IsExtensionEnabled(kPossibleAppIds[x]))
> > false in GetExtensionById(id, false) is for "include_disabled", 
> > so you probably don't need IsExtensionEnabled check
> 
> I looked into the IsExtensionEnabled code and it appeared as though it would
> return true for an unknown extension, under the assumption that it just hasn't
> been loaded yet.

Oh, sorry, I misread your comment.

[rginda, 2012-03-23 00:03:17]

PTAL

https://chromiumcodereview.appspot.com/9826032/diff/2001/chrome/browser/exten...
File chrome/browser/extensions/api/terminal/terminal_extension_helper.cc
(right):

https://chromiumcodereview.appspot.com/9826032/diff/2001/chrome/browser/exten...
chrome/browser/extensions/api/terminal/terminal_extension_helper.cc:31: if
(extension && service->IsExtensionEnabled(kPossibleAppIds[x]))
On 2012/03/22 23:53:56, tonibarzic wrote:
> false in GetExtensionById(id, false) is for "include_disabled", 
> so you probably don't need IsExtensionEnabled check

Done.

[tbarzic, 2012-03-23 00:11:30]

On 2012/03/23 00:03:17, rginda wrote:
> PTAL
> 
>
https://chromiumcodereview.appspot.com/9826032/diff/2001/chrome/browser/exten...
> File chrome/browser/extensions/api/terminal/terminal_extension_helper.cc
> (right):
> 
>
https://chromiumcodereview.appspot.com/9826032/diff/2001/chrome/browser/exten...
> chrome/browser/extensions/api/terminal/terminal_extension_helper.cc:31: if
> (extension && service->IsExtensionEnabled(kPossibleAppIds[x]))
> On 2012/03/22 23:53:56, tonibarzic wrote:
> > false in GetExtensionById(id, false) is for "include_disabled", 
> > so you probably don't need IsExtensionEnabled check
> 
> Done.
lgtm

[rginda, 2012-03-23 01:07:34]

+ben for chrome/browser/browser_resources.grd owner review.
Aaron, can you do an owner review for the */extensions/* stuff?

[Ben Goodger (Google), 2012-03-23 16:57:07]

LGTM for any bits I am owner of.

[Aaron Boodman, 2012-03-23 18:30:26]

LGTM

[commit-bot: I haz the power, 2012-03-23 20:43:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rginda@chromium.org/9826032/7001

[commit-bot: I haz the power, 2012-03-24 01:27:48]

Change committed as 128642