android build: progress on content and libjingle.

android build: progress on content and libjingle.


BUG=None
TEST=


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

[jam, 2012-01-23 05:11:56]

http://codereview.chromium.org/9271028/diff/1/content/app/content_main.cc
File content/app/content_main.cc (right):

http://codereview.chromium.org/9271028/diff/1/content/app/content_main.cc#new...
content/app/content_main.cc:145: !defined(OS_LINUX) && !defined(OS_ANDROID)
curious: why not do this now for linux?

http://codereview.chromium.org/9271028/diff/1/content/content_renderer.gypi
File content/content_renderer.gypi (right):

http://codereview.chromium.org/9271028/diff/1/content/content_renderer.gypi#n...
content/content_renderer.gypi:265: # TODO(jrg): remove the OS=="android"
section?
TODOs have a habit of hanging around too long :) can this change be put off
until this is better understood?

http://codereview.chromium.org/9271028/diff/1/third_party/libjingle/libjingle...
File third_party/libjingle/libjingle.gyp (right):

http://codereview.chromium.org/9271028/diff/1/third_party/libjingle/libjingle...
third_party/libjingle/libjingle.gyp:83: ],
i'm not family with libjingle, so perhaps whoever committed this gyp file should
review this?

[jam, 2012-01-23 17:13:13]

http://codereview.chromium.org/9271028/diff/1/content/browser/browsing_instan...
File content/browser/browsing_instance.cc (right):

http://codereview.chromium.org/9271028/diff/1/content/browser/browsing_instan...
content/browser/browsing_instance.cc:19: struct hash<content::BrowserContext*> {
btw i just noticed that browser_context already has this, behind an 

#if defined(COMPILER_GCC) && !defined(OS_ANDROID)

so just take out the android part of the ifdef there

[michaelbai, 2012-01-23 17:26:28]

Do we need libjingle?

[John Grabowski, 2012-01-25 20:29:15]

Michael: for now, yes, I believe we do need libjingle.  Let's discuss offline.
JAM: all feedback applied or offline email sent.

http://codereview.chromium.org/9271028/diff/1/content/app/content_main.cc
File content/app/content_main.cc (right):

http://codereview.chromium.org/9271028/diff/1/content/app/content_main.cc#new...
content/app/content_main.cc:145: !defined(OS_LINUX) && !defined(OS_ANDROID)
On 2012/01/23 05:11:56, John Abd-El-Malek wrote:
> curious: why not do this now for linux?

That shouldn't be in there; removed.

http://codereview.chromium.org/9271028/diff/1/content/content_renderer.gypi
File content/content_renderer.gypi (right):

http://codereview.chromium.org/9271028/diff/1/content/content_renderer.gypi#n...
content/content_renderer.gypi:265: # TODO(jrg): remove the OS=="android"
section?
On 2012/01/23 05:11:56, John Abd-El-Malek wrote:
> TODOs have a habit of hanging around too long :) can this change be put off
> until this is better understood?

Sent email

http://codereview.chromium.org/9271028/diff/1/third_party/libjingle/libjingle...
File third_party/libjingle/libjingle.gyp (right):

http://codereview.chromium.org/9271028/diff/1/third_party/libjingle/libjingle...
third_party/libjingle/libjingle.gyp:83: ],
On 2012/01/23 05:11:56, John Abd-El-Malek wrote:
> i'm not family with libjingle, so perhaps whoever committed this gyp file
should
> review this?

Removing from CL

[jam, 2012-01-25 21:27:52]

http://codereview.chromium.org/9271028/diff/1/content/browser/browsing_instan...
File content/browser/browsing_instance.cc (right):

http://codereview.chromium.org/9271028/diff/1/content/browser/browsing_instan...
content/browser/browsing_instance.cc:19: struct hash<content::BrowserContext*> {
On 2012/01/23 17:13:13, John Abd-El-Malek wrote:
> btw i just noticed that browser_context already has this, behind an 
> 
> #if defined(COMPILER_GCC) && !defined(OS_ANDROID)
> 
> so just take out the android part of the ifdef there

what about this one?

[John Grabowski, 2012-01-25 21:48:07]

On 2012/01/25 21:27:52, John Abd-El-Malek wrote:
>
http://codereview.chromium.org/9271028/diff/1/content/browser/browsing_instan...
> File content/browser/browsing_instance.cc (right):
> 
>
http://codereview.chromium.org/9271028/diff/1/content/browser/browsing_instan...
> content/browser/browsing_instance.cc:19: struct hash<content::BrowserContext*>
{
> On 2012/01/23 17:13:13, John Abd-El-Malek wrote:
> > btw i just noticed that browser_context already has this, behind an 
> > 
> > #if defined(COMPILER_GCC) && !defined(OS_ANDROID)
> > 
> > so just take out the android part of the ifdef there
> 
> what about this one?

Sorry... got lost.  New diffs up.

[John Grabowski, 2012-01-26 05:37:09]

On 2012/01/25 21:48:07, John Grabowski wrote:
> On 2012/01/25 21:27:52, John Abd-El-Malek wrote:
> >
>
http://codereview.chromium.org/9271028/diff/1/content/browser/browsing_instan...
> > File content/browser/browsing_instance.cc (right):
> > 
> >
>
http://codereview.chromium.org/9271028/diff/1/content/browser/browsing_instan...
> > content/browser/browsing_instance.cc:19: struct
hash<content::BrowserContext*>
> {
> > On 2012/01/23 17:13:13, John Abd-El-Malek wrote:
> > > btw i just noticed that browser_context already has this, behind an 
> > > 
> > > #if defined(COMPILER_GCC) && !defined(OS_ANDROID)
> > > 
> > > so just take out the android part of the ifdef there
> > 
> > what about this one?
> 
> Sorry... got lost.  New diffs up.

To be clear: feedback applied, which means browsing_instance.cc is no longer
part of this CL (but browser_context.h is newly added).

[jam, 2012-01-26 05:44:59]

lgtm

sorry I missed your earlier email

[commit-bot: I haz the power, 2012-01-26 21:08:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jrg@chromium.org/9271028/8002

[commit-bot: I haz the power, 2012-01-26 21:50:39]

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

[commit-bot: I haz the power, 2012-01-26 23:35:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jrg@chromium.org/9271028/7005

[commit-bot: I haz the power, 2012-01-27 01:57:13]

Change committed as 119334