Extend the PhysicalWeb UrlManager cache

Extend the PhysicalWeb UrlManager cache

1. Instead of resetting the cache everytime the Physical Web starts,
this change only resets the cache when we explicitly call
stopPhysicalWeb().

2. This allows the cache to grow up to 100 URLs, but kicks all that
have been in the cache longer than 24 hours.

BUG=598106, 596668
Committed: https://crrev.com/f9f0e3cb412633e68d98b485c436998703f9b9dd
Cr-Commit-Position: refs/heads/master@{#393299}

[cco3, 2016-05-10 22:13:14]

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

[mattreynolds, 2016-05-10 22:34:24]

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

https://codereview.chromium.org/1971583002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java:348:
mUrlsSortedByTimestamp.add(urlInfo.getUrl());
What if the URL is already in the PriorityQueue?  mUrlInfoMap.put will correctly
overwrite the old UrlInfo, but mUrlsSortedByTimestamp will add a duplicate item.

(also please add a test for this case)

[cco3, 2016-05-10 23:15:59]

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

https://codereview.chromium.org/1971583002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java:348:
mUrlsSortedByTimestamp.add(urlInfo.getUrl());
On 2016/05/10 22:34:24, mattreynolds wrote:
> What if the URL is already in the PriorityQueue?  mUrlInfoMap.put will
correctly
> overwrite the old UrlInfo, but mUrlsSortedByTimestamp will add a duplicate
item.
> 
> (also please add a test for this case)

Done.

[mattreynolds, 2016-05-10 23:22:57]

lgtm

[cco3, 2016-05-10 23:30:02]

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

[nyquist, 2016-05-12 16:46:45]

lgtm

https://codereview.chromium.org/1971583002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java
(right):

https://codereview.chromium.org/1971583002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java:617:
int ret = 0;
remove

https://codereview.chromium.org/1971583002/diff/40001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java
(right):

https://codereview.chromium.org/1971583002/diff/40001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java:27:
static final String URL2 = "https://google.com/";
Nit: Why are these package protected and not private?

https://codereview.chromium.org/1971583002/diff/40001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java:177:
// Make sure the URL is still in the cache.
Nit: Make sure only URL2 is still in the cache?

[cco3, 2016-05-12 17:19:27]

https://codereview.chromium.org/1971583002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java
(right):

https://codereview.chromium.org/1971583002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java:617:
int ret = 0;
On 2016/05/12 16:46:45, nyquist wrote:
> remove

Done.

https://codereview.chromium.org/1971583002/diff/40001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java
(right):

https://codereview.chromium.org/1971583002/diff/40001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java:27:
static final String URL2 = "https://google.com/";
On 2016/05/12 16:46:45, nyquist wrote:
> Nit: Why are these package protected and not private?

I recall someone telling me not to bother with that for tests.  Doesn't matter
to me.

https://codereview.chromium.org/1971583002/diff/40001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java:177:
// Make sure the URL is still in the cache.
On 2016/05/12 16:46:45, nyquist wrote:
> Nit: Make sure only URL2 is still in the cache?

Done.

[cco3, 2016-05-12 17:19:37]

The CQ bit was checked by cco3@chromium.org

[cco3, 2016-05-12 17:19:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
mattreynolds@chromium.org, nyquist@chromium.org
Link to the patchset: https://codereview.chromium.org/1971583002/#ps60001
(title: "Handle nyquist's comments")

[commit-bot: I haz the power, 2016-05-12 17:20:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1971583002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1971583002/60001

[commit-bot: I haz the power, 2016-05-12 18:02:30]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-12 18:04:41]

Description was changed from

==========
Extend the PhysicalWeb UrlManager cache

1. Instead of resetting the cache everytime the Physical Web starts,
this change only resets the cache when we explicitly call
stopPhysicalWeb().

2. This allows the cache to grow up to 100 URLs, but kicks all that
have been in the cache longer than 24 hours.

BUG=598106,596668
==========

to

==========
Extend the PhysicalWeb UrlManager cache

1. Instead of resetting the cache everytime the Physical Web starts,
this change only resets the cache when we explicitly call
stopPhysicalWeb().

2. This allows the cache to grow up to 100 URLs, but kicks all that
have been in the cache longer than 24 hours.

BUG=598106,596668

Committed: https://crrev.com/f9f0e3cb412633e68d98b485c436998703f9b9dd
Cr-Commit-Position: refs/heads/master@{#393299}
==========

[commit-bot: I haz the power, 2016-05-12 18:04:42]

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