Allow extension APIs to be compiled out.

Allow extension APIs to be compiled out.

BUG=117407
TEST=


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

[Aaron Boodman, 2012-04-09 21:49:47]

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/exte...
File chrome/browser/extensions/api/declarative/rules_registry_service.cc
(right):

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/exte...
chrome/browser/extensions/api/declarative/rules_registry_service.cc:1: //
Copyright (c) 2012 The Chromium Authors. All rights reserved.
This file should not be compiled.

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/exte...
File chrome/browser/extensions/app_notify_channel_ui.cc (right):

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/exte...
chrome/browser/extensions/app_notify_channel_ui.cc:138: #if
defined(ENABLE_EXTENSIONS)
What's the issue with this method?

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/exte...
File chrome/browser/extensions/extension_disabled_ui.cc (right):

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/exte...
chrome/browser/extensions/extension_disabled_ui.cc:251: #if
defined(ENABLE_EXTENSIONS)
What's the issue with this method?

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/ui/w...
File chrome/browser/ui/webui/extensions/extension_settings_handler.cc (right):

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/ui/w...
chrome/browser/ui/webui/extensions/extension_settings_handler.cc:812: #if
defined(ENABLE_EXTENSIONS)
Why this one?

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/chrome_brows...
File chrome/chrome_browser_extensions.gypi (right):

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/chrome_brows...
chrome/chrome_browser_extensions.gypi:500: ['exclude',
'^browser/accessibility/accessibility_extension_api.cc'],
This isn't everything. There are a whole bunch of things in
browser/extensions/api/ too.

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/chrome_brows...
chrome/chrome_browser_extensions.gypi:500: ['exclude',
'^browser/accessibility/accessibility_extension_api.cc'],
There are also all the resources in chrome/common/common_resources.grd (search
'json') and chrome/renderer/renderer_resources.grd (search 'bindings').

[Jesse Greenwald, 2012-04-09 22:44:47]

Responding to some of the "Why is this here" type comments.

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/exte...
File chrome/browser/extensions/app_notify_channel_ui.cc (right):

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/exte...
chrome/browser/extensions/app_notify_channel_ui.cc:138: #if
defined(ENABLE_EXTENSIONS)
On 2012/04/09 21:49:47, Aaron Boodman wrote:
> What's the issue with this method?

We don't have browser/ui/webui/signin on Android

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/exte...
File chrome/browser/extensions/extension_disabled_ui.cc (right):

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/exte...
chrome/browser/extensions/extension_disabled_ui.cc:251: #if
defined(ENABLE_EXTENSIONS)
On 2012/04/09 21:49:47, Aaron Boodman wrote:
> What's the issue with this method?

We don't have an implementation of ExtensionUninstallDialog::Create on Android

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/ui/w...
File chrome/browser/ui/webui/extensions/extension_settings_handler.cc (right):

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/ui/w...
chrome/browser/ui/webui/extensions/extension_settings_handler.cc:812: #if
defined(ENABLE_EXTENSIONS)
On 2012/04/09 21:49:47, Aaron Boodman wrote:
> Why this one?

We don't have an implementation of ExtensionUninstallDialog::Create on Android

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/chrome_brows...
File chrome/chrome_browser_extensions.gypi (right):

https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/chrome_brows...
chrome/chrome_browser_extensions.gypi:500: ['exclude',
'^browser/accessibility/accessibility_extension_api.cc'],
On 2012/04/09 21:49:47, Aaron Boodman wrote:
> This isn't everything. There are a whole bunch of things in
> browser/extensions/api/ too.

There is a filter for browser/extensions/api on line 508

[Aaron Boodman, 2012-04-09 22:49:46]

On Mon, Apr 9, 2012 at 3:44 PM,  <jgreenwald@chromium.org> wrote:
> Reviewers: Aaron Boodman,
>
> Message:
> Responding to some of the "Why is this here" type comments.
>
>
>
>
https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/exte...
> File chrome/browser/extensions/app_notify_channel_ui.cc (right):
>
>
https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/exte...
> chrome/browser/extensions/app_notify_channel_ui.cc:138: #if
> defined(ENABLE_EXTENSIONS)
> On 2012/04/09 21:49:47, Aaron Boodman wrote:
>>
>> What's the issue with this method?
>
>
> We don't have browser/ui/webui/signin on Android
>
>
>
https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/exte...
> File chrome/browser/extensions/extension_disabled_ui.cc (right):
>
>
https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/exte...
> chrome/browser/extensions/extension_disabled_ui.cc:251: #if
> defined(ENABLE_EXTENSIONS)
> On 2012/04/09 21:49:47, Aaron Boodman wrote:
>>
>> What's the issue with this method?
>
>
> We don't have an implementation of ExtensionUninstallDialog::Create on
> Android
>
>
>
https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/ui/w...
> File chrome/browser/ui/webui/extensions/extension_settings_handler.cc
> (right):
>
>
https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/browser/ui/w...
> chrome/browser/ui/webui/extensions/extension_settings_handler.cc:812:
> #if defined(ENABLE_EXTENSIONS)
> On 2012/04/09 21:49:47, Aaron Boodman wrote:
>>
>> Why this one?
>
>
> We don't have an implementation of ExtensionUninstallDialog::Create on
> Android
>
>
>
https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/chrome_brows...
> File chrome/chrome_browser_extensions.gypi (right):
>
>
https://chromiumcodereview.appspot.com/10031002/diff/2001/chrome/chrome_brows...
> chrome/chrome_browser_extensions.gypi:500: ['exclude',
> '^browser/accessibility/accessibility_extension_api.cc'],
> On 2012/04/09 21:49:47, Aaron Boodman wrote:
>>
>> This isn't everything. There are a whole bunch of things in
>> browser/extensions/api/ too.
>
>
> There is a filter for browser/extensions/api on line 508

Ah! (on all accounts). Thanks.

- a

[Erik does not do reviews, 2012-04-10 18:58:31]

https://chromiumcodereview.appspot.com/10031002/diff/8001/chrome/common/commo...
File chrome/common/common_resources.grd (right):

https://chromiumcodereview.appspot.com/10031002/diff/8001/chrome/common/commo...
chrome/common/common_resources.grd:15: <if expr="pp_ifdef('enable_extensions')">
Does it make sense to split this out into a separate grd file rather than having
an ifdef?

[Jesse Greenwald, 2012-04-10 21:14:07]

https://chromiumcodereview.appspot.com/10031002/diff/8001/chrome/common/commo...
File chrome/common/common_resources.grd (right):

https://chromiumcodereview.appspot.com/10031002/diff/8001/chrome/common/commo...
chrome/common/common_resources.grd:15: <if expr="pp_ifdef('enable_extensions')">
On 2012/04/10 18:58:32, Erik Kay wrote:
> Does it make sense to split this out into a separate grd file rather than
having
> an ifdef?

Sure, maybe in: chrome/browser/extensions/api/api_resources.grd ?

[Aaron Boodman, 2012-04-11 06:36:39]

http://codereview.chromium.org/10031002/diff/8001/chrome/chrome_browser_exten...
File chrome/chrome_browser_extensions.gypi (right):

http://codereview.chromium.org/10031002/diff/8001/chrome/chrome_browser_exten...
chrome/chrome_browser_extensions.gypi:498: ['enable_extensions==0', {
Is it possible to only include these files in the first place if
enable_extensions=1? I find the exclude pattern very
spooky-action-at-a-distance.

[Aaron Boodman, 2012-04-11 14:15:58]

On 2012/04/11 06:36:39, Aaron Boodman wrote:
>
http://codereview.chromium.org/10031002/diff/8001/chrome/chrome_browser_exten...
> File chrome/chrome_browser_extensions.gypi (right):
> 
>
http://codereview.chromium.org/10031002/diff/8001/chrome/chrome_browser_exten...
> chrome/chrome_browser_extensions.gypi:498: ['enable_extensions==0', {
> Is it possible to only include these files in the first place if
> enable_extensions=1? I find the exclude pattern very
> spooky-action-at-a-distance.

Otherwise, FWIW, change is looking much nicer.

[Jesse Greenwald, 2012-04-11 19:11:57]

https://chromiumcodereview.appspot.com/10031002/diff/8001/chrome/chrome_brows...
File chrome/chrome_browser_extensions.gypi (right):

https://chromiumcodereview.appspot.com/10031002/diff/8001/chrome/chrome_brows...
chrome/chrome_browser_extensions.gypi:498: ['enable_extensions==0', {
On 2012/04/11 06:36:40, Aaron Boodman wrote:
> Is it possible to only include these files in the first place if
> enable_extensions=1? I find the exclude pattern very
> spooky-action-at-a-distance.

Can you clarify?  Are you talking about the top list of excludes here and/or the
bottom list of excluding things from browser/extensions/api?

[Jesse Greenwald, 2012-04-12 02:54:47]

Resolved review comments (as I understood them).  PTAL

[Aaron Boodman, 2012-04-12 04:00:25]

LGTM

https://chromiumcodereview.appspot.com/10031002/diff/15001/chrome/chrome_brow...
File chrome/chrome_browser.gypi (right):

https://chromiumcodereview.appspot.com/10031002/diff/15001/chrome/chrome_brow...
chrome/chrome_browser.gypi:3981: ['enable_extensions==1', {
Cool, that's what I was hoping for. Can you add a little comment here indicating
this is only for extension API implementations?

[commit-bot: I haz the power, 2012-04-13 02:55:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jgreenwald@chromium.org/10031002/19002

[commit-bot: I haz the power, 2012-04-13 02:55:34]

Presubmit check for 10031002-19002 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/browser/ui/webui

Presubmit checks took 2.6s to calculate.

[Jesse Greenwald, 2012-04-13 03:02:15]

jhawkins, can you review:
  chrome/browser/ui/webui/extensions/extension_settings_handler.cc

Thanks,

[James Hawkins, 2012-04-13 03:06:56]

lgtm

[commit-bot: I haz the power, 2012-04-13 14:44:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jgreenwald@chromium.org/10031002/19002

[commit-bot: I haz the power, 2012-04-13 22:33:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jgreenwald@chromium.org/10031002/27003

[commit-bot: I haz the power, 2012-04-14 01:16:17]

Try job failure for 10031002-27003 (retry) on mac_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-04-16 17:18:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jgreenwald@chromium.org/10031002/33001

[commit-bot: I haz the power, 2012-04-16 19:22:56]

Change committed as 132440