Final step of the java_cpp_template -> java_cpp_enum migration.

Final step of the java_cpp_template -> java_cpp_enum migration.

This CL migrates the remaining gyp/gn rules away from the
java_cpp_template rule.

BUG=405532
Committed: https://crrev.com/dfdc7fb2bcde7ea4a9458967fdc5f7fa15abb3fa
Cr-Commit-Position: refs/heads/master@{#300881}

[mkosiba (inactive), 2014-10-14 18:07:27]

mkosiba@chromium.org changed reviewers:
+ cjhopman@chromium.org, mmenke@chromium.org, sky@chromium.org,
thestig@chromium.org, vrk@chromium.org, yfriedmanc@chromium.org

[mkosiba (inactive), 2014-10-14 18:07:28]

PTAL

Other than the changes under build/ this should just be a mechanical change (and
the last one of this kind), however I did end up renaming classes/fields in
these cases:
- ui::PageTransition(Type)
- base::MemoryPressureListener
- MemoryPressureLevelList
- BookmarkType
- AndroidImageFormat

mmenke:
 chrome/browser/profiles/*
 components/cronet*

yfriedman:
  chrome/android*
  chrome/browser/android*
  base/android*
  content/public/android*
  components/bookmarks*

sky:
  ui/*
  components/bookmarks.gypi OWNERS stamp

thestig:
  base/*
  chrome/*

vrk:
  media/*

cjhopman:
  build/*

[sky, 2014-10-14 19:51:28]

ui (no ui/android) and components/bookmarks.gypi LGTM. Please get a more local
owner for ui/android.

[Lei Zhang, 2014-10-14 22:52:40]

yfriedman: I would suggest making a pass over base/android/* in case I missed
anything.

https://codereview.chromium.org/659493003/diff/20001/base/base.gyp
File base/base.gyp (right):

https://codereview.chromium.org/659493003/diff/20001/base/base.gyp#newcode1362
base/base.gyp:1362: 'target_name': 'base_java_memory_pressure_level',
I'm guessing there's no way to combine this with base_java_application_state
into a single target like you are doing in GN?

https://codereview.chromium.org/659493003/diff/20001/base/memory/memory_press...
File base/memory/memory_pressure_listener.h (right):

https://codereview.chromium.org/659493003/diff/20001/base/memory/memory_press...
base/memory/memory_pressure_listener.h:62: MEMORY_PRESSURE_LEVEL_CRITICAL = 3,
In memory_pressure_level_list.h, this was 2. Is the change to 3 intentional?

https://codereview.chromium.org/659493003/diff/20001/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/659493003/diff/20001/chrome/android/BUILD.gn#...
chrome/android/BUILD.gn:102: ":chrome_android_java_enums_srcjar",
nit: alphabetical order, ditto below.

https://codereview.chromium.org/659493003/diff/20001/chrome/browser/android/e...
File chrome/browser/android/enhanced_bookmarks/enhanced_bookmarks_bridge.cc
(right):

https://codereview.chromium.org/659493003/diff/20001/chrome/browser/android/e...
chrome/browser/android/enhanced_bookmarks/enhanced_bookmarks_bridge.cc:46:
DCHECK_EQ(type, BookmarkType::BOOKMARK_TYPE_NORMAL);
If you don't mind, flip this around because it's DCHECK_EQ(expected, actual).

[Yaron, 2014-10-15 02:14:02]

yfriedman@chromium.org changed reviewers:
+ yfriedman@google.com
- yfriedmanc@chromium.org

[Yaron, 2014-10-15 02:36:47]

yfriedman@chromium.org changed reviewers:
+ yfriedman@chromium.org

[Yaron, 2014-10-15 02:36:49]

https://codereview.chromium.org/659493003/diff/20001/android_webview/java/src...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/659493003/diff/20001/android_webview/java/src...
android_webview/java/src/org/chromium/android_webview/AwContents.java:61: import
org.chromium.ui.base.PageTransition;
I bet this impacts the internal repo too. Can you prepare a CL for that?

https://codereview.chromium.org/659493003/diff/20001/base/memory/memory_press...
File base/memory/memory_pressure_listener.h (right):

https://codereview.chromium.org/659493003/diff/20001/base/memory/memory_press...
base/memory/memory_pressure_listener.h:62: MEMORY_PRESSURE_LEVEL_CRITICAL = 3,
On 2014/10/14 22:52:40, Lei Zhang wrote:
> In memory_pressure_level_list.h, this was 2. Is the change to 3 intentional?

Agreed. This seems wrong.

https://codereview.chromium.org/659493003/diff/20001/build/get_landmines.py
File build/get_landmines.py (right):

https://codereview.chromium.org/659493003/diff/20001/build/get_landmines.py#n...
build/get_landmines.py:62: print 'Delete stale generated .java files again.
crbug.com/349592'
dup? or is this file special in that adding an output line generates a new
landmine?

https://codereview.chromium.org/659493003/diff/20001/chrome/browser/android/a...
File chrome/browser/android/activity_type_id_list.h (left):

https://codereview.chromium.org/659493003/diff/20001/chrome/browser/android/a...
chrome/browser/android/activity_type_id_list.h:8: // Define Activities we are
interested in tracking.  IDs are assigned
This comment was lost

https://codereview.chromium.org/659493003/diff/20001/chrome/browser/sync/prof...
File chrome/browser/sync/profile_sync_service_android.cc (right):

https://codereview.chromium.org/659493003/diff/20001/chrome/browser/sync/prof...
chrome/browser/sync/profile_sync_service_android.cc:52: AUTOFILL = 1 << 0,
Did you check that this actually worked? Can you add a test for it? (i.e. an
expression for the enum value)

https://codereview.chromium.org/659493003/diff/20001/components/cronet/androi...
File components/cronet/android/chromium_url_request.h (right):

https://codereview.chromium.org/659493003/diff/20001/components/cronet/androi...
components/cronet/android/chromium_url_request.h:14: enum UrlRequestPriority {
Doesn't this need a directive to call it ChromiumUrlRequestPriority? 
(ChromiumUrlRequest.java depends on this type)

https://codereview.chromium.org/659493003/diff/20001/components/cronet/androi...
components/cronet/android/chromium_url_request.h:24: enum UrlRequestError {
same here

[mkosiba (inactive), 2014-10-15 09:46:08]

https://codereview.chromium.org/659493003/diff/20001/android_webview/java/src...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/659493003/diff/20001/android_webview/java/src...
android_webview/java/src/org/chromium/android_webview/AwContents.java:61: import
org.chromium.ui.base.PageTransition;
On 2014/10/15 02:36:49, Yaron wrote:
> I bet this impacts the internal repo too. Can you prepare a CL for that?

I know :) I was planning to prepare the matching patch when I have all the LGTMs

https://codereview.chromium.org/659493003/diff/20001/base/base.gyp
File base/base.gyp (right):

https://codereview.chromium.org/659493003/diff/20001/base/base.gyp#newcode1362
base/base.gyp:1362: 'target_name': 'base_java_memory_pressure_level',
On 2014/10/14 22:52:40, Lei Zhang wrote:
> I'm guessing there's no way to combine this with base_java_application_state
> into a single target like you are doing in GN?

It's possible but it would mean all the generated files would have to end up in
the same folder. Not having this separation causes problems because gyp java
rules operate on folders (and so we can end up accidentally pull in stale
files). The GN rules are a bit smarter and so this is not a problem there.

https://codereview.chromium.org/659493003/diff/20001/base/memory/memory_press...
File base/memory/memory_pressure_listener.h (right):

https://codereview.chromium.org/659493003/diff/20001/base/memory/memory_press...
base/memory/memory_pressure_listener.h:62: MEMORY_PRESSURE_LEVEL_CRITICAL = 3,
On 2014/10/15 02:36:49, Yaron wrote:
> On 2014/10/14 22:52:40, Lei Zhang wrote:
> > In memory_pressure_level_list.h, this was 2. Is the change to 3 intentional?
> 
> Agreed. This seems wrong.

I need to unmap C-X in my vi config...
Just to double-check that everything is fine I did a diff of generated files
from before and after my CL. It looks like this is the only place where
constants were changed.

https://codereview.chromium.org/659493003/diff/20001/build/get_landmines.py
File build/get_landmines.py (right):

https://codereview.chromium.org/659493003/diff/20001/build/get_landmines.py#n...
build/get_landmines.py:62: print 'Delete stale generated .java files again.
crbug.com/349592'
On 2014/10/15 02:36:49, Yaron wrote:
> dup? or is this file special in that adding an output line generates a new
> landmine?

Intentional and seems to work. I changed the line so that it's obvious that it's
intentional.

https://codereview.chromium.org/659493003/diff/20001/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/659493003/diff/20001/chrome/android/BUILD.gn#...
chrome/android/BUILD.gn:102: ":chrome_android_java_enums_srcjar",
On 2014/10/14 22:52:40, Lei Zhang wrote:
> nit: alphabetical order, ditto below.

Done.

[mmenke, 2014-10-15 13:33:12]

I'm only an owner of the portion of profiles/ that relates to ProfileIOData, so
you'll need another profile owner to review that part of the CL.

https://codereview.chromium.org/659493003/diff/40001/components/cronet.gypi
File components/cronet.gypi (right):

https://codereview.chromium.org/659493003/diff/40001/components/cronet.gypi#n...
components/cronet.gypi:208: '**/ChromiumUrlRequestPriority.java',
You need to replace **/ChromiumUrlRequestPriority.java and
**/ChromiumUrlRequestError.java with the new file(s) your script creates
instead.

https://codereview.chromium.org/659493003/diff/40001/components/cronet/androi...
File components/cronet/android/chromium_url_request.h (right):

https://codereview.chromium.org/659493003/diff/40001/components/cronet/androi...
components/cronet/android/chromium_url_request.h:24: enum UrlRequestError {
You presumably need to update the java files that actually used these as well.

See ChromiumUrlRequestError and ChromiumUrlRequestPriority in
components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequest.java

https://codereview.chromium.org/659493003/diff/40001/components/cronet/androi...
components/cronet/android/chromium_url_request.h:25: REQUEST_ERROR_SUCCESS = 1,
BUG:  Success should be 0.

[Yaron, 2014-10-15 18:31:58]

https://codereview.chromium.org/659493003/diff/20001/chrome/browser/android/a...
File chrome/browser/android/activity_type_id_list.h (left):

https://codereview.chromium.org/659493003/diff/20001/chrome/browser/android/a...
chrome/browser/android/activity_type_id_list.h:8: // Define Activities we are
interested in tracking.  IDs are assigned
On 2014/10/15 02:36:49, Yaron wrote:
> This comment was lost

bump

https://codereview.chromium.org/659493003/diff/20001/components/cronet/androi...
File components/cronet/android/chromium_url_request.h (right):

https://codereview.chromium.org/659493003/diff/20001/components/cronet/androi...
components/cronet/android/chromium_url_request.h:14: enum UrlRequestPriority {
On 2014/10/15 02:36:49, Yaron wrote:
> Doesn't this need a directive to call it ChromiumUrlRequestPriority? 
> (ChromiumUrlRequest.java depends on this type)

bump

[Lei Zhang, 2014-10-15 18:40:16]

base and chrome lgtm

[mkosiba (inactive), 2014-10-16 12:45:05]

mkosiba@chromium.org changed reviewers:
+ newt@chromium.org, noms@chromium.org

[mkosiba (inactive), 2014-10-16 12:45:07]

On 2014/10/14 19:51:28, sky wrote:
> ui (no ui/android) and components/bookmarks.gypi LGTM. Please get a more local
> owner for ui/android.

thanks sky!
+newt@ for ui/android

On 2014/10/15 13:33:12, mmenke wrote:
> I'm only an owner of the portion of profiles/ that relates to ProfileIOData,
so
> you'll need another profile owner to review that part of the CL.

Thanks!
+noms@chromium.org for chrome/browser/profiles/*

https://codereview.chromium.org/659493003/diff/20001/chrome/browser/android/a...
File chrome/browser/android/activity_type_id_list.h (left):

https://codereview.chromium.org/659493003/diff/20001/chrome/browser/android/a...
chrome/browser/android/activity_type_id_list.h:8: // Define Activities we are
interested in tracking.  IDs are assigned
On 2014/10/15 18:31:57, Yaron wrote:
> On 2014/10/15 02:36:49, Yaron wrote:
> > This comment was lost
> 
> bump

Not sure how I missed these comments, thanks for bumping them!

https://codereview.chromium.org/659493003/diff/20001/chrome/browser/android/e...
File chrome/browser/android/enhanced_bookmarks/enhanced_bookmarks_bridge.cc
(right):

https://codereview.chromium.org/659493003/diff/20001/chrome/browser/android/e...
chrome/browser/android/enhanced_bookmarks/enhanced_bookmarks_bridge.cc:46:
DCHECK_EQ(type, BookmarkType::BOOKMARK_TYPE_NORMAL);
On 2014/10/14 22:52:40, Lei Zhang wrote:
> If you don't mind, flip this around because it's DCHECK_EQ(expected, actual).

Done.

https://codereview.chromium.org/659493003/diff/20001/chrome/browser/sync/prof...
File chrome/browser/sync/profile_sync_service_android.cc (right):

https://codereview.chromium.org/659493003/diff/20001/chrome/browser/sync/prof...
chrome/browser/sync/profile_sync_service_android.cc:52: AUTOFILL = 1 << 0,
On 2014/10/15 02:36:49, Yaron wrote:
> Did you check that this actually worked? Can you add a test for it? (i.e. an
> expression for the enum value)

The generated files don't differ (the template mechanism also resulted in Java
constants of the format:

  public static final int DEVICE_INFO = 1<<11;

I added the test.

https://codereview.chromium.org/659493003/diff/20001/components/cronet/androi...
File components/cronet/android/chromium_url_request.h (right):

https://codereview.chromium.org/659493003/diff/20001/components/cronet/androi...
components/cronet/android/chromium_url_request.h:14: enum UrlRequestPriority {
On 2014/10/15 18:31:57, Yaron wrote:
> On 2014/10/15 02:36:49, Yaron wrote:
> > Doesn't this need a directive to call it ChromiumUrlRequestPriority? 
> > (ChromiumUrlRequest.java depends on this type)
> 
> bump

yup. Completely missed this as cronet does not build with my build config by
default (and it doesn't look like it builds on any of the trybots either)?. I
added the missing directives.

https://codereview.chromium.org/659493003/diff/40001/components/cronet.gypi
File components/cronet.gypi (right):

https://codereview.chromium.org/659493003/diff/40001/components/cronet.gypi#n...
components/cronet.gypi:208: '**/ChromiumUrlRequestPriority.java',
On 2014/10/15 13:33:11, mmenke wrote:
> You need to replace **/ChromiumUrlRequestPriority.java and
> **/ChromiumUrlRequestError.java with the new file(s) your script creates
> instead.

the script generates the files with the same names. I verified by compiling
cronet_package.

https://codereview.chromium.org/659493003/diff/40001/components/cronet/androi...
File components/cronet/android/chromium_url_request.h (right):

https://codereview.chromium.org/659493003/diff/40001/components/cronet/androi...
components/cronet/android/chromium_url_request.h:24: enum UrlRequestError {
On 2014/10/15 13:33:11, mmenke wrote:
> You presumably need to update the java files that actually used these as well.
> 
> See ChromiumUrlRequestError and ChromiumUrlRequestPriority in
> components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequest.java

I'll just instruct the generator to generate the enum with a different name.

https://codereview.chromium.org/659493003/diff/40001/components/cronet/androi...
components/cronet/android/chromium_url_request.h:25: REQUEST_ERROR_SUCCESS = 1,
On 2014/10/15 13:33:12, mmenke wrote:
> BUG:  Success should be 0.

Fixed.  Didn't realize I wasn't compiling cronet, so my 'diff generated files'
verification strategy didn't work here :/

[mmenke, 2014-10-16 15:30:35]

mmenke@chromium.org changed reviewers:
+ mef@chromium.org

[mmenke, 2014-10-16 15:30:35]

[+mef] Handing off the cronet part of the review to mef.

[Yaron, 2014-10-16 15:57:21]

On 2014/10/16 15:30:35, mmenke wrote:
> [+mef] Handing off the cronet part of the review to mef.

lgtm

[noms (inactive), 2014-10-16 16:40:00]

profiles lgtm

[newt (away), 2014-10-16 16:45:03]

ui/android lgtm

[cjhopman, 2014-10-16 16:54:51]

build/ lgtm

https://codereview.chromium.org/659493003/diff/20001/build/get_landmines.py
File build/get_landmines.py (right):

https://codereview.chromium.org/659493003/diff/20001/build/get_landmines.py#n...
build/get_landmines.py:62: print 'Delete stale generated .java files again.
crbug.com/349592'
On 2014/10/15 09:46:08, mkosiba wrote:
> On 2014/10/15 02:36:49, Yaron wrote:
> > dup? or is this file special in that adding an output line generates a new
> > landmine?
> 
> Intentional and seems to work. I changed the line so that it's obvious that
it's
> intentional.

For the record, a landmine is triggered whenever the output of running this
script changes. Having a single print statement for each condition is sufficient
if you just change what is printed.

https://codereview.chromium.org/659493003/diff/60001/build/android/gyp/java_c...
File build/android/gyp/java_cpp_enum.py (right):

https://codereview.chromium.org/659493003/diff/60001/build/android/gyp/java_c...
build/android/gyp/java_cpp_enum.py:138: current_definition.prefix_to_strip =
self._GetCurrentEnumPrefixToStrip()
We should probably have this check that you aren't using an unrecognized
directive.

[mef, 2014-10-16 17:42:45]

components/cronet lgtm.

Cronet builds and tests pass.

[mkosiba (inactive), 2014-10-20 15:09:06]

vrk - ping?

https://codereview.chromium.org/659493003/diff/60001/build/android/gyp/java_c...
File build/android/gyp/java_cpp_enum.py (right):

https://codereview.chromium.org/659493003/diff/60001/build/android/gyp/java_c...
build/android/gyp/java_cpp_enum.py:138: current_definition.prefix_to_strip =
self._GetCurrentEnumPrefixToStrip()
On 2014/10/16 16:54:51, cjhopman wrote:
> We should probably have this check that you aren't using an unrecognized
> directive.

Done.

[mkosiba (inactive), 2014-10-21 09:51:09]

mkosiba@chromium.org changed reviewers:
+ ddorwin@chromium.org

[mkosiba (inactive), 2014-10-21 09:51:10]

+ddorwin for media/

are all of the media owners out on vacation or something?

[mcasas, 2014-10-21 10:56:03]

mcasas@chromium.org changed reviewers:
+ mcasas@chromium.org

[mcasas, 2014-10-21 10:56:04]

media/base/android/java/src/org/chromium/media/* 
and
media/video/capture/android/*
LGTM 

(I should be an OWNER, but a recent one, and does
not include media.gyp, ping dalecurtis@ for that)

[ddorwin, 2014-10-21 16:08:18]

media/ LGTM

[mkosiba (inactive), 2014-10-21 17:21:39]

On 2014/10/21 16:08:18, ddorwin wrote:
> media/ LGTM

Thanks!

[mkosiba (inactive), 2014-10-23 09:10:04]

mkosiba@chromium.org changed reviewers:
+ jochen@chromium.org

[mkosiba (inactive), 2014-10-23 09:12:53]

+jochen TBR for content/renderer

[mkosiba (inactive), 2014-10-23 09:13:28]

The CQ bit was checked by mkosiba@chromium.org

[commit-bot: I haz the power, 2014-10-23 09:14:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/659493003/140001

[jochen (gone - plz use gerrit), 2014-10-23 09:39:31]

lgtm

[commit-bot: I haz the power, 2014-10-23 09:47:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-23 09:47:41]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_32 on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mkosiba (inactive), 2014-10-23 12:00:50]

The CQ bit was checked by mkosiba@chromium.org

[mkosiba (inactive), 2014-10-23 12:00:59]

The CQ bit was unchecked by mkosiba@chromium.org

[mkosiba (inactive), 2014-10-23 12:01:17]

The CQ bit was checked by mkosiba@chromium.org

[commit-bot: I haz the power, 2014-10-23 12:02:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/659493003/140001

[commit-bot: I haz the power, 2014-10-23 13:56:22]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2014-10-23 13:57:02]

Patchset 8 (id:??) landed as
https://crrev.com/dfdc7fb2bcde7ea4a9458967fdc5f7fa15abb3fa
Cr-Commit-Position: refs/heads/master@{#300881}