[LogoTracker] Correct fingerprint append in async parameter.

[LogoTracker] Correct fingerprint append in async parameter.

Clients that already set the async parameter (in this case, with 'cta:1'), will
end up with query strings with two differeny async parameters, instead of a
comma delimited async param.

E.g. converts ?async=cta:1&async=es_dfp:123 -> ?async=cta:1,es_dfp:123

BUG=414528
Committed: https://crrev.com/c438c1843041746877c9dc83d9ce63a3c43bb6bb
Cr-Commit-Position: refs/heads/master@{#324174}

R=newt@chromium.org

Committed: https://chromium.googlesource.com/chromium/src/+/89f3945a15ebf0a87cc4b009801fe57d6238b27e

[justincohen, 2015-04-07 15:34:49]

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

[justincohen, 2015-04-07 15:34:50]

Thoughts?

[newt (away), 2015-04-07 18:01:16]

I'd prefer to avoid such URL parsing if possible.

How about changing append_fingerprint_func_ to append_queryparams_func_, and
always calling it in LogoTracker::FetchLogo(), not just when a fingerprint is
available?

[justincohen, 2015-04-08 01:00:08]

How about this?

[newt (away), 2015-04-08 01:08:37]

lgtm, thanks!

just some comments about comments

https://codereview.chromium.org/1066923002/diff/40001/components/search_provi...
File components/search_provider_logos/logo_tracker.h (right):

https://codereview.chromium.org/1066923002/diff/40001/components/search_provi...
components/search_provider_logos/logo_tracker.h:125: const
AppendQueryparamsToLogoURL& append_fingerprint_func,
update param name and comment above

https://codereview.chromium.org/1066923002/diff/40001/components/search_provi...
components/search_provider_logos/logo_tracker.h:187: // The function used to
include the cached logo's fingerprint in the logo URL.
update comment

[justincohen, 2015-04-08 01:36:27]

https://codereview.chromium.org/1066923002/diff/40001/components/search_provi...
File components/search_provider_logos/logo_tracker.h (right):

https://codereview.chromium.org/1066923002/diff/40001/components/search_provi...
components/search_provider_logos/logo_tracker.h:125: const
AppendQueryparamsToLogoURL& append_fingerprint_func,
On 2015/04/08 01:08:37, newt wrote:
> update param name and comment above

Done.

https://codereview.chromium.org/1066923002/diff/40001/components/search_provi...
components/search_provider_logos/logo_tracker.h:187: // The function used to
include the cached logo's fingerprint in the logo URL.
On 2015/04/08 01:08:37, newt wrote:
> update comment

Done.

[justincohen, 2015-04-08 01:55:03]

The CQ bit was checked by justincohen@chromium.org

[justincohen, 2015-04-08 01:55:03]

The patchset sent to the CQ was uploaded after l-g-t-m from newt@chromium.org
Link to the patchset: https://codereview.chromium.org/1066923002/#ps60001
(title: "Fix tests and comment nits")

[commit-bot: I haz the power, 2015-04-08 01:55:16]

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

[commit-bot: I haz the power, 2015-04-08 02:14:38]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-08 02:15:41]

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

[newt (away), 2015-04-08 19:05:51]

Committed patchset #4 (id:60001) manually as
89f3945a15ebf0a87cc4b009801fe57d6238b27e.