Add skeleton of content shell for android.

Add skeleton of content shell for android.

This is heavily based on http://codereview.chromium.org/9834037/ which added
an APK-based test runner for android. This CL adds a content_shell_apk target
for android containing an empty activity.

BUG=118591
TEST=build/test/install ContentShell-debug.apk on device

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

[Ted C, 2012-03-27 17:16:22]

lgtm

Nothing particularly blocking, although moving the java to -> src is probably
something we should do.

http://codereview.chromium.org/9860032/diff/1007/content/content.gyp
File content/content.gyp (right):

http://codereview.chromium.org/9860032/diff/1007/content/content.gyp#newcode14
content/content.gyp:14: 'shell/android/content_shell.gypi',
any reason we don't just put this in content/content_shell.gypi?

Don't know about precedence, so maybe this is just how it is done.

http://codereview.chromium.org/9860032/diff/1007/content/shell/android/conten...
File content/shell/android/content_shell.gypi (right):

http://codereview.chromium.org/9860032/diff/1007/content/shell/android/conten...
content/shell/android/content_shell.gypi:18: '<!@(find shell/android/java -name
"*.java")',
Not sure if we should start including other dependencies yet, but should we also
include:
base/android/java/* as well

Although, I'm not sure what our approach long term will be.  Are we going to
include all the .java files explicitly here or are we going to you other gyp
targets to build .jar files and we include the jar files?

http://codereview.chromium.org/9860032/diff/1007/content/shell/android/java/o...
File
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java
(right):

http://codereview.chromium.org/9860032/diff/1007/content/shell/android/java/o...
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java:1:
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
I think to make this easier to match other android applications, the path should
be include src before the org.

The typical layout I've seen is:

AndroidManifest.xml
src -> path/to/source
res -> drawables/layouts/etc...needed by the application

http://codereview.chromium.org/9860032/diff/1007/content/shell/android/java/o...
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java:11:
public class ContentShellActivity extends Activity {
Class level javadoc...let's start this project out on a good note :-)

[Satish, 2012-03-27 20:15:27]

Thanks for the quick review.

http://codereview.chromium.org/9860032/diff/1007/content/content.gyp
File content/content.gyp (right):

http://codereview.chromium.org/9860032/diff/1007/content/content.gyp#newcode14
content/content.gyp:14: 'shell/android/content_shell.gypi',
On 2012/03/27 17:16:23, Ted C wrote:
> any reason we don't just put this in content/content_shell.gypi?
> 
> Don't know about precedence, so maybe this is just how it is done.

I followed the same model that jrg used in his first CL
http://chromecodereview.appspot.com/9860032/ as that seemed clean, containing
all android related files under the /android directory

http://codereview.chromium.org/9860032/diff/1007/content/shell/android/conten...
File content/shell/android/content_shell.gypi (right):

http://codereview.chromium.org/9860032/diff/1007/content/shell/android/conten...
content/shell/android/content_shell.gypi:18: '<!@(find shell/android/java -name
"*.java")',
On 2012/03/27 17:16:23, Ted C wrote:
> Not sure if we should start including other dependencies yet, but should we
also
> include:
> base/android/java/* as well
> 
> Although, I'm not sure what our approach long term will be.  Are we going to
> include all the .java files explicitly here or are we going to you other gyp
> targets to build .jar files and we include the jar files?

Ideally base/android/java should be listed as a dependency instead of including
the java files here. But I haven't tried so don't know if that will work. In
this CL base/android/java isn't used yet and in a future CL where we start using
it I'll add it the right way.

http://codereview.chromium.org/9860032/diff/1007/content/shell/android/java/o...
File
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java
(right):

http://codereview.chromium.org/9860032/diff/1007/content/shell/android/java/o...
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java:1:
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
On 2012/03/27 17:16:23, Ted C wrote:
> I think to make this easier to match other android applications, the path
should
> be include src before the org.
> 
> The typical layout I've seen is:
> 
> AndroidManifest.xml
> src -> path/to/source
> res -> drawables/layouts/etc...needed by the application

Agree on the general dir structure. 'src' has a predefined meaning in chromium
and webkit because of the 'src' and 'include' split where .cc files go under src
and .h under include. So we seem to be going with 'java' instead (e.g.
base/android/java/org/... )

http://codereview.chromium.org/9860032/diff/1007/content/shell/android/java/o...
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java:11:
public class ContentShellActivity extends Activity {
On 2012/03/27 17:16:23, Ted C wrote:
> Class level javadoc...let's start this project out on a good note :-)

Done.

[Ted C, 2012-03-27 20:33:01]

+Grace for the apk directory/package structure question

http://codereview.chromium.org/9860032/diff/1007/content/shell/android/java/o...
File
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java
(right):

http://codereview.chromium.org/9860032/diff/1007/content/shell/android/java/o...
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java:1:
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
On 2012/03/27 20:15:28, Satish wrote:
> On 2012/03/27 17:16:23, Ted C wrote:
> > I think to make this easier to match other android applications, the path
> should
> > be include src before the org.
> > 
> > The typical layout I've seen is:
> > 
> > AndroidManifest.xml
> > src -> path/to/source
> > res -> drawables/layouts/etc...needed by the application
> 
> Agree on the general dir structure. 'src' has a predefined meaning in chromium
> and webkit because of the 'src' and 'include' split where .cc files go under
src
> and .h under include. So we seem to be going with 'java' instead (e.g.
> base/android/java/org/... )

I will argue that is not a good precedence.  All Android apps I've seen follow
the directory structure defined below, and I think we will be in a much better
shape if we do the same.

http://developer.android.com/guide/developing/projects/index.html

I think having the simple /java directory is good for things we will be making
.jars out of, but APKs should follow the android guidelines.

I'll cc Grace on this and hopefully she can comment as to whether this is
necessary.  Maybe, this isn't as big of a deal as it seems to me.

[klobag.chromium, 2012-03-28 00:43:15]

For apk directory structure, I think java is fine. Then we will have

java/org/chromium
jni/
res/drawable

http://codereview.chromium.org/9860032/diff/3002/content/shell/android/Androi...
File content/shell/android/AndroidManifest.xml (right):

http://codereview.chromium.org/9860032/diff/3002/content/shell/android/Androi...
content/shell/android/AndroidManifest.xml:17:
android:configChanges="orientation|keyboardHidden">
You may want full exclusion "orientation|keyboardHidden|keyboard|screenSize"

[Satish, 2012-03-28 08:51:40]

http://codereview.chromium.org/9860032/diff/3002/content/shell/android/Androi...
File content/shell/android/AndroidManifest.xml (right):

http://codereview.chromium.org/9860032/diff/3002/content/shell/android/Androi...
content/shell/android/AndroidManifest.xml:17:
android:configChanges="orientation|keyboardHidden">
On 2012/03/28 00:43:15, klobag.chromium wrote:
> You may want full exclusion "orientation|keyboardHidden|keyboard|screenSize"

Done.

[Satish, 2012-03-28 09:45:28]

I've also added content_shell_apk to the 'All' target in build/all_android.gyp
so it gets built with the rest of targets on the try/main bots. PTAL

[Peter Beverloo, 2012-03-28 09:46:30]

On 2012/03/28 09:45:28, Satish wrote:
> I've also added content_shell_apk to the 'All' target in build/all_android.gyp
> so it gets built with the rest of targets on the try/main bots. PTAL

LGTM, but please wait until the try turns green before toggling the CQ bit.

[Peter Beverloo, 2012-03-28 09:47:33]

On 2012/03/28 09:46:30, Peter Beverloo wrote:
> On 2012/03/28 09:45:28, Satish wrote:
> > I've also added content_shell_apk to the 'All' target in
build/all_android.gyp
> > so it gets built with the rest of targets on the try/main bots. PTAL
> 
> LGTM, but please wait until the try turns green before toggling the CQ bit.

Oh, you'll need OWNERS approval as well, of course.

[Satish, 2012-03-28 09:55:02]

+jam for content OWNERS approval

[jam, 2012-03-28 16:23:48]

http://codereview.chromium.org/9860032/diff/7002/content/shell/android/conten...
File content/shell/android/content_shell.gypi (right):

http://codereview.chromium.org/9860032/diff/7002/content/shell/android/conten...
content/shell/android/content_shell.gypi:1: # Copyright (c) 2012 The Chromium
Authors. All rights reserved.
we should avoid two gypi files with the same name (makes searching hard among
other things).

why are you adding another gypi instead of putting this stuff in
content/content_shell.gypi? all the gypi files in content are in top level

[Ted C, 2012-03-28 16:27:16]

http://codereview.chromium.org/9860032/diff/7002/content/shell/android/java/o...
File
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java
(right):

http://codereview.chromium.org/9860032/diff/7002/content/shell/android/java/o...
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java:5:
package org.chromium.content_shell;
i just noticed this doesn't follow the directory structure of the other content
shell code.  Any reason it isn't content/shell like the rest?  Underscores in
package names are pretty uncommon as well and I typically just see the two words
mashed together if need be.

[jam, 2012-03-28 16:34:35]

http://codereview.chromium.org/9860032/diff/7002/content/shell/android/java/o...
File
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java
(right):

http://codereview.chromium.org/9860032/diff/7002/content/shell/android/java/o...
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java:5:
package org.chromium.content_shell;
On 2012/03/28 16:27:16, Ted C wrote:
> i just noticed this doesn't follow the directory structure of the other
content
> shell code.  Any reason it isn't content/shell like the rest?

mac started putting code in content_shell/mac, so I think it's fine to do this
for android. we probably want to move the rest of the mac specific code to /mac,
and do the same for win/linux.

>  Underscores in
> package names are pretty uncommon as well and I typically just see the two
words
> mashed together if need be.

[Ted C, 2012-03-28 18:33:53]

On 2012/03/28 16:34:35, John Abd-El-Malek wrote:
>
http://codereview.chromium.org/9860032/diff/7002/content/shell/android/java/o...
> File
>
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java
> (right):
> 
>
http://codereview.chromium.org/9860032/diff/7002/content/shell/android/java/o...
>
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java:5:
> package org.chromium.content_shell;
> On 2012/03/28 16:27:16, Ted C wrote:
> > i just noticed this doesn't follow the directory structure of the other
> content
> > shell code.  Any reason it isn't content/shell like the rest?
> 
> mac started putting code in content_shell/mac, so I think it's fine to do this
> for android. we probably want to move the rest of the mac specific code to
/mac,
> and do the same for win/linux.

Alrighty...let's keep it consistent with the rest of chrome then.  Although
underscores are not common, they are not forbidden or entirely unused in package
names.

> 
> >  Underscores in
> > package names are pretty uncommon as well and I typically just see the two
> words
> > mashed together if need be.

[Satish, 2012-03-28 20:24:49]

New CL uploaded, PTAL.

http://codereview.chromium.org/9860032/diff/7002/content/shell/android/conten...
File content/shell/android/content_shell.gypi (right):

http://codereview.chromium.org/9860032/diff/7002/content/shell/android/conten...
content/shell/android/content_shell.gypi:1: # Copyright (c) 2012 The Chromium
Authors. All rights reserved.
On 2012/03/28 16:23:48, John Abd-El-Malek wrote:
> we should avoid two gypi files with the same name (makes searching hard among
> other things).
> 
> why are you adding another gypi instead of putting this stuff in
> content/content_shell.gypi? all the gypi files in content are in top level

Done.

[John Grabowski, 2012-03-28 21:30:54]

LGTM

http://codereview.chromium.org/9860032/diff/11005/content/shell/android/OWNERS
File content/shell/android/OWNERS (right):

http://codereview.chromium.org/9860032/diff/11005/content/shell/android/OWNER...
content/shell/android/OWNERS:1: satish@chromium.org
Add more names here

http://codereview.chromium.org/9860032/diff/11005/content/shell/android/java/...
File
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java
(right):

http://codereview.chromium.org/9860032/diff/11005/content/shell/android/java/...
content/shell/android/java/org/chromium/content_shell/ContentShellActivity.java:12:
* This is the main activity for content_shell on android, gets invoked when the
app is invoked
>80
Yes, I know java is 100, but it is trivial to accomodate here.

[jam, 2012-03-29 02:26:07]

lgtm

[commit-bot: I haz the power, 2012-03-29 08:07:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/satish@chromium.org/9860032/18001

[commit-bot: I haz the power, 2012-03-29 10:03:22]

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

[commit-bot: I haz the power, 2012-03-30 08:33:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/satish@chromium.org/9860032/18001

[commit-bot: I haz the power, 2012-03-30 11:44:08]

Change committed as 129831