Use Nearby to background scan for URLs

Use Nearby to background scan for URLs

This change initiates a background scan through Nearby on startup so
that the Physical Web feature can surface URLs.

BUG=623638
Committed: https://crrev.com/7ea6156fc386b92dfbc42847d77add3bffc7b0cd
Cr-Commit-Position: refs/heads/master@{#408253}

[cco3, 2016-07-26 22:39:02]

Description was changed from

==========
Use Nearby to background scan for URLs

This change initiates a background scan through Nearby on startup so
that the Physical Web feature can surface URLs.

BUG=623638
==========

to

==========
Use Nearby to background scan for URLs

This change initiates a background scan through Nearby on startup so
that the Physical Web feature can surface URLs.

BUG=623638
==========

[cco3, 2016-07-26 22:39:02]

cco3@chromium.org changed reviewers:
+ mattreynolds@chromium.org

[mattreynolds, 2016-07-27 00:04:51]

lgtm

[cco3, 2016-07-27 16:35:20]

cco3@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[gone, 2016-07-27 17:38:47]

https://codereview.chromium.org/2176373007/diff/1/chrome/android/java/Android...
File chrome/android/java/AndroidManifest.xml (right):

https://codereview.chromium.org/2176373007/diff/1/chrome/android/java/Android...
chrome/android/java/AndroidManifest.xml:535: android:exported="false"
Put /> right after the android:exported bit.  See line 552.

https://codereview.chromium.org/2176373007/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbyBackgroundSubscription.java
(right):

https://codereview.chromium.org/2176373007/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbyBackgroundSubscription.java:52:
getGoogleApiClient(), createNearbySubscribeIntent(), createSubscribeOptions());
indentation is off.  Also I'm guessing it's intentional that the SUBSCRIBING
mode unsubscribes and vice versa?  Not familiar with the API but it looks weird.

https://codereview.chromium.org/2176373007/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbyBackgroundSubscription.java:67:
void subscribe(Runnable callback) {
This isn't something you need a queue for, is it?  If an unsubscribe() comes
immediately after a subscribe(), it'll wipe out the mode and run the wrong
callback.

[cco3, 2016-07-27 18:54:59]

https://codereview.chromium.org/2176373007/diff/1/chrome/android/java/Android...
File chrome/android/java/AndroidManifest.xml (right):

https://codereview.chromium.org/2176373007/diff/1/chrome/android/java/Android...
chrome/android/java/AndroidManifest.xml:535: android:exported="false"
On 2016/07/27 17:38:47, dfalcantara wrote:
> Put /> right after the android:exported bit.  See line 552.

Done.

https://codereview.chromium.org/2176373007/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbyBackgroundSubscription.java
(right):

https://codereview.chromium.org/2176373007/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbyBackgroundSubscription.java:52:
getGoogleApiClient(), createNearbySubscribeIntent(), createSubscribeOptions());
On 2016/07/27 17:38:47, dfalcantara wrote:
> indentation is off.  Also I'm guessing it's intentional that the SUBSCRIBING
> mode unsubscribes and vice versa?  Not familiar with the API but it looks
weird.

No, good catch!  I think I know why my testing didn't pick up on this.

https://codereview.chromium.org/2176373007/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbyBackgroundSubscription.java:67:
void subscribe(Runnable callback) {
On 2016/07/27 17:38:47, dfalcantara wrote:
> This isn't something you need a queue for, is it?  If an unsubscribe() comes
> immediately after a subscribe(), it'll wipe out the mode and run the wrong
> callback.

I think it's OK (see reason below if you are curious), but I'm going to change
this to make it more clear, since we only use the object once before discarding
it anyway.

If an unsubscribe comes immediately after a subscribe, it will unsubscribe on
connection and the second callback will be run.  I'm not sure we would want to
run the callback sent with subscribe in that case since we would never have
subscribed.

[gone, 2016-07-27 20:06:23]

lgtm

https://codereview.chromium.org/2176373007/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbyBackgroundSubscription.java
(right):

https://codereview.chromium.org/2176373007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbyBackgroundSubscription.java:23:
private int mAction;
private final int mAction?

https://codereview.chromium.org/2176373007/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbySubscription.java
(right):

https://codereview.chromium.org/2176373007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbySubscription.java:30:
public static final int SUBSCRIBE = 1;
nit: public static final above private static finals

[cco3, 2016-07-27 21:14:26]

https://codereview.chromium.org/2176373007/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbyBackgroundSubscription.java
(right):

https://codereview.chromium.org/2176373007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbyBackgroundSubscription.java:23:
private int mAction;
On 2016/07/27 20:06:23, dfalcantara wrote:
> private final int mAction?

Done.

https://codereview.chromium.org/2176373007/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbySubscription.java
(right):

https://codereview.chromium.org/2176373007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbySubscription.java:30:
public static final int SUBSCRIBE = 1;
On 2016/07/27 20:06:23, dfalcantara wrote:
> nit: public static final above private static finals

I thought to do that, but I feel like I've done it before and been told that the
TAG must come first.

[gone, 2016-07-27 21:15:30]

https://codereview.chromium.org/2176373007/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbySubscription.java
(right):

https://codereview.chromium.org/2176373007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbySubscription.java:30:
public static final int SUBSCRIBE = 1;
On 2016/07/27 21:14:26, cco3 wrote:
> On 2016/07/27 20:06:23, dfalcantara wrote:
> > nit: public static final above private static finals
> 
> I thought to do that, but I feel like I've done it before and been told that
the
> TAG must come first.

Acknowledged.  Yeah, we really could use a consistent style guide to point to. 
Keep it as you like.

[cco3, 2016-07-27 21:17:41]

The CQ bit was checked by cco3@chromium.org

[cco3, 2016-07-27 21:17:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
mattreynolds@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2176373007/#ps40001
(title: "Make some variables private static final")

[commit-bot: I haz the power, 2016-07-27 21:17:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[cco3, 2016-07-27 21:18:09]

On 2016/07/27 21:15:30, dfalcantara wrote:
>
https://codereview.chromium.org/2176373007/diff/20001/chrome/android/java/src...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbySubscription.java
> (right):
> 
>
https://codereview.chromium.org/2176373007/diff/20001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbySubscription.java:30:
> public static final int SUBSCRIBE = 1;
> On 2016/07/27 21:14:26, cco3 wrote:
> > On 2016/07/27 20:06:23, dfalcantara wrote:
> > > nit: public static final above private static finals
> > 
> > I thought to do that, but I feel like I've done it before and been told that
> the
> > TAG must come first.
> 
> Acknowledged.  Yeah, we really could use a consistent style guide to point to.

> Keep it as you like.

I went ahead and changed it (I like it your way better anyway)

[commit-bot: I haz the power, 2016-07-27 22:27:12]

Description was changed from

==========
Use Nearby to background scan for URLs

This change initiates a background scan through Nearby on startup so
that the Physical Web feature can surface URLs.

BUG=623638
==========

to

==========
Use Nearby to background scan for URLs

This change initiates a background scan through Nearby on startup so
that the Physical Web feature can surface URLs.

BUG=623638
==========

[commit-bot: I haz the power, 2016-07-27 22:27:13]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-27 22:31:34]

Description was changed from

==========
Use Nearby to background scan for URLs

This change initiates a background scan through Nearby on startup so
that the Physical Web feature can surface URLs.

BUG=623638
==========

to

==========
Use Nearby to background scan for URLs

This change initiates a background scan through Nearby on startup so
that the Physical Web feature can surface URLs.

BUG=623638

Committed: https://crrev.com/7ea6156fc386b92dfbc42847d77add3bffc7b0cd
Cr-Commit-Position: refs/heads/master@{#408253}
==========

[commit-bot: I haz the power, 2016-07-27 22:31:35]

Patchset 3 (id:??) landed as
https://crrev.com/7ea6156fc386b92dfbc42847d77add3bffc7b0cd
Cr-Commit-Position: refs/heads/master@{#408253}