Correctly record "No logo today" instead of "Parsing error"

Correctly record "No logo today" instead of "Parsing error"

When there is no logo, the server sends out a json with an empty update
field. This is not the same as what we conjectured when the UMA for logo
was recorded. This CL fixes it.

BUG=543308
Committed: https://crrev.com/efa847a54ec8dbb71ab7d630a05fb289c2991a78
Cr-Commit-Position: refs/heads/master@{#371328}

[Ian Wen, 2016-01-21 23:56:24]

ianwen@chromium.org changed reviewers:
+ newt@chromium.org

[Ian Wen, 2016-01-21 23:56:24]

PTAL

[newt (away), 2016-01-22 03:11:21]

lgtm after nits

Thanks!

p.s. you might want to update your user nickname now

https://codereview.chromium.org/1618043002/diff/1/components/search_provider_...
File components/search_provider_logos/google_logo_api.cc (right):

https://codereview.chromium.org/1618043002/diff/1/components/search_provider_...
components/search_provider_logos/google_logo_api.cc:75: *parsing_failed = true;
I'd move this line to very beginning of the function now. (or at least, after
the initial implementation comment)

https://codereview.chromium.org/1618043002/diff/1/components/search_provider_...
components/search_provider_logos/google_logo_api.cc:88: // If there is no logo
today, the server sends out an empty update.
Slightly less ambiguous, I think: "If there is no logo today, the "update"
dictionary will be empty."

[Ian Wen, 2016-01-25 21:14:12]

On 2016/01/22 03:11:21, newt wrote:
> lgtm after nits
> 
> Thanks!
> 
> p.s. you might want to update your user nickname now
> 
>
https://codereview.chromium.org/1618043002/diff/1/components/search_provider_...
> File components/search_provider_logos/google_logo_api.cc (right):
> 
>
https://codereview.chromium.org/1618043002/diff/1/components/search_provider_...
> components/search_provider_logos/google_logo_api.cc:75: *parsing_failed =
true;
> I'd move this line to very beginning of the function now. (or at least, after
> the initial implementation comment)
> 
>
https://codereview.chromium.org/1618043002/diff/1/components/search_provider_...
> components/search_provider_logos/google_logo_api.cc:88: // If there is no logo
> today, the server sends out an empty update.
> Slightly less ambiguous, I think: "If there is no logo today, the "update"
> dictionary will be empty."

Done.

[Ian Wen, 2016-01-25 21:14:58]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2016-01-25 21:14:58]

The patchset sent to the CQ was uploaded after l-g-t-m from newt@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1618043002/#ps20001
(title: "respond to comments")

[commit-bot: I haz the power, 2016-01-25 21:15:27]

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

[commit-bot: I haz the power, 2016-01-25 22:07:39]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-25 22:10:09]

Description was changed from

==========
Correctly record "No logo today" instead of "Parsing error"

When there is no logo, the server sends out a json with an empty update
field. This is not the same as what we conjectured when the UMA for logo
was recorded. This CL fixes it.

BUG=543308
==========

to

==========
Correctly record "No logo today" instead of "Parsing error"

When there is no logo, the server sends out a json with an empty update
field. This is not the same as what we conjectured when the UMA for logo
was recorded. This CL fixes it.

BUG=543308

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

[commit-bot: I haz the power, 2016-01-25 22:10:10]

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