Android content shell bringup.

Android content shell bringup.

Build media java files (we weren't).
Fix adb_install_content_shell for cases where the app was stuck.
Add upstream staging gyp var / #define.
Be more consistent about jar output files (all in lib.java).
Upstream a bunch of random files (e.g. ppapi).
Upstream a bunch of java and native code hit as part of shlib init.
Properly package jar files in content shell.

BUG=
TEST=


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

[John Grabowski, 2012-05-08 23:46:24]

Please review all:
yaron, satish (if available)

OWNERS:
mento: base/
jam: content/
fishman: media/

[Ami GONE FROM CHROMIUM, 2012-05-08 23:54:32]

LGTM for media/media.gyp.  
I have no idea what media/base/android/java/media.xml is doing and expect one
Yaron/Satish will cover that.

[Mark Mentovai, 2012-05-09 00:05:36]

LGTM. I only reviewed the files in base and build.

http://chromiumcodereview.appspot.com/10377059/diff/1/build/java.gypi
File build/java.gypi (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/build/java.gypi#newcode43
build/java.gypi:43: '<(PRODUCT_DIR)/lib.java/chromium_<(package_name).jar',
Fix the indentation here.

[Ted C, 2012-05-09 00:26:41]

http://chromiumcodereview.appspot.com/10377059/diff/1/content/browser/android...
File content/browser/android/command_line.cc (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/browser/android...
content/browser/android/command_line.cc:1: // Copyright (c) 2011 The Chromium
Authors. All rights reserved.
2012

http://chromiumcodereview.appspot.com/10377059/diff/1/content/browser/android...
File content/browser/android/command_line.h (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/browser/android...
content/browser/android/command_line.h:1: // Copyright (c) 2011 The Chromium
Authors. All rights reserved.
2012

http://chromiumcodereview.appspot.com/10377059/diff/1/content/browser/android...
File content/browser/android/jni_helper.cc (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/browser/android...
content/browser/android/jni_helper.cc:1: // Copyright (c) 2010 The Chromium
Authors. All rights reserved.
2012

http://chromiumcodereview.appspot.com/10377059/diff/1/content/browser/android...
content/browser/android/jni_helper.cc:25: AutoLocalFrame::AutoLocalFrame(int
capacity) {
should this all be namespaced to content/browser?

http://chromiumcodereview.appspot.com/10377059/diff/1/content/browser/android...
File content/browser/android/jni_helper.h (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/browser/android...
content/browser/android/jni_helper.h:1: // Copyright (c) 2010 The Chromium
Authors. All rights reserved.
2012

http://chromiumcodereview.appspot.com/10377059/diff/1/content/browser/android...
File content/browser/android/library_loader_hooks.cc (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/browser/android...
content/browser/android/library_loader_hooks.cc:31: namespace android {
create a content_jni_registrar

http://chromiumcodereview.appspot.com/10377059/diff/1/content/browser/android...
content/browser/android/library_loader_hooks.cc:106:
"org/chromium/content/browser/LibraryLoader";
+2 indent

http://chromiumcodereview.appspot.com/10377059/diff/1/content/browser/android...
File content/browser/android/trace_event_binding.h (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/browser/android...
content/browser/android/trace_event_binding.h:1: // Copyright (c) 2011 The
Chromium Authors. All rights reserved.
i suspect you are getting the idea.

http://chromiumcodereview.appspot.com/10377059/diff/1/content/content_shell.gypi
File content/content_shell.gypi (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/content_shell.g...
content/content_shell.gypi:539: 'outputs':
['<(PRODUCT_DIR)/content_shell/java/libs/chromium_media.jar'],
add as input to content_shell_apk...same with the above

http://chromiumcodereview.appspot.com/10377059/diff/1/content/ppapi_plugin/pp...
File content/ppapi_plugin/ppapi_webkitplatformsupport_impl.cc (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/ppapi_plugin/pp...
content/ppapi_plugin/ppapi_webkitplatformsupport_impl.cc:138: // TODO(jrg):
implement
PpapiWebKitPlatformSupportImpl::SandboxSupport::getFontFamilyForCharacters()
>80?

http://chromiumcodereview.appspot.com/10377059/diff/1/content/public/android/...
File content/public/android/java/org/chromium/content/browser/CommandLine.java
(right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/public/android/...
content/public/android/java/org/chromium/content/browser/CommandLine.java:113:
// Mirrors of switches defined in native code.  It is up to you,
We probably don't want to include any of the flags above this.

http://chromiumcodereview.appspot.com/10377059/diff/1/content/shell/android/j...
File
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java
(right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/shell/android/j...
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java:42:
// Debug.waitForDebugger();
remove?

http://chromiumcodereview.appspot.com/10377059/diff/1/content/shell/android/j...
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java:43:
LibraryLoader.setLibraryToLoad(NATIVE_LIBRARY);
specifying the library to load should go into ContentShellApplication so it will
be shared with the sandbox processes

[Yaron, 2012-05-09 01:01:42]

http://chromiumcodereview.appspot.com/10377059/diff/1/build/android/adb_insta...
File build/android/adb_install_content_shell (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/build/android/adb_insta...
build/android/adb_install_content_shell:7: # install -r doesn't always work? 
Try uninstalling first.
Really? Why not?

http://chromiumcodereview.appspot.com/10377059/diff/1/build/android/gdb_conte...
File build/android/gdb_content_shell (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/build/android/gdb_conte...
build/android/gdb_content_shell:75: # set solib-absolute-prefix null
Can you remove instead? Why is this needed?

http://chromiumcodereview.appspot.com/10377059/diff/1/build/android/gdb_conte...
build/android/gdb_content_shell:89: # ${gdb} -x $cmdfile $* $app_process
Remove

http://chromiumcodereview.appspot.com/10377059/diff/1/content/content.gyp
File content/content.gyp (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/content.gyp#new...
content/content.gyp:215: 'includes': [ 'content_jni.gypi' ],
Wheres' this file?

http://chromiumcodereview.appspot.com/10377059/diff/1/content/content_browser...
File content/content_browser.gypi (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/content_browser...
content/content_browser.gypi:184: 'browser/android/command_line.h',
sort

http://chromiumcodereview.appspot.com/10377059/diff/1/content/ppapi_plugin/pp...
File content/ppapi_plugin/ppapi_webkitplatformsupport_impl.cc (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/ppapi_plugin/pp...
content/ppapi_plugin/ppapi_webkitplatformsupport_impl.cc:88: // TODO(jeremy):
Remove once http://webk.it/66935 lands.
Why are you adding this to the mac section? Remove this part.

http://chromiumcodereview.appspot.com/10377059/diff/1/content/ppapi_plugin/pp...
content/ppapi_plugin/ppapi_webkitplatformsupport_impl.cc:137: #if
defined(OS_ANDROID)
Isn't this dead code? I think it's just bit-rot. With the if for ANDROID above,
this won't get evaluated.

http://chromiumcodereview.appspot.com/10377059/diff/1/content/ppapi_plugin/pp...
content/ppapi_plugin/ppapi_webkitplatformsupport_impl.cc:152: #if
defined(OS_ANDROID)
same hedre. please remove

http://chromiumcodereview.appspot.com/10377059/diff/1/content/public/android/...
File content/public/android/java/org/chromium/content/browser/LibraryLoader.java
(right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/public/android/...
content/public/android/java/org/chromium/content/browser/LibraryLoader.java:22:
* upstream libchromeview.so */
There may still be a libchromeview.so though. If there's anything above content
that needs to be upstreamed.

http://chromiumcodereview.appspot.com/10377059/diff/1/content/shell/android/c...
File content/shell/android/content_shell_apk.xml (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/shell/android/c...
content/shell/android/content_shell_apk.xml:25: <property
name="native.libs.absolute.dir" location="${out.dir}/libs" />
Fix indentation

http://chromiumcodereview.appspot.com/10377059/diff/1/content/shell/android/s...
File content/shell/android/shell_library_loader.cc (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/shell/android/s...
content/shell/android/shell_library_loader.cc:22: content::ContentMainRunner*
gContentMainRunner = NULL;
Shouldn't this be g_content_main_runner?

http://chromiumcodereview.appspot.com/10377059/diff/1/content/shell/android/s...
content/shell/android/shell_library_loader.cc:39: // That must be done before
ContentMainRunner::Initialize().
This pairs with the "return 0" early-out in content_main_runner.cc, right? If
so, please TODO remove this.

http://chromiumcodereview.appspot.com/10377059/diff/1/media/base/android/java...
File media/base/android/java/media.xml (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/media/base/android/java...
media/base/android/java/media.xml:18: <property name="jar.dir"
location="${out.dir}"/>
jar.dir is redundant since we have out.dir. It's kinda more descriptive though
so better if you rename out.dir (and change other ant buildfiles). We should
really just templatize this already to avoid this problem.

http://chromiumcodereview.appspot.com/10377059/diff/1/media/base/android/java...
media/base/android/java/media.xml:31: <mkdir dir="${dest.dir}"/>
I don't see dest.dir specified as a property. Can you standardize this with
base.xml and content.xml. I think content has it best with "classes.dir" and set
it to ${out.dir}/java/${PACKAGE_NAME}

http://chromiumcodereview.appspot.com/10377059/diff/1/media/base/android/java...
media/base/android/java/media.xml:36: <!-- Compile the java code from ${src}
into ${build} -->
Comment shoudl be "... into ${classes.dir}"

[John Grabowski, 2012-05-09 17:18:15]

Feedback applied except where noted.

http://chromiumcodereview.appspot.com/10377059/diff/1/build/android/adb_insta...
File build/android/adb_install_content_shell (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/build/android/adb_insta...
build/android/adb_install_content_shell:7: # install -r doesn't always work? 
Try uninstalling first.
On 2012/05/09 01:01:42, Yaron wrote:
> Really? Why not?

Not clear.  Maybe when the Eclipse debugger is attached.  In any case, this
definitely makes things more reliable.

http://chromiumcodereview.appspot.com/10377059/diff/1/build/android/gdb_conte...
File build/android/gdb_content_shell (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/build/android/gdb_conte...
build/android/gdb_content_shell:75: # set solib-absolute-prefix null
On 2012/05/09 01:01:42, Yaron wrote:
> Can you remove instead? Why is this needed?

This work isn't done; still not happy with debuggability.
Can we defer cleanup until this settles down?

http://chromiumcodereview.appspot.com/10377059/diff/1/content/browser/android...
File content/browser/android/library_loader_hooks.cc (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/browser/android...
content/browser/android/library_loader_hooks.cc:106:
"org/chromium/content/browser/LibraryLoader";
On 2012/05/09 00:26:41, Ted C wrote:
> +2 indent

The style guide is ambiguous on this point (as far as I can tell).  This is not
a wrapped parameter; default indentation is 2.  So I disagree.

http://chromiumcodereview.appspot.com/10377059/diff/1/content/content.gyp
File content/content.gyp (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/content.gyp#new...
content/content.gyp:215: 'includes': [ 'content_jni.gypi' ],
On 2012/05/09 01:01:42, Yaron wrote:
> Wheres' this file?

Oops!  Added

http://chromiumcodereview.appspot.com/10377059/diff/1/content/ppapi_plugin/pp...
File content/ppapi_plugin/ppapi_webkitplatformsupport_impl.cc (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/ppapi_plugin/pp...
content/ppapi_plugin/ppapi_webkitplatformsupport_impl.cc:88: // TODO(jeremy):
Remove once http://webk.it/66935 lands.
On 2012/05/09 01:01:42, Yaron wrote:
> Why are you adding this to the mac section? Remove this part.

You are absolutely right; removing from downstream.

http://chromiumcodereview.appspot.com/10377059/diff/1/content/public/android/...
File content/public/android/java/org/chromium/content/browser/CommandLine.java
(right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/public/android/...
content/public/android/java/org/chromium/content/browser/CommandLine.java:113:
// Mirrors of switches defined in native code.  It is up to you,
On 2012/05/09 00:26:41, Ted C wrote:
> We probably don't want to include any of the flags above this.

I'm not sure I agree with that.  These are merely string constants.  Including
them here doesn't enable them as such.  If we disable them for an
official/release build, we'd need to disable them in native code.  I'm not sure
there is really a need to split this file into upstreamable/not-upstreamable
strings.

http://chromiumcodereview.appspot.com/10377059/diff/1/content/public/android/...
File content/public/android/java/org/chromium/content/browser/LibraryLoader.java
(right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/public/android/...
content/public/android/java/org/chromium/content/browser/LibraryLoader.java:22:
* upstream libchromeview.so */
On 2012/05/09 01:01:42, Yaron wrote:
> There may still be a libchromeview.so though. If there's anything above
content
> that needs to be upstreamed.

Perhaps; naming for action at that level is a bit unknown (e.g. maybe it'll be a
libchromium.so, since it's not view-specific).  I think TODO is the right choice
for now.

http://chromiumcodereview.appspot.com/10377059/diff/1/content/shell/android/j...
File
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java
(right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/shell/android/j...
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java:42:
// Debug.waitForDebugger();
On 2012/05/09 00:26:41, Ted C wrote:
> remove?

Not until we have command line support (for --wait-for-debugger).  TODO added.

http://chromiumcodereview.appspot.com/10377059/diff/1/content/shell/android/s...
File content/shell/android/shell_library_loader.cc (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/shell/android/s...
content/shell/android/shell_library_loader.cc:39: // That must be done before
ContentMainRunner::Initialize().
On 2012/05/09 01:01:42, Yaron wrote:
> This pairs with the "return 0" early-out in content_main_runner.cc, right? If
> so, please TODO remove this.

That isn't clear.  If we don't PreSandboxStartup(), we'll crash before the
early-out.

http://chromiumcodereview.appspot.com/10377059/diff/1/media/base/android/java...
File media/base/android/java/media.xml (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/media/base/android/java...
media/base/android/java/media.xml:18: <property name="jar.dir"
location="${out.dir}"/>
On 2012/05/09 01:01:42, Yaron wrote:
> jar.dir is redundant since we have out.dir. It's kinda more descriptive though
> so better if you rename out.dir (and change other ant buildfiles). We should
> really just templatize this already to avoid this problem.

Problem was I'm trying to be more consistent with both base and content_apk.
Made more consistent (with base), and yes will template next.

[Yaron, 2012-05-09 17:31:46]

Just the one issue remaining for me

http://chromiumcodereview.appspot.com/10377059/diff/1/content/shell/android/s...
File content/shell/android/shell_library_loader.cc (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/shell/android/s...
content/shell/android/shell_library_loader.cc:39: // That must be done before
ContentMainRunner::Initialize().
On 2012/05/09 17:18:15, John Grabowski wrote:
> On 2012/05/09 01:01:42, Yaron wrote:
> > This pairs with the "return 0" early-out in content_main_runner.cc, right?
If
> > so, please TODO remove this.
> 
> That isn't clear.  If we don't PreSandboxStartup(), we'll crash before the
> early-out.  

Well, downstream we don't need to do this. Also, if you look at
content_main_runner, just below your early-out we attempt to call this. What you
have seems transitional.

[John Grabowski, 2012-05-09 17:44:34]

http://chromiumcodereview.appspot.com/10377059/diff/1/content/shell/android/s...
File content/shell/android/shell_library_loader.cc (right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/shell/android/s...
content/shell/android/shell_library_loader.cc:39: // That must be done before
ContentMainRunner::Initialize().
On 2012/05/09 17:31:47, Yaron wrote:
> On 2012/05/09 17:18:15, John Grabowski wrote:
> > On 2012/05/09 01:01:42, Yaron wrote:
> > > This pairs with the "return 0" early-out in content_main_runner.cc, right?
> If
> > > so, please TODO remove this.
> > 
> > That isn't clear.  If we don't PreSandboxStartup(), we'll crash before the
> > early-out.  
> 
> Well, downstream we don't need to do this. Also, if you look at
> content_main_runner, just below your early-out we attempt to call this. What
you
> have seems transitional.

TODO added

[Yaron, 2012-05-09 17:49:23]

lgtm

[Ted C, 2012-05-09 18:12:54]

lgtm

Although I disagree with the CommandLine stuff, it's not a blocker for me.

http://chromiumcodereview.appspot.com/10377059/diff/1/content/public/android/...
File content/public/android/java/org/chromium/content/browser/CommandLine.java
(right):

http://chromiumcodereview.appspot.com/10377059/diff/1/content/public/android/...
content/public/android/java/org/chromium/content/browser/CommandLine.java:113:
// Mirrors of switches defined in native code.  It is up to you,
On 2012/05/09 17:18:15, John Grabowski wrote:
> On 2012/05/09 00:26:41, Ted C wrote:
> > We probably don't want to include any of the flags above this.
> 
> I'm not sure I agree with that.  These are merely string constants.  Including
> them here doesn't enable them as such.  If we disable them for an
> official/release build, we'd need to disable them in native code.  I'm not
sure
> there is really a need to split this file into upstreamable/not-upstreamable
> strings.

But by having them included, it implies they do actually do something. 
Consumers of this shouldn't have to dig around in the code to see what flags
work upstream and which don't.  

If you just want to put a TODO with my name on it to split it out into ones that
are used upstream, that is also fine.

[John Grabowski, 2012-05-09 18:21:41]

On 2012/05/09 18:12:54, Ted C wrote:
> lgtm
> 
> Although I disagree with the CommandLine stuff, it's not a blocker for me.
> 
>
http://chromiumcodereview.appspot.com/10377059/diff/1/content/public/android/...
> File content/public/android/java/org/chromium/content/browser/CommandLine.java
> (right):
> 
>
http://chromiumcodereview.appspot.com/10377059/diff/1/content/public/android/...
> content/public/android/java/org/chromium/content/browser/CommandLine.java:113:
> // Mirrors of switches defined in native code.  It is up to you,
> On 2012/05/09 17:18:15, John Grabowski wrote:
> > On 2012/05/09 00:26:41, Ted C wrote:
> > > We probably don't want to include any of the flags above this.
> > 
> > I'm not sure I agree with that.  These are merely string constants. 
Including
> > them here doesn't enable them as such.  If we disable them for an
> > official/release build, we'd need to disable them in native code.  I'm not
> sure
> > there is really a need to split this file into upstreamable/not-upstreamable
> > strings.
> 
> But by having them included, it implies they do actually do something. 
> Consumers of this shouldn't have to dig around in the code to see what flags
> work upstream and which don't.  
> 
> If you just want to put a TODO with my name on it to split it out into ones
that
> are used upstream, that is also fine.

TODO with both our names added.

[Avi (use Gerrit), 2012-05-09 18:48:28]

There is very little I can intelligently comment on. LGTM for owner role stuff;
can you tweak the comments that I noted?

http://chromiumcodereview.appspot.com/10377059/diff/1033/content/app/content_...
File content/app/content_main_runner.cc (right):

http://chromiumcodereview.appspot.com/10377059/diff/1033/content/app/content_...
content/app/content_main_runner.cc:377: // On Android, do not override the
signal handlers so that we can get stack
This comment and the one below are awkward; you describe what android doesn't do
and then you go and do it. I understand the flow, but can you figure out a
better way of comment them?

http://chromiumcodereview.appspot.com/10377059/diff/1033/content/browser/andr...
File content/browser/android/jni_helper.h (right):

http://chromiumcodereview.appspot.com/10377059/diff/1033/content/browser/andr...
content/browser/android/jni_helper.h:18: class Size;
no indentation

[John Grabowski, 2012-05-09 19:03:56]

Thx for the review.

http://chromiumcodereview.appspot.com/10377059/diff/1033/content/app/content_...
File content/app/content_main_runner.cc (right):

http://chromiumcodereview.appspot.com/10377059/diff/1033/content/app/content_...
content/app/content_main_runner.cc:377: // On Android, do not override the
signal handlers so that we can get stack
On 2012/05/09 18:48:28, Avi wrote:
> This comment and the one below are awkward; you describe what android doesn't
do
> and then you go and do it. I understand the flow, but can you figure out a
> better way of comment them?

Tried to clarify.

http://chromiumcodereview.appspot.com/10377059/diff/1033/content/browser/andr...
File content/browser/android/jni_helper.h (right):

http://chromiumcodereview.appspot.com/10377059/diff/1033/content/browser/andr...
content/browser/android/jni_helper.h:18: class Size;
On 2012/05/09 18:48:28, Avi wrote:
> no indentation

Done.

[Avi (use Gerrit), 2012-05-09 19:57:18]

http://chromiumcodereview.appspot.com/10377059/diff/8041/content/app/content_...
File content/app/content_main_runner.cc (right):

http://chromiumcodereview.appspot.com/10377059/diff/8041/content/app/content_...
content/app/content_main_runner.cc:371: #if !defined(OS_ANDROID)
Not what I meant. On line 401, the comment about how Android works is before the
ifdef, so that people could understand why it was if-ed out. Here you're saying
"disable on android" right before each item that you are calling, and the user
has to look up to see that they're in an ifdef block for it to make sense. Can
you please move your "we don't do this on android" comments to be close to the
ifdef, like you do on line 401, so that it's in a readable flow?

[John Grabowski, 2012-05-09 20:24:05]

On 2012/05/09 19:57:18, Avi wrote:
>
http://chromiumcodereview.appspot.com/10377059/diff/8041/content/app/content_...
> File content/app/content_main_runner.cc (right):
> 
>
http://chromiumcodereview.appspot.com/10377059/diff/8041/content/app/content_...
> content/app/content_main_runner.cc:371: #if !defined(OS_ANDROID)
> Not what I meant. On line 401, the comment about how Android works is before
the
> ifdef, so that people could understand why it was if-ed out. Here you're
saying
> "disable on android" right before each item that you are calling, and the user
> has to look up to see that they're in an ifdef block for it to make sense. Can
> you please move your "we don't do this on android" comments to be close to the
> ifdef, like you do on line 401, so that it's in a readable flow?

Fixed
Thx for review

[Avi (use Gerrit), 2012-05-09 21:01:34]

lgtm

http://chromiumcodereview.appspot.com/10377059/diff/9037/content/app/content_...
File content/app/content_main_runner.cc (right):

http://chromiumcodereview.appspot.com/10377059/diff/9037/content/app/content_...
content/app/content_main_runner.cc:376: // Thus, these are all disabled.
Thanks!

[John Grabowski, 2012-05-09 21:26:41]

+ evan for OWNERS of content/app/content_main_runner.cc

[Evan Martin, 2012-05-09 21:30:21]

The change LGTM, but you probably want to find someone else for your
reviews in the future as I no longer work on Chrome.  :)

On Wed, May 9, 2012 at 2:26 PM,  <jrg@chromium.org> wrote:
> + evan for OWNERS of content/app/content_main_runner.cc
>
>
> http://chromiumcodereview.appspot.com/10377059/

[commit-bot: I haz the power, 2012-05-09 23:22:06]

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

[commit-bot: I haz the power, 2012-05-09 23:36:47]

Try job failure for 10377059-43 (retry) on android for steps "Compile, build".
It's a second try, previously, steps "Compile, build" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android&nu...

[commit-bot: I haz the power, 2012-05-09 23:51:15]

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

[commit-bot: I haz the power, 2012-05-10 01:50:50]

Change committed as 136219