Scan through Nearby when listing PW URLs

Scan through Nearby when listing PW URLs

When we list Physical Web URLs, we need to scan through Nearby in order
to ensure the list of URLs is up to date.

BUG=623638
Committed: https://crrev.com/6fb7063faf68a39d3af22abd9274220f4998cc9b
Cr-Commit-Position: refs/heads/master@{#407910}

[cco3, 2016-07-21 22:56:29]

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

[mattreynolds, 2016-07-22 00:48:42]

lgtm after addressing comments

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

https://codereview.chromium.org/2171053002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebBleClient.java:35:
};
remove semicolon

https://codereview.chromium.org/2171053002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebBleClient.java:118:
* @param The builder to be modified.
nit: @param builder The builder... (here and below)

https://codereview.chromium.org/2171053002/diff/1/chrome/android/java_sources...
File chrome/android/java_sources.gni (right):

https://codereview.chromium.org/2171053002/diff/1/chrome/android/java_sources...
chrome/android/java_sources.gni:633:
"java/src/org/chromium/chrome/browser/physicalweb/NearbySubscription.java",
Insert these above NearbyUrlsAdapter to maintain ASCIIbetical order

[cco3, 2016-07-22 17:37:19]

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

[cco3, 2016-07-22 17:37:19]

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

https://codereview.chromium.org/2171053002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebBleClient.java:35:
};
On 2016/07/22 00:48:42, mattreynolds wrote:
> remove semicolon

Done.

https://codereview.chromium.org/2171053002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebBleClient.java:118:
* @param The builder to be modified.
On 2016/07/22 00:48:41, mattreynolds wrote:
> nit: @param builder The builder... (here and below)

Done.

https://codereview.chromium.org/2171053002/diff/1/chrome/android/java_sources...
File chrome/android/java_sources.gni (right):

https://codereview.chromium.org/2171053002/diff/1/chrome/android/java_sources...
chrome/android/java_sources.gni:633:
"java/src/org/chromium/chrome/browser/physicalweb/NearbySubscription.java",
On 2016/07/22 00:48:42, mattreynolds wrote:
> Insert these above NearbyUrlsAdapter to maintain ASCIIbetical order

Done.

[gone, 2016-07-22 23:06:29]

https://chromiumcodereview.appspot.com/2171053002/diff/40001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbyForegroundSubscription.java
(right):

https://chromiumcodereview.appspot.com/2171053002/diff/40001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbyForegroundSubscription.java:55:
void unsubscribe() {
Since connecting seems to be asynchronous, can unsubscribe() be called before
onConnected()'s ResultCallback is called?  If it did, it seems you'd be in a bad
state because mSubscribed would be false and this would disconnect, then the
callback would fire and mark mSubscribed = true.

[cco3, 2016-07-25 18:20:55]

https://chromiumcodereview.appspot.com/2171053002/diff/40001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbyForegroundSubscription.java
(right):

https://chromiumcodereview.appspot.com/2171053002/diff/40001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbyForegroundSubscription.java:55:
void unsubscribe() {
On 2016/07/22 23:06:29, dfalcantara wrote:
> Since connecting seems to be asynchronous, can unsubscribe() be called before
> onConnected()'s ResultCallback is called?  If it did, it seems you'd be in a
bad
> state because mSubscribed would be false and this would disconnect, then the
> callback would fire and mark mSubscribed = true.

Right, I was kind of sloppy about the race condition because the disconnect will
trigger an unsubscribe regardless of the situation (the actual unsubscribe is
kind of superfluous given the way we are using it).  I'll tighten it down.

[gone, 2016-07-26 00:40:58]

lgtm, I think.

[cco3, 2016-07-26 16:33:56]

Given your hesitancy, I'll have Matt take another look.

Matt, could you take a look at the way NearbyForegroundSubscription is being
used at this point? (Do you see potential race condition issues?) Thanks.

[mattreynolds, 2016-07-26 19:42:25]

On 2016/07/26 16:33:56, cco3 wrote:
> Given your hesitancy, I'll have Matt take another look.
> 
> Matt, could you take a look at the way NearbyForegroundSubscription is being
> used at this point? (Do you see potential race condition issues?) Thanks.

lgtm

[cco3, 2016-07-26 19:46:12]

The CQ bit was checked by cco3@chromium.org

[cco3, 2016-07-26 19:46:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org, mattreynolds@chromium.org
Link to the patchset: https://codereview.chromium.org/2171053002/#ps80001
(title: "Update a comment")

[commit-bot: I haz the power, 2016-07-26 19:46:50]

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

[commit-bot: I haz the power, 2016-07-26 20:55:49]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-07-26 20:57:46]

Description was changed from

==========
Scan through Nearby when listing PW URLs

When we list Physical Web URLs, we need to scan through Nearby in order
to ensure the list of URLs is up to date.

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

to

==========
Scan through Nearby when listing PW URLs

When we list Physical Web URLs, we need to scan through Nearby in order
to ensure the list of URLs is up to date.

BUG=623638

Committed: https://crrev.com/6fb7063faf68a39d3af22abd9274220f4998cc9b
Cr-Commit-Position: refs/heads/master@{#407910}
==========

[commit-bot: I haz the power, 2016-07-26 20:57:47]

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