Fix unexpected error message when quickly select different wallpapers

Fix unexpected error message when quickly select different wallpapers


When quickly select different wallpapers from wallpaper picker, sometimes
an error message shows up that saying "Chrome cannot download this wallpaper".
The error message is not expected. This CL should fix the problem.

BUG=242581
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=222053

[bshe, 2013-09-06 15:25:22]

Hey Rob.

Would you mind to take a look at this wallpaper related CL?

Thanks!

Biao

[flackr, 2013-09-06 15:44:00]

https://codereview.chromium.org/24040002/diff/1/chrome/browser/resources/chro...
File chrome/browser/resources/chromeos/wallpaper_manager/js/util.js (right):

https://codereview.chromium.org/24040002/diff/1/chrome/browser/resources/chro...
chrome/browser/resources/chromeos/wallpaper_manager/js/util.js:64: } else if
(this.status == 0) {
A status of 0 can also result from a network error it seems:
http://www.w3.org/TR/XMLHttpRequest/#the-status-attribute

I agree if the request is aborted that we shouldn't trigger an error because it
was an intentional cancellation but we should be careful not to silently fail in
other cases.

https://codereview.chromium.org/24040002/diff/1/chrome/browser/resources/chro...
File chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js
(right):

https://codereview.chromium.org/24040002/diff/1/chrome/browser/resources/chro...
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js:510:
if (image) {
onSuccess can actually be called if there's no response? Can you instead handle
this in fetchURL and avoid calling onSuccess if there's no response? Also,
shouldn't no response trigger the error handler?

[bshe, 2013-09-06 16:40:13]

On 2013/09/06 15:44:00, flackr wrote:
>
https://codereview.chromium.org/24040002/diff/1/chrome/browser/resources/chro...
> File chrome/browser/resources/chromeos/wallpaper_manager/js/util.js (right):
> 
>
https://codereview.chromium.org/24040002/diff/1/chrome/browser/resources/chro...
> chrome/browser/resources/chromeos/wallpaper_manager/js/util.js:64: } else if
> (this.status == 0) {
> A status of 0 can also result from a network error it seems:
> http://www.w3.org/TR/XMLHttpRequest/#the-status-attribute
> 
> I agree if the request is aborted that we shouldn't trigger an error because
it
> was an intentional cancellation but we should be careful not to silently fail
in
> other cases.
> 
>
https://codereview.chromium.org/24040002/diff/1/chrome/browser/resources/chro...
> File
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js
> (right):
> 
>
https://codereview.chromium.org/24040002/diff/1/chrome/browser/resources/chro...
>
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js:510:
> if (image) {
> onSuccess can actually be called if there's no response? Can you instead
handle
> this in fetchURL and avoid calling onSuccess if there's no response? Also,
> shouldn't no response trigger the error handler?

Talked offline. It seems use 'load' and 'error' message can avoid adding all the
checks. Would you mind to take another look?

Thanks!

[flackr, 2013-09-09 15:19:36]

lgtm

[commit-bot: I haz the power, 2013-09-09 15:31:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bshe@chromium.org/24040002/6001

[commit-bot: I haz the power, 2013-09-09 17:43:13]

Change committed as 222053