Use BitmapHttpRequest to fetch favicon for Nearby URLs

[physicalweb] Use BitmapHttpRequest to fetch favicon for Nearby URLs

Cache icon bitmaps in NearbyUrlsAdapter.
Fix a bug in HttpRequest that incorrectly set doOutput to true for
  requests that do not need to send additional data.

BUG=529962
Committed: https://crrev.com/82c2d901e90b6d8afaa74c43df8b2e66054a2df7
Cr-Commit-Position: refs/heads/master@{#356724}

[mattreynolds, 2015-10-28 23:18:09]

Description was changed from

==========
Use BitmapHttpRequest to fetch favicon for Nearby URLs

Cache icon bitmaps in NearbyUrlsAdapter.
Fix a bug in HttpRequest that incorrectly set doOutput to true for
  requests that do not need to send additional data.

BUG=529962
==========

to

==========
[phyiscalweb] Use BitmapHttpRequest to fetch favicon for Nearby URLs

Cache icon bitmaps in NearbyUrlsAdapter.
Fix a bug in HttpRequest that incorrectly set doOutput to true for
  requests that do not need to send additional data.

BUG=529962
==========

[mattreynolds, 2015-10-28 23:18:27]

Description was changed from

==========
[phyiscalweb] Use BitmapHttpRequest to fetch favicon for Nearby URLs

Cache icon bitmaps in NearbyUrlsAdapter.
Fix a bug in HttpRequest that incorrectly set doOutput to true for
  requests that do not need to send additional data.

BUG=529962
==========

to

==========
[physicalweb] Use BitmapHttpRequest to fetch favicon for Nearby URLs

Cache icon bitmaps in NearbyUrlsAdapter.
Fix a bug in HttpRequest that incorrectly set doOutput to true for
  requests that do not need to send additional data.

BUG=529962
==========

[mattreynolds, 2015-10-28 23:22:21]

mattreynolds@chromium.org changed reviewers:
+ cco3@chromium.org, dfalcantara@chromium.org, dvh@chromium.org

[mattreynolds, 2015-10-28 23:23:07]

Hi dfalcantara, could you please take a look at //chrome/android/java?  Thanks!

[cco3, 2015-10-28 23:31:26]

lgtm

[gone, 2015-10-28 23:41:35]

lgtm

Are you missing some files?  Your comment said to look at files under
//chrome/android/java, but every file in this CL qualifies.

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

https://codereview.chromium.org/1427663004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/NearbyUrlsAdapter.java:24:
private HashMap<String, Bitmap> mIconUrlToIconMap;
private final

[dvh, 2015-10-28 23:46:40]

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

https://codereview.chromium.org/1427663004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/ListUrlsActivity.java:104:
AsyncTask.THREAD_POOL_EXECUTOR.execute(iconRequest);
I would rather move the threading logic to PwsClient.

[mattreynolds, 2015-10-29 00:34:44]

> Are you missing some files?  Your comment said to look at files under
> //chrome/android/java, but every file in this CL qualifies.

Nope, just new and still learning the local customs.  Sorry to confuse!

> private HashMap<String, Bitmap> mIconUrlToIconMap;
> private final

Done

> AsyncTask.THREAD_POOL_EXECUTOR.execute(iconRequest);
> I would rather move the threading logic to PwsClient.

Done, please take another look

[cco3, 2015-10-29 00:38:13]

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

https://codereview.chromium.org/1427663004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/ListUrlsActivity.java:104:
AsyncTask.THREAD_POOL_EXECUTOR.execute(iconRequest);
On 2015/10/28 23:46:40, dvh wrote:
> I would rather move the threading logic to PwsClient.

Yea, this whole method could be moved to PwsClient...you could pass in a
PwsResult instead of an iconUrl if you want to make it more clearly associated
with the PwsClient.

[cco3, 2015-10-29 00:38:39]

lgtm

[dvh, 2015-10-29 00:39:43]

lgtm

[mattreynolds, 2015-10-29 00:42:23]

The CQ bit was checked by mattreynolds@chromium.org

[mattreynolds, 2015-10-29 00:42:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/1427663004/#ps20001
(title: "wip (taking changes from dfalcantara,dvh)")

[commit-bot: I haz the power, 2015-10-29 00:43:19]

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

[commit-bot: I haz the power, 2015-10-29 01:23:53]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-29 01:24:39]

Patchset 2 (id:??) landed as
https://crrev.com/82c2d901e90b6d8afaa74c43df8b2e66054a2df7
Cr-Commit-Position: refs/heads/master@{#356724}