[GCM] Move GCMListener into chrome/

[GCM] Move GCMListener into chrome/

In preparation for pulling cacheinvalidation library into
components/invalidation, we need to remove dependencies on cacheinvalidation
from components/gcm_driver (which components/invalidation depends on).
GCMListener is the only file that depends on cacheinvalidation, so move it into
chrome/android.

BUG=472898
TBR=mnaganov@chromium.org, benm@chromium.org

Committed: https://crrev.com/fa54938f9b94a6e6e7600343a42557c62ad37547
Cr-Commit-Position: refs/heads/master@{#324663}

[Nicolas Zea, 2015-04-09 20:43:00]

zea@chromium.org changed reviewers:
+ nyquist@chromium.org, peter@chromium.org

[Nicolas Zea, 2015-04-09 20:43:01]

+Tommy for chrome/android, +Peter for components/gcm_driver/android. As best I
can tell everything is working with this approach. Let me know if you think it
might be breaking something though.

[nyquist, 2015-04-09 21:33:37]

peter: see discussion in e-mail thread.

I am OK with this approach, by the way, but I'd like to wait for your response.

[nyquist, 2015-04-09 21:34:17]

https://codereview.chromium.org/1063633004/diff/60001/components/gcm_driver/a...
File
components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java
(right):

https://codereview.chromium.org/1063633004/diff/60001/components/gcm_driver/a...
components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java:120:
public static void onMessageReceived(Context context, final String appId, final
Bundle extras) {
This feels a bit bad in fact.

[Peter Beverloo, 2015-04-10 11:52:02]

lgtm

Tommy: I'm not aware of an e-mail thread. Did you mean the bug, or am I not
looking in the right places?

https://codereview.chromium.org/1063633004/diff/60001/chrome/android/shell/ja...
File chrome/android/shell/java/AndroidManifest.xml.jinja2 (right):

https://codereview.chromium.org/1063633004/diff/60001/chrome/android/shell/ja...
chrome/android/shell/java/AndroidManifest.xml.jinja2:141:
android:name="org.chromium.chrome.browser.services.gcm.GCMListener"/>
We'll need to update this on our private manifest as well.

https://codereview.chromium.org/1063633004/diff/60001/components/gcm_driver/a...
File
components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java
(right):

https://codereview.chromium.org/1063633004/diff/60001/components/gcm_driver/a...
components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java:120:
public static void onMessageReceived(Context context, final String appId, final
Bundle extras) {
On 2015/04/09 21:34:16, nyquist wrote:
> This feels a bit bad in fact.

I'm not super concerned about this actually - what ends up calling this doesn't
really matter for the functionality of this class.

Since it's now a public API we should have a brief docblock comment, but I'd be
happy to take care of that after this patch.

[Peter Beverloo, 2015-04-10 11:53:12]

https://codereview.chromium.org/1063633004/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/services/gcm/GCMListener.java
(right):

https://codereview.chromium.org/1063633004/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/services/gcm/GCMListener.java:5:
package org.chromium.chrome.browser.services.gcm;
nit: You may want to add an OWNERS file here to make sure that we don't have to
nag chrome/ Java OWNERS for each little change.

[johnme, 2015-04-10 13:24:58]

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

[johnme, 2015-04-10 13:24:59]

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

https://codereview.chromium.org/1063633004/diff/60001/chrome/android/BUILD.gn...
chrome/android/BUILD.gn:99: "//third_party/android_tools:android_gcm_java",
Why is this now needed?

[Nicolas Zea, 2015-04-10 17:10:30]

Comments addressed. Tommy, was there anything else?

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

https://codereview.chromium.org/1063633004/diff/60001/chrome/android/BUILD.gn...
chrome/android/BUILD.gn:99: "//third_party/android_tools:android_gcm_java",
On 2015/04/10 13:24:59, johnme wrote:
> Why is this now needed?

GCMListener is overriding several methods from
com.google.android.gcm.GCMBaseIntentService (via MultiplexingGCMListener) and
also uses a couple android gcm methods (like getApplicationContext). As I
understand it Java GN doesn't allow transitive dependencies, so have to express
it here.

https://codereview.chromium.org/1063633004/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/services/gcm/GCMListener.java
(right):

https://codereview.chromium.org/1063633004/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/services/gcm/GCMListener.java:5:
package org.chromium.chrome.browser.services.gcm;
On 2015/04/10 11:53:12, Peter Beverloo wrote:
> nit: You may want to add an OWNERS file here to make sure that we don't have
to
> nag chrome/ Java OWNERS for each little change.

Done.

https://codereview.chromium.org/1063633004/diff/60001/chrome/android/shell/ja...
File chrome/android/shell/java/AndroidManifest.xml.jinja2 (right):

https://codereview.chromium.org/1063633004/diff/60001/chrome/android/shell/ja...
chrome/android/shell/java/AndroidManifest.xml.jinja2:141:
android:name="org.chromium.chrome.browser.services.gcm.GCMListener"/>
On 2015/04/10 11:52:02, Peter Beverloo wrote:
> We'll need to update this on our private manifest as well.

Acknowledged.

https://codereview.chromium.org/1063633004/diff/60001/components/gcm_driver/a...
File
components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java
(right):

https://codereview.chromium.org/1063633004/diff/60001/components/gcm_driver/a...
components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java:120:
public static void onMessageReceived(Context context, final String appId, final
Bundle extras) {
On 2015/04/10 11:52:02, Peter Beverloo wrote:
> On 2015/04/09 21:34:16, nyquist wrote:
> > This feels a bit bad in fact.
> 
> I'm not super concerned about this actually - what ends up calling this
doesn't
> really matter for the functionality of this class.
> 
> Since it's now a public API we should have a brief docblock comment, but I'd
be
> happy to take care of that after this patch.

Sounds good, thanks Peter!

[johnme, 2015-04-10 17:11:57]

lgtm, thanks :)

[nyquist, 2015-04-10 17:31:34]

lgtm

[Nicolas Zea, 2015-04-10 17:36:48]

The CQ bit was checked by zea@chromium.org

[Nicolas Zea, 2015-04-10 17:36:49]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org
Link to the patchset: https://codereview.chromium.org/1063633004/#ps80001
(title: "Add OWNERS")

[commit-bot: I haz the power, 2015-04-10 17:37:05]

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

[commit-bot: I haz the power, 2015-04-10 17:46:04]

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

[commit-bot: I haz the power, 2015-04-10 17:46:05]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Nicolas Zea, 2015-04-10 17:57:15]

zea@chromium.org changed reviewers:
+ benm@chromium.org, mnaganov@chromium.org

[Nicolas Zea, 2015-04-10 17:57:37]

+TBR mnaganov@chromium.org and benm@chromium.org for reordering of DEPS file.

[Nicolas Zea, 2015-04-10 17:57:42]

The CQ bit was checked by zea@chromium.org

[commit-bot: I haz the power, 2015-04-10 17:59:25]

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

[commit-bot: I haz the power, 2015-04-10 18:04:52]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-04-10 18:05:55]

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

[mnaganov (inactive), 2015-04-13 10:35:50]

lgtm