Fix most remaining WebUI link errors on Android.

Fix most remaining WebUI link errors on Android.

The remaining ones are related to extensions and session service which
will be handled separately.

Changes included:
- Remove remaining login/sync bits from NTP. They're handled in native
  ui.
- Temporarily remove chrome://inspect and chrome://flags (both don't
  work)
- Stub out part of chrome://history that relies on feature code to be
  upstreamed.

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

[Yaron, 2012-03-27 02:39:09]

atwilson: sync parts, gyp
estate: browser/ui/webui, gyp

[Yaron, 2012-03-27 02:39:32]

On 2012/03/27 02:39:09, Yaron wrote:
> atwilson: sync parts, gyp
> estate: browser/ui/webui, gyp

err estade :)

[Yaron, 2012-03-27 03:00:33]

battre: chrome/browser/prefs

Sorry, missed that one in the first pass.

[battre, 2012-03-27 05:58:18]

http://codereview.chromium.org/9836103/diff/2002/chrome/browser/prefs/browser...
File chrome/browser/prefs/browser_prefs.cc (right):

http://codereview.chromium.org/9836103/diff/2002/chrome/browser/prefs/browser...
chrome/browser/prefs/browser_prefs.cc:148: FlagsUI::RegisterPrefs(local_state);
I think if you disable registration of these prefs here, you should also disable
their use in about_flags.cc

[Yaron, 2012-03-27 16:07:31]

http://codereview.chromium.org/9836103/diff/2002/chrome/browser/prefs/browser...
File chrome/browser/prefs/browser_prefs.cc (right):

http://codereview.chromium.org/9836103/diff/2002/chrome/browser/prefs/browser...
chrome/browser/prefs/browser_prefs.cc:148: FlagsUI::RegisterPrefs(local_state);
On 2012/03/27 05:58:18, battre wrote:
> I think if you disable registration of these prefs here, you should also
disable
> their use in about_flags.cc

Ya, I guess it would crash when we tried to read it. I just added guards around
the code for retrieving/setting them because it seems like they're unmodifiable
if you can't access this ui.

Also, I haven't changed the test because I can't run it, so I'll have to come
back to it once we can link.

[Andrew T Wilson (Slow), 2012-03-27 17:33:16]

http://codereview.chromium.org/9836103/diff/6002/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

http://codereview.chromium.org/9836103/diff/6002/chrome/browser/about_flags.c...
chrome/browser/about_flags.cc:679: NOTIMPLEMENTED() << "Experimental labs aren't
supported on Android.";
I think it would be ideal if you could remove about_flags and FlagsUI entirely
rather than #ifing the files.

http://codereview.chromium.org/9836103/diff/6002/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service.cc (right):

http://codereview.chromium.org/9836103/diff/6002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:968: void
ProfileSyncService::ShowErrorUI() {
Might as well put a NOTREACHED here for android. This routine will go away soon
anyway as part of moving all the UI stuff out of ProfileSyncService.

http://codereview.chromium.org/9836103/diff/6002/chrome/browser/ui/sync/one_c...
File chrome/browser/ui/sync/one_click_signin_sync_starter.cc (right):

http://codereview.chromium.org/9836103/diff/6002/chrome/browser/ui/sync/one_c...
chrome/browser/ui/sync/one_click_signin_sync_starter.cc:58: // Android uses
native UI for sync setup.
I think we should just remove all of the one_click_signin code for Android. We
don't need it and in fact it'd be actively harmful to include it.

http://codereview.chromium.org/9836103/diff/6002/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/9836103/diff/6002/chrome/chrome_browser.gypi#n...
chrome/chrome_browser.gypi:4435: 'browser/ui/webui/sync_setup_handler.cc',
Add the one_click stuff here too.

[Evan Stade, 2012-03-27 18:03:02]

webui lgtm

[battre, 2012-03-27 18:54:30]

I'll defer to atwilson and the try bots for the remainder, so that you are not
blocked on me (CEST timezone).

LGTM

http://codereview.chromium.org/9836103/diff/3010/chrome/browser/prefs/browser...
File chrome/browser/prefs/browser_prefs.cc (right):

http://codereview.chromium.org/9836103/diff/3010/chrome/browser/prefs/browser...
chrome/browser/prefs/browser_prefs.cc:7: #include "chrome/browser/about_flags.h"
do you need to #ifdef this if you remove about_flags.h from the gyp files?

http://codereview.chromium.org/9836103/diff/3010/chrome/browser/ui/webui/hist...
File chrome/browser/ui/webui/history_ui.cc (right):

http://codereview.chromium.org/9836103/diff/3010/chrome/browser/ui/webui/hist...
chrome/browser/ui/webui/history_ui.cc:263: NOTIMPLEMENTED() << "TODO(yfriedman):
Upstream the Android version.";
Does the string end in the release binary? In that case, I would move it into a
comment.

[Yaron, 2012-03-27 19:06:52]

Thanks for the quick reviews. Another pass with about_flags removed.

http://codereview.chromium.org/9836103/diff/6002/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

http://codereview.chromium.org/9836103/diff/6002/chrome/browser/about_flags.c...
chrome/browser/about_flags.cc:679: NOTIMPLEMENTED() << "Experimental labs aren't
supported on Android.";
On 2012/03/27 17:33:16, Andrew T Wilson wrote:
> I think it would be ideal if you could remove about_flags and FlagsUI entirely
> rather than #ifing the files.

Done.

http://codereview.chromium.org/9836103/diff/6002/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service.cc (right):

http://codereview.chromium.org/9836103/diff/6002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:968: void
ProfileSyncService::ShowErrorUI() {
On 2012/03/27 17:33:16, Andrew T Wilson wrote:
> Might as well put a NOTREACHED here for android. This routine will go away
soon
> anyway as part of moving all the UI stuff out of ProfileSyncService.

Done.

http://codereview.chromium.org/9836103/diff/6002/chrome/browser/ui/sync/one_c...
File chrome/browser/ui/sync/one_click_signin_sync_starter.cc (right):

http://codereview.chromium.org/9836103/diff/6002/chrome/browser/ui/sync/one_c...
chrome/browser/ui/sync/one_click_signin_sync_starter.cc:58: // Android uses
native UI for sync setup.
On 2012/03/27 17:33:16, Andrew T Wilson wrote:
> I think we should just remove all of the one_click_signin code for Android. We
> don't need it and in fact it'd be actively harmful to include it.

Done.

http://codereview.chromium.org/9836103/diff/6002/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/9836103/diff/6002/chrome/chrome_browser.gypi#n...
chrome/chrome_browser.gypi:4435: 'browser/ui/webui/sync_setup_handler.cc',
On 2012/03/27 17:33:16, Andrew T Wilson wrote:
> Add the one_click stuff here too.

Done.

http://codereview.chromium.org/9836103/diff/3010/chrome/browser/prefs/browser...
File chrome/browser/prefs/browser_prefs.cc (right):

http://codereview.chromium.org/9836103/diff/3010/chrome/browser/prefs/browser...
chrome/browser/prefs/browser_prefs.cc:7: #include "chrome/browser/about_flags.h"
On 2012/03/27 18:54:30, battre wrote:
> do you need to #ifdef this if you remove about_flags.h from the gyp files?

Technically it's fine as the header is still included regardless of gyp. It just
means that what could be a compiler error becomes a linker error. That seems
like a "good thing" but it would result in many more if-defs and at least it's
not a runtime error. For that reason, I've err'd on the side of less if-defs and
not if-defing includes.

http://codereview.chromium.org/9836103/diff/3010/chrome/browser/ui/webui/hist...
File chrome/browser/ui/webui/history_ui.cc (right):

http://codereview.chromium.org/9836103/diff/3010/chrome/browser/ui/webui/hist...
chrome/browser/ui/webui/history_ui.cc:263: NOTIMPLEMENTED() << "TODO(yfriedman):
Upstream the Android version.";
On 2012/03/27 18:54:30, battre wrote:
> Does the string end in the release binary? In that case, I would move it into
a
> comment.

I don't believe it ends up in a release binary and I prefer to have it being
more descriptive. Regardless, it's Android-specific and we're quite a ways off
of being able to build off upstream.

http://codereview.chromium.org/9836103/diff/11001/chrome/browser/chrome_brows...
File chrome/browser/chrome_browser_main.cc (right):

http://codereview.chromium.org/9836103/diff/11001/chrome/browser/chrome_brows...
chrome/browser/chrome_browser_main.cc:1303:
local_state_->UpdateCommandLinePrefStore(CommandLine::ForCurrentProcess());
It's not clear to me if this should be in the if-def as well. It seems like it's
for applying the changes that were made to the commandline but I don't know if
there's an implicit dep on this running from elsewhere.

[Andrew T Wilson (Slow), 2012-03-27 20:00:18]

lgtm

LGTM

[commit-bot: I haz the power, 2012-03-27 20:33:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yfriedman@chromium.org/9836103/11001

[commit-bot: I haz the power, 2012-03-27 20:33:04]

Can't apply patch for file chrome/browser/prefs/browser_prefs.cc.
While running patch -p1 --forward --force;
patching file chrome/browser/prefs/browser_prefs.cc
Hunk #1 succeeded at 113 with fuzz 1 (offset 1 line).
Hunk #2 FAILED at 145.
1 out of 2 hunks FAILED -- saving rejects to file
chrome/browser/prefs/browser_prefs.cc.rej

[commit-bot: I haz the power, 2012-03-27 22:48:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yfriedman@chromium.org/9836103/14001

[commit-bot: I haz the power, 2012-03-27 22:57:02]

Try job failure for 9836103-14001 on win for step "update".
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-03-27 23:05:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yfriedman@chromium.org/9836103/14001

[commit-bot: I haz the power, 2012-03-27 23:48:58]

Try job failure for 9836103-14001 on win for step "update".
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-03-28 01:36:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yfriedman@chromium.org/9836103/14001

[commit-bot: I haz the power, 2012-03-28 04:54:47]

Try job failure for 9836103-14001 (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-28 14:06:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yfriedman@chromium.org/9836103/14001

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

Change committed as 129422