Exclude ProtectorService code from Android build.

Exclude ProtectorService code from Android build.

The change adds a ENABLE_PROTECTOR_SERVICE condition to exclude
ProtectorService and related code. I have introduced protector_utils.h which
contains methods used by keyword_table.cc to sign and verify keywords.

Note: ChromeOS also excludes protector from the tests
(chrome_tests.gypi), but disables ProtectorService using a command line
switch.

BUG=117407
TEST=


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

[Yaron, 2012-03-28 21:23:22]

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/automation/te...
File chrome/browser/automation/testing_automation_provider.cc (right):

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/automation/te...
chrome/browser/automation/testing_automation_provider.cc:2532: #if
defined(ENABLE_PROTECTOR_SERVICE)
Hmm. This shouldn't really be necessary as chrome/browser/automation is excluded
for Android builds but I guess it's more correct.

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/automation/te...
chrome/browser/automation/testing_automation_provider.cc:3505: #endif
When this is far enough away from the #if, add: #endif   //
defined(ENABLE_PROTECTOR_SERVICE)

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/protector/pro...
File chrome/browser/protector/protector.h (right):

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/protector/pro...
chrome/browser/protector/protector.h:5: #ifndef
CHROME_BROWSER_PROTECTOR_PROTECTOR_H_
Consider renaming protector_utils

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/search_engine...
File chrome/browser/search_engines/template_url_service.cc (right):

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/search_engine...
chrome/browser/search_engines/template_url_service.cc:632: #if
defined(ENALBE_PROTECTOR_SERVICE)
Typo in if-def. Should be ENABLE_PROTECTOR_SERVICE
Looks like your try-bots are green so this must not be tested.

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/webdata/keywo...
File chrome/browser/webdata/keyword_table.cc (right):

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/webdata/keywo...
chrome/browser/webdata/keyword_table.cc:88: return
protector::SignSetting(base::Int64ToString(id));
I think I'm missing something as this is still using "protector". Was it that
the move of those utils out of protector service because they're actually more
eneral util such as for signing, here? That's why we can do it regardless of
protector? If so, we could probably rename this to protector_utils::... or just
SigningUtils

[jam, 2012-03-28 21:28:16]

lgtm

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/protector/pro...
File chrome/browser/protector/protector.h (right):

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/protector/pro...
chrome/browser/protector/protector.h:5: #ifndef
CHROME_BROWSER_PROTECTOR_PROTECTOR_H_
On 2012/03/28 21:23:22, Yaron wrote:
> Consider renaming protector_utils

+1

[nilesh, 2012-03-28 23:35:11]

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/automation/te...
File chrome/browser/automation/testing_automation_provider.cc (right):

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/automation/te...
chrome/browser/automation/testing_automation_provider.cc:3505: #endif
On 2012/03/28 21:23:22, Yaron wrote:
> When this is far enough away from the #if, add: #endif   //
> defined(ENABLE_PROTECTOR_SERVICE)

Done.

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/protector/pro...
File chrome/browser/protector/protector.h (right):

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/protector/pro...
chrome/browser/protector/protector.h:5: #ifndef
CHROME_BROWSER_PROTECTOR_PROTECTOR_H_
On 2012/03/28 21:23:22, Yaron wrote:
> Consider renaming protector_utils

Done.

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/search_engine...
File chrome/browser/search_engines/template_url_service.cc (right):

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/search_engine...
chrome/browser/search_engines/template_url_service.cc:632: #if
defined(ENALBE_PROTECTOR_SERVICE)
On 2012/03/28 21:23:22, Yaron wrote:
> Typo in if-def. Should be ENABLE_PROTECTOR_SERVICE
> Looks like your try-bots are green so this must not be tested.

Thanks for catching this.

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/webdata/keywo...
File chrome/browser/webdata/keyword_table.cc (right):

http://codereview.chromium.org/9863047/diff/6001/chrome/browser/webdata/keywo...
chrome/browser/webdata/keyword_table.cc:88: return
protector::SignSetting(base::Int64ToString(id));
On 2012/03/28 21:23:22, Yaron wrote:
> I think I'm missing something as this is still using "protector". Was it that
> the move of those utils out of protector service because they're actually more
> eneral util such as for signing, here? That's why we can do it regardless of
> protector? If so, we could probably rename this to protector_utils::... or
just
> SigningUtils

Yeah the methods moved out of ProtectorService are more general. I am renaming
protector.h to protector_utils.h. I can't easily move them out of protector/ as
they depend on browser/protector/internal (separate repository) for the
signing-key.

[Yaron, 2012-03-28 23:46:45]

lgtm

I think you might need to rebase without my browser patch in your client (or
wait for mine to land) as otherwise I think the CQ will fail to apply your
patch. Also, you might need more targetted OWNERS than jam but I don't know if
any of the dirs use "set noparent"

[nilesh, 2012-03-29 17:07:48]

Adding more OWNERS.

[Elliot Glaysher, 2012-03-29 17:12:14]

profiles lgtm

[nilesh, 2012-03-29 17:13:44]

Hi Scott, 
Please take a look at the changes in:
chrome/browser/search_engines/template_url_service.cc
chrome/browser/ui/browser_init.cc

Thanks.

[sky, 2012-03-29 17:43:59]

LGTM

[commit-bot: I haz the power, 2012-03-29 18:11:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nileshagrawal@chromium.org/9863047/24001

[commit-bot: I haz the power, 2012-03-29 21:11:09]

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

[commit-bot: I haz the power, 2012-03-29 21:13:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nileshagrawal@chromium.org/9863047/24001

[commit-bot: I haz the power, 2012-03-29 23:43:39]

Try job failure for 9863047-24001 (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-03-29 23:57:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nileshagrawal@chromium.org/9863047/24001

[commit-bot: I haz the power, 2012-03-30 04:33:05]

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

[commit-bot: I haz the power, 2012-03-30 04:35:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nileshagrawal@chromium.org/9863047/24001

[commit-bot: I haz the power, 2012-03-30 08:16:37]

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

[commit-bot: I haz the power, 2012-03-30 14:40:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nileshagrawal@chromium.org/9863047/24001

[commit-bot: I haz the power, 2012-03-30 16:53:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nileshagrawal@chromium.org/9863047/45001

[commit-bot: I haz the power, 2012-03-30 19:57:34]

Change committed as 129910