[MediaNotification] Don't check HTTP status code for image fetch

[MediaNotification] Don't check HTTP status code for image fetch

ImageDownloader does not return valid HTTP status code for local
URLs. This CL removes the check for HTTP status code from
MediaImageManager, so that it only checks for the list of
downloaded to see if the image fetch succeeded or not.

This CL also adds tests for verifying ImageDownloader returns
correct results.

BUG=679694
Review-Url: https://codereview.chromium.org/2625703002
Cr-Commit-Position: refs/heads/master@{#442949}
Committed: https://chromium.googlesource.com/chromium/src/+/56eb36cf3d8aea3d182695e7d518daa1a7a46a66

[Zhiqiang Zhang (Slow), 2017-01-10 13:17:38]

Description was changed from

==========
[ImageDownloader] Return 200 as HTTP status code for data URLs

Previously ImageDownloader returns 0 as HTTP status code, which brings
trouble for the clients when checking the code for success/fail.
Instead, it should return 200, which can also be observed in the
developer tools when fetching data URLs.

BUG=679694
==========

to

==========
[ImageDownloader] Return 200 as HTTP status code for data URLs

Previously ImageDownloader returns 0 as HTTP status code, which
is not a valid status code and brings trouble for the clients
when checking the code for success/fail.

This causes MediaNotification to reset the large icon when data
image is downloaded, because it only accepts 2xx status codes.

Instead, it should return 200, which can also be observed in the
developer tools when fetching data URLs.

BUG=679694
==========

[Zhiqiang Zhang (Slow), 2017-01-10 13:25:42]

Description was changed from

==========
[ImageDownloader] Return 200 as HTTP status code for data URLs

Previously ImageDownloader returns 0 as HTTP status code, which
is not a valid status code and brings trouble for the clients
when checking the code for success/fail.

This causes MediaNotification to reset the large icon when data
image is downloaded, because it only accepts 2xx status codes.

Instead, it should return 200, which can also be observed in the
developer tools when fetching data URLs.

BUG=679694
==========

to

==========
[ImageDownloader] Return 200 as HTTP status code for data URLs

Previously ImageDownloader returns 0 as HTTP status code, which
is not a valid status code and brings trouble for the clients
when checking the code for success/fail. This causes
MediaImageManager.java to reset the large icon when data image is
downloaded, because it only accepts 2xx status codes.

Instead, it should return 200, which can also be observed in the
developer tools when fetching data URLs.

No client-side change is needed since the HTTP status code is
only used by MediaImageManager.java

BUG=679694
==========

[Zhiqiang Zhang (Slow), 2017-01-10 13:33:06]

zqzhang@chromium.org changed reviewers:
+ jochen@chromium.org

[Zhiqiang Zhang (Slow), 2017-01-10 13:33:06]

PTAL

I think this CL is more Blink-related but the code is in content

[jochen (gone - plz use gerrit), 2017-01-10 14:18:09]

can you add a test please?

[Zhiqiang Zhang (Slow), 2017-01-10 16:55:36]

Description was changed from

==========
[ImageDownloader] Return 200 as HTTP status code for data URLs

Previously ImageDownloader returns 0 as HTTP status code, which
is not a valid status code and brings trouble for the clients
when checking the code for success/fail. This causes
MediaImageManager.java to reset the large icon when data image is
downloaded, because it only accepts 2xx status codes.

Instead, it should return 200, which can also be observed in the
developer tools when fetching data URLs.

No client-side change is needed since the HTTP status code is
only used by MediaImageManager.java

BUG=679694
==========

to

==========
[MediaNotification] Don't check HTTP status code for image fetch

ImageDownloader does not return valid HTTP status code for local
URLs. This CL removes the check for HTTP status code from
MediaImageManager, so that it only checks for the list of
downloaded to see if the image fetch succeeded or not.

This CL also adds tests for verifying ImageDownloader returns
correct results.

BUG=679694
==========

[Zhiqiang Zhang (Slow), 2017-01-10 16:56:25]

zqzhang@chromium.org changed reviewers:
+ mlamouri@chromium.org

[Zhiqiang Zhang (Slow), 2017-01-10 16:56:25]

PTAL

+mlamouri for MediaImageDownloader since the fix is now on the client side.

On 2017/01/10 14:18:09, jochen wrote:
> can you add a test please?

Thanks for reminding. I wrote some tests before for ImageDownloader and totally
forgot it.

Actually I just compared with Firefox and Safari, and found that file and data
URLs do not have HTTP status code.
So it's reasonable to not modify the status code returned by ImageDownloader.
In the new patch, I fixed the issue on the client side, so that the client do
not check the status code (as other clients did).

Also, some new tests are added.

[jochen (gone - plz use gerrit), 2017-01-11 09:47:00]

test lgtm. deferring to mlamouri@ for the java change

[mlamouri (slow - plz ping), 2017-01-11 11:16:52]

lgtm

[mlamouri (slow - plz ping), 2017-01-11 11:17:00]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2017-01-11 11:17:17]

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, 2017-01-11 12:23:21]

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

[commit-bot: I haz the power, 2017-01-11 12:23:21]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Zhiqiang Zhang (Slow), 2017-01-11 14:56:41]

The CQ bit was checked by zqzhang@chromium.org

[commit-bot: I haz the power, 2017-01-11 14:57:13]

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, 2017-01-11 16:11:29]

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

[commit-bot: I haz the power, 2017-01-11 16:11:30]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Zhiqiang Zhang (Slow), 2017-01-11 16:49:40]

The CQ bit was checked by zqzhang@chromium.org

[Zhiqiang Zhang (Slow), 2017-01-11 16:49:41]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/2625703002/#ps40001
(title: "fixed junit test & cl format")

[commit-bot: I haz the power, 2017-01-11 16:49:55]

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, 2017-01-11 17:49:00]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484153380876910,
"parent_rev": "690c3cf203b94623e5adbece7d6b4afc3501cd94", "commit_rev":
"56eb36cf3d8aea3d182695e7d518daa1a7a46a66"}

[commit-bot: I haz the power, 2017-01-11 17:49:57]

Description was changed from

==========
[MediaNotification] Don't check HTTP status code for image fetch

ImageDownloader does not return valid HTTP status code for local
URLs. This CL removes the check for HTTP status code from
MediaImageManager, so that it only checks for the list of
downloaded to see if the image fetch succeeded or not.

This CL also adds tests for verifying ImageDownloader returns
correct results.

BUG=679694
==========

to

==========
[MediaNotification] Don't check HTTP status code for image fetch

ImageDownloader does not return valid HTTP status code for local
URLs. This CL removes the check for HTTP status code from
MediaImageManager, so that it only checks for the list of
downloaded to see if the image fetch succeeded or not.

This CL also adds tests for verifying ImageDownloader returns
correct results.

BUG=679694

Review-Url: https://codereview.chromium.org/2625703002
Cr-Commit-Position: refs/heads/master@{#442949}
Committed:
https://chromium.googlesource.com/chromium/src/+/56eb36cf3d8aea3d182695e7d518...
==========

[commit-bot: I haz the power, 2017-01-11 17:49:58]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/56eb36cf3d8aea3d182695e7d518...