Chrome on Android : Begin removing references to themes.

Chrome on Android : Begin removing references to themes.

Android will not in the forseeeable future be supporting themes, so this begins
removing their support to get unit_tests linking upstream for android.

BUG=113487
TEST=

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

[Yaron, 2012-02-29 00:02:17]

http://codereview.chromium.org/9515010/diff/1/chrome/browser/prefs/browser_pr...
File chrome/browser/prefs/browser_prefs.cc (right):

http://codereview.chromium.org/9515010/diff/1/chrome/browser/prefs/browser_pr...
chrome/browser/prefs/browser_prefs.cc:212:
SyncPromoUI::RegisterUserPrefs(user_prefs);
I think this is merged in from Jesse's change.

http://codereview.chromium.org/9515010/diff/1/chrome/browser/ui/browser.cc
File chrome/browser/ui/browser.cc (right):

http://codereview.chromium.org/9515010/diff/1/chrome/browser/ui/browser.cc#ne...
chrome/browser/ui/browser.cc:373: registrar_.Add(
Please add TODO(yfriedman): Remove this ifdef when browser is excluded from
Android builds.

http://codereview.chromium.org/9515010/diff/1/chrome/browser/ui/webui/ntp/ntp...
File chrome/browser/ui/webui/ntp/ntp_resource_cache_android.cc (right):

http://codereview.chromium.org/9515010/diff/1/chrome/browser/ui/webui/ntp/ntp...
chrome/browser/ui/webui/ntp/ntp_resource_cache_android.cc:2: // Use of this
source code is governed by a BSD-style license that can be
This looks different from downstream. Have you tried applying these changes
there and see that it works?

[Ted C, 2012-03-01 18:16:02]

Oh sorry, forgot to say what people should look at:

erg:chrome/browser/profile
atwilson:chrome/browser/sync
ben:chrome/browser/ui/...

Thanks

[Elliot Glaysher, 2012-03-02 00:23:05]

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile.cc (right):

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/profiles/prof...
chrome/browser/profiles/profile.cc:131: // Themes are not supported in android.
This block is going away and is being moved to the ThemeServiceFactory, but it
depends on the ExtensionServiceFactory landing first. Fine for now, just FYI.

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service_factory.cc (right):

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service_factory.cc:48: #if !defined(OS_ANDROID)
Is this enough? I thought that there was additional sync datatype registration.

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/ui/browser.cc
File chrome/browser/ui/browser.cc (right):

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/ui/browser.cc...
chrome/browser/ui/browser.cc:373: #if !defined(OS_ANDROID)
It would really be nice if you had a cleaner way to do this, since most of the
rest of this patch is this same pattern, over and over.

[Elliot Glaysher, 2012-03-05 23:26:43]

+mirandac

[Elliot Glaysher, 2012-03-07 23:52:46]

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/ui/browser.cc
File chrome/browser/ui/browser.cc (right):

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/ui/browser.cc...
chrome/browser/ui/browser.cc:4413: case
chrome::NOTIFICATION_BROWSER_THEME_CHANGED:
#ifdef this part out too

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/ui/webui/ntp/...
File chrome/browser/ui/webui/ntp/new_tab_ui.cc (right):

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/ui/webui/ntp/...
chrome/browser/ui/webui/ntp/new_tab_ui.cc:139: #if !defined(OS_ANDROID)
Comment about why this is temporary

[Ted C, 2012-03-08 19:42:07]

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile.cc (right):

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/profiles/prof...
chrome/browser/profiles/profile.cc:131: // Themes are not supported in android.
On 2012/03/02 00:23:05, Elliot Glaysher wrote:
> This block is going away and is being moved to the ThemeServiceFactory, but it
> depends on the ExtensionServiceFactory landing first. Fine for now, just FYI.

Hazaa...yay for making my patch smaller.  This file will no longer have any
changes.

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service_factory.cc (right):

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service_factory.cc:48: #if !defined(OS_ANDROID)
On 2012/03/02 00:23:05, Elliot Glaysher wrote:
> Is this enough? I thought that there was additional sync datatype
registration.

This removed the only link errors I was seeing, but I may need to expand once we
try out additional targets.

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/ui/browser.cc
File chrome/browser/ui/browser.cc (right):

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/ui/browser.cc...
chrome/browser/ui/browser.cc:373: #if !defined(OS_ANDROID)
On 2012/03/02 00:23:05, Elliot Glaysher wrote:
> It would really be nice if you had a cleaner way to do this, since most of the
> rest of this patch is this same pattern, over and over.

I added a TODO of why this particular change will go away as Android will not be
using browser.cc in the long term.

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/ui/browser.cc...
chrome/browser/ui/browser.cc:4413: case
chrome::NOTIFICATION_BROWSER_THEME_CHANGED:
On 2012/03/07 23:52:46, Elliot Glaysher wrote:
> #ifdef this part out too

Done.

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/ui/webui/ntp/...
File chrome/browser/ui/webui/ntp/new_tab_ui.cc (right):

http://codereview.chromium.org/9515010/diff/5001/chrome/browser/ui/webui/ntp/...
chrome/browser/ui/webui/ntp/new_tab_ui.cc:139: #if !defined(OS_ANDROID)
On 2012/03/07 23:52:46, Elliot Glaysher wrote:
> Comment about why this is temporary

Done.  Comment added.  I will also look into creating an android specific
version of this file in the short term as I did with the ntp_resource_cache

[Elliot Glaysher, 2012-03-08 20:05:10]

lgtm

[Ben Goodger (Google), 2012-03-08 23:33:48]

http://codereview.chromium.org/9515010/diff/13012/chrome/browser/ui/browser.cc
File chrome/browser/ui/browser.cc (right):

http://codereview.chromium.org/9515010/diff/13012/chrome/browser/ui/browser.c...
chrome/browser/ui/browser.cc:375: #if !defined(OS_ANDROID)
akin to my comment in the other CL: USE_THEMES?

[Ted C, 2012-03-12 22:55:38]

On 2012/03/08 23:33:48, Ben Goodger (Google) wrote:
> http://codereview.chromium.org/9515010/diff/13012/chrome/browser/ui/browser.cc
> File chrome/browser/ui/browser.cc (right):
> 
>
http://codereview.chromium.org/9515010/diff/13012/chrome/browser/ui/browser.c...
> chrome/browser/ui/browser.cc:375: #if !defined(OS_ANDROID)
> akin to my comment in the other CL: USE_THEMES?

Done.  I have added ENABLE_THEMES (for naming consistency) and updated the
references and did a few more exclusions.  This is not everything, but seems
like a good enough start.

PTAL

[Ben Goodger (Google), 2012-03-12 23:33:12]

lgtm

[Ted C, 2012-03-14 16:47:46]

+akalin

[akalin, 2012-03-15 11:24:25]

sync LGTM

although you may want to edit the CL description -- sync_unit_tests should now
link for android without this change (it just pushes the problem to unit_tests,
I think)

[commit-bot: I haz the power, 2012-03-15 17:22:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tedchoc@chromium.org/9515010/26001

[commit-bot: I haz the power, 2012-03-15 20:50:41]

Change committed as 126988