New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 690467 link

Starred by 5 users
Status: Fixed
Owner:
treib@chromium.org
Closed: Oct 2017
Cc:
jshneier@google.com
treib@chromium.org
█ tabanao@google.com
justincohen@chromium.org
linds...@chromium.org
fhorschig@chromium.org
ios-bugs@chromium.org
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android , iOS
Pri: 2
Type: Bug

Blocked on:
issue 702601
issue 703227
issue 719513
Blocking:
issue 583290
issue 625806
issue 693082
issue 708955
issue 738073
issue 773719



Sign in to add a comment

Tracking: New Doodle endpoint

Project Member Reported by treib@chromium.org, Feb 9 2017 
Currently, Chrome on Android and iOS fetches Doodles from www.google.com/async/newtab_mobile, a special endpoint only used by the Chrome NTP. Instead, we should migrate to the standard Doodle endpoint www.google.com/async/ddljson.

Design doc: go/ntp-doodles

Comment 1 by jshneier@google.com, Feb 9 2017

Cc: tabanao@google.com

Comment 2 by linds...@chromium.org, Feb 13 2017

Cc: linds...@chromium.org

Comment 3 by treib@chromium.org, Feb 14 2017

Blocking: 625806

Comment 5 by treib@chromium.org, Feb 16 2017

Blocking: 693082
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 22 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5dae4677eefa9631e709b9d82b62417b01a98dc0

commit 5dae4677eefa9631e709b9d82b62417b01a98dc0
Author: treib <treib@chromium.org>
Date: Wed Feb 22 17:16:24 2017

[Doodle] Pull a doodle_types.h out of doodle_fetcher.h

This will ease further changes, such as separating a DoodleFetcher
interface from the Impl, and adding a DoodleService.

BUG= 690467 

Review-Url: https://codereview.chromium.org/2710673006
Cr-Commit-Position: refs/heads/master@{#452103}

[modify] https://crrev.com/5dae4677eefa9631e709b9d82b62417b01a98dc0/components/doodle/BUILD.gn
[modify] https://crrev.com/5dae4677eefa9631e709b9d82b62417b01a98dc0/components/doodle/doodle_fetcher.h
[add] https://crrev.com/5dae4677eefa9631e709b9d82b62417b01a98dc0/components/doodle/doodle_types.h

Comment 10 by fhorschig@chromium.org, Feb 28 2017

Labels: zine-tracking
Project Member

Comment 11 by bugdroid1@chromium.org, Mar 2 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d83555d59cc6dba31282c95cd402ebbcdc42d016

commit d83555d59cc6dba31282c95cd402ebbcdc42d016
Author: treib <treib@chromium.org>
Date: Thu Mar 02 13:59:10 2017

[Doodle] Replace the expiry_date in DoodleConfig by time_to_live

The service will require both Time (for persisting) and TimeTicks (for
scheduling), so better to just pass the TimeDelta.
It also makes the fetcher nicer (no need for injectable clock).

In a later follow-up, I'll move the ttl out of DoodleConfig (and
instead pass it as a separate param to DoodleFetcher::FinishedCallback).

BUG= 690467 

Review-Url: https://codereview.chromium.org/2726883002
Cr-Commit-Position: refs/heads/master@{#454254}

[modify] https://crrev.com/d83555d59cc6dba31282c95cd402ebbcdc42d016/components/doodle/doodle_fetcher_impl.cc
[modify] https://crrev.com/d83555d59cc6dba31282c95cd402ebbcdc42d016/components/doodle/doodle_fetcher_impl.h
[modify] https://crrev.com/d83555d59cc6dba31282c95cd402ebbcdc42d016/components/doodle/doodle_fetcher_impl_unittest.cc
[modify] https://crrev.com/d83555d59cc6dba31282c95cd402ebbcdc42d016/components/doodle/doodle_types.cc
[modify] https://crrev.com/d83555d59cc6dba31282c95cd402ebbcdc42d016/components/doodle/doodle_types.h
Project Member

Comment 12 by bugdroid1@chromium.org, Mar 3 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ebcd266f8e04c1ebebf3e0eca08f6683641cf613

commit ebcd266f8e04c1ebebf3e0eca08f6683641cf613
Author: treib <treib@chromium.org>
Date: Fri Mar 03 10:16:22 2017

[Doodle] Pull time_to_live out of DoodleConfig

It didn't belong there, since by definition it can't be kept around.
Also clients aren't interested in it, since the DoodleService will
handle expiration.

As a positive side effect, this allows us to implement
DoodleConfig::operator==, which makes some things much nicer.

This is a follow-up to https://codereview.chromium.org/2726883002/

BUG= 690467 

Review-Url: https://codereview.chromium.org/2725143003
Cr-Commit-Position: refs/heads/master@{#454559}

[modify] https://crrev.com/ebcd266f8e04c1ebebf3e0eca08f6683641cf613/components/doodle/doodle_fetcher.h
[modify] https://crrev.com/ebcd266f8e04c1ebebf3e0eca08f6683641cf613/components/doodle/doodle_fetcher_impl.cc
[modify] https://crrev.com/ebcd266f8e04c1ebebf3e0eca08f6683641cf613/components/doodle/doodle_fetcher_impl.h
[modify] https://crrev.com/ebcd266f8e04c1ebebf3e0eca08f6683641cf613/components/doodle/doodle_fetcher_impl_unittest.cc
[modify] https://crrev.com/ebcd266f8e04c1ebebf3e0eca08f6683641cf613/components/doodle/doodle_service.cc
[modify] https://crrev.com/ebcd266f8e04c1ebebf3e0eca08f6683641cf613/components/doodle/doodle_service.h
[modify] https://crrev.com/ebcd266f8e04c1ebebf3e0eca08f6683641cf613/components/doodle/doodle_service_unittest.cc
[modify] https://crrev.com/ebcd266f8e04c1ebebf3e0eca08f6683641cf613/components/doodle/doodle_types.cc
[modify] https://crrev.com/ebcd266f8e04c1ebebf3e0eca08f6683641cf613/components/doodle/doodle_types.h
Project Member

Comment 18 by bugdroid1@chromium.org, Mar 15 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f235d928f6d89dd7decb34082353a0bde50c6597

commit f235d928f6d89dd7decb34082353a0bde50c6597
Author: treib <treib@chromium.org>
Date: Wed Mar 15 15:01:51 2017

components/image_fetcher: Add a from_http_cache flag to RequestMetadata

To be used by clients (components/doodle in particular) for metrics purposes.

This also includes a bunch of semi-related cleanups to RequestMetadata:
- Give it a ctor (because I dislike uninitialized values).
- Move RESPONSE_CODE_INVALID in there (from ImageDataFetcher).
- Rename response_code to http_response_code.

BUG= 690467 

Review-Url: https://codereview.chromium.org/2749283002
Cr-Commit-Position: refs/heads/master@{#457080}

[modify] https://crrev.com/f235d928f6d89dd7decb34082353a0bde50c6597/components/favicon/ios/web_favicon_driver.mm
[modify] https://crrev.com/f235d928f6d89dd7decb34082353a0bde50c6597/components/image_fetcher/image_data_fetcher.cc
[modify] https://crrev.com/f235d928f6d89dd7decb34082353a0bde50c6597/components/image_fetcher/image_data_fetcher.h
[modify] https://crrev.com/f235d928f6d89dd7decb34082353a0bde50c6597/components/image_fetcher/image_data_fetcher_unittest.cc
[modify] https://crrev.com/f235d928f6d89dd7decb34082353a0bde50c6597/components/image_fetcher/request_metadata.cc
[modify] https://crrev.com/f235d928f6d89dd7decb34082353a0bde50c6597/components/image_fetcher/request_metadata.h
[modify] https://crrev.com/f235d928f6d89dd7decb34082353a0bde50c6597/components/image_fetcher/request_metadata_unittest.cc

Comment 20 by treib@chromium.org, Mar 17 2017

Blockedon: 702601
Project Member

Comment 21 by bugdroid1@chromium.org, Mar 17 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/72fb3888ae3ae20cc2706f5a6152bdce3a75b796

commit 72fb3888ae3ae20cc2706f5a6152bdce3a75b796
Author: treib <treib@chromium.org>
Date: Fri Mar 17 14:58:55 2017

[Doodle] Hook up LogoBridge to the new DoodleService

Hidden behind a flag, so no user-visible changes yet.

BUG= 690467 

Review-Url: https://codereview.chromium.org/2749813002
Cr-Commit-Position: refs/heads/master@{#457769}

[modify] https://crrev.com/72fb3888ae3ae20cc2706f5a6152bdce3a75b796/chrome/browser/android/logo_bridge.cc
[modify] https://crrev.com/72fb3888ae3ae20cc2706f5a6152bdce3a75b796/chrome/browser/android/logo_bridge.h
[modify] https://crrev.com/72fb3888ae3ae20cc2706f5a6152bdce3a75b796/chrome/browser/prefs/browser_prefs.cc
Project Member

Comment 22 by bugdroid1@chromium.org, Mar 20 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e

commit 0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e
Author: treib <treib@chromium.org>
Date: Mon Mar 20 13:10:30 2017

components/image_fetcher: Expose RequestMetadata from ImageFetcher

Before this CL, the metadata was returned by ImageDataFetcher, but not
forwarded by ImageFetcher to its clients. Now it is passed on from
ImageFetcher too.

So far, no client uses the metadata, but components/doodle will use it
soon, to detect if an image request was served from the http cache.
See also https://codereview.chromium.org/2749283002/.

TBRing mechanical changes to hats_notification_controller and logo_bridge
TBR=achuith@chromium.org,bauerb@chromium.org

BUG= 690467 

Review-Url: https://codereview.chromium.org/2757643002
Cr-Commit-Position: refs/heads/master@{#458048}

[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/chrome/browser/android/logo_bridge.cc
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/chrome/browser/android/logo_bridge.h
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/chrome/browser/chromeos/hats/hats_notification_controller.cc
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/chrome/browser/chromeos/hats/hats_notification_controller.h
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/chrome/browser/chromeos/hats/hats_notification_controller_unittest.cc
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/chrome/browser/search/suggestions/image_fetcher_impl_browsertest.cc
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/chrome/browser/search/thumbnail_source.cc
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/chrome/browser/search/thumbnail_source.h
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/components/image_fetcher/image_fetcher.h
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/components/image_fetcher/image_fetcher_impl.cc
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/components/image_fetcher/image_fetcher_impl.h
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/components/ntp_tiles/icon_cacher_impl.cc
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/components/ntp_tiles/icon_cacher_impl.h
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/components/ntp_tiles/icon_cacher_impl_unittest.cc
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/components/suggestions/image_manager.cc
[modify] https://crrev.com/0a6cfc6ed4370aa3f4974ac7ed0e1009e0f9687e/components/suggestions/image_manager_unittest.cc

Comment 23 by treib@chromium.org, Mar 20 2017

Blockedon: 703227
Project Member

Comment 25 by bugdroid1@chromium.org, Mar 23 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9a17ccc9dee66e65bef17be5a0d6fe6c5ee79767

commit 9a17ccc9dee66e65bef17be5a0d6fe6c5ee79767
Author: treib <treib@chromium.org>
Date: Thu Mar 23 08:57:20 2017

[Doodle] Cleanups in DoodleFetcherImplTest

BUG= 690467 

Review-Url: https://codereview.chromium.org/2765793004
Cr-Commit-Position: refs/heads/master@{#459028}

[modify] https://crrev.com/9a17ccc9dee66e65bef17be5a0d6fe6c5ee79767/components/doodle/doodle_fetcher_impl_unittest.cc
Project Member

Comment 27 by bugdroid1@chromium.org, Mar 27 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f617c93ceb1b5a6c56476d03f70832acf818e3e0

commit f617c93ceb1b5a6c56476d03f70832acf818e3e0
Author: fhorschig <fhorschig@chromium.org>
Date: Mon Mar 27 11:03:12 2017

Remove deprecated Doodle search URLs

The target_url contains urls to a search, a youtube video or an fpdoodle.

To provide a link on mobile, this is sufficient.
Desktop uses the same target_url for static doodles and receives an
HTML/JS snippet for everything else.

BUG= 690467 

Review-Url: https://codereview.chromium.org/2780453002
Cr-Commit-Position: refs/heads/master@{#459748}

[modify] https://crrev.com/f617c93ceb1b5a6c56476d03f70832acf818e3e0/components/doodle/doodle_fetcher_impl_unittest.cc
[modify] https://crrev.com/f617c93ceb1b5a6c56476d03f70832acf818e3e0/components/doodle/doodle_types.cc
[modify] https://crrev.com/f617c93ceb1b5a6c56476d03f70832acf818e3e0/components/doodle/doodle_types.h
[modify] https://crrev.com/f617c93ceb1b5a6c56476d03f70832acf818e3e0/components/doodle/doodle_types_unittest.cc
Project Member

Comment 30 by bugdroid1@chromium.org, Mar 28 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8442995784857246e240b4993ff16c1bff718430

commit 8442995784857246e240b4993ff16c1bff718430
Author: treib <treib@chromium.org>
Date: Tue Mar 28 15:23:32 2017

[Doodle] Set a maximum download size of 1 MB for the image

Analogous to what LogoTracker from components/search_provider_logos does.

BUG= 690467 

Review-Url: https://codereview.chromium.org/2780773003
Cr-Commit-Position: refs/heads/master@{#460108}

[modify] https://crrev.com/8442995784857246e240b4993ff16c1bff718430/chrome/browser/android/logo_bridge.cc

Comment 32 by treib@chromium.org, Apr 6 2017

Blocking: 708955

Comment 33 by treib@chromium.org, Apr 6 2017

Labels: -OS-iOS
Status: Fixed (was: Started)
Android implementation is done; iOS split off into  bug 708955 .

Comment 34 by treib@chromium.org, May 8 2017

Blockedon: 719513

Comment 35 by treib@chromium.org, Jun 23 2017

Status: Started (was: Fixed)
Reopening since plans have changed: The /async/ddljson API is still coming, but the client-side implementation will be different from the original plan. It'll live in the existing components/search_provider_logos; the new components/doodle will be removed again. Due to that, things will be delayed to M62.

Design doc isn't updated yet; I'll do that one of these days.
Project Member

Comment 36 by bugdroid1@chromium.org, Jun 23 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b74c4cd8feb9fa03d2b6e61e911d7043e98989d

commit 6b74c4cd8feb9fa03d2b6e61e911d7043e98989d
Author: Marc Treib <treib@chromium.org>
Date: Fri Jun 23 14:19:50 2017

Remove UseNewDoodleApi feature

This removes the plumbing in LogoBridge for using components/doodle.
The actual components/doodle code will be deleted separately later.

TBRing deletion of the c/b/doodle folder (which I in fact own, but the
commit queue doesn't seem to care :)
TBR=jochen

Bug:  690467 
Change-Id: Ie9b52bb3ab79f14795cbdd70d3fba4b9ada5032f
Reviewed-on: https://chromium-review.googlesource.com/544311
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481875}
[modify] https://crrev.com/6b74c4cd8feb9fa03d2b6e61e911d7043e98989d/chrome/browser/BUILD.gn
[modify] https://crrev.com/6b74c4cd8feb9fa03d2b6e61e911d7043e98989d/chrome/browser/about_flags.cc
[modify] https://crrev.com/6b74c4cd8feb9fa03d2b6e61e911d7043e98989d/chrome/browser/android/chrome_feature_list.cc
[modify] https://crrev.com/6b74c4cd8feb9fa03d2b6e61e911d7043e98989d/chrome/browser/android/chrome_feature_list.h
[modify] https://crrev.com/6b74c4cd8feb9fa03d2b6e61e911d7043e98989d/chrome/browser/android/logo_bridge.cc
[modify] https://crrev.com/6b74c4cd8feb9fa03d2b6e61e911d7043e98989d/chrome/browser/android/logo_bridge.h
[delete] https://crrev.com/410d60fc5fa219aed0af63776dc3998daecde2bd/chrome/browser/doodle/OWNERS
[delete] https://crrev.com/410d60fc5fa219aed0af63776dc3998daecde2bd/chrome/browser/doodle/doodle_service_factory.cc
[delete] https://crrev.com/410d60fc5fa219aed0af63776dc3998daecde2bd/chrome/browser/doodle/doodle_service_factory.h

Comment 37 by treib@chromium.org, Jun 29 2017

Blocking: 738073

Comment 38 by treib@chromium.org, Jul 5 2017

Cc: treib@chromium.org justincohen@chromium.org
 Issue 708955  has been merged into this issue.

Comment 39 by treib@chromium.org, Jul 5 2017

Labels: OS-iOS
Project Member

Comment 40 by bugdroid1@chromium.org, Jul 10 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bd5803be7cc328512d0d7817bf38612f524f69ac

commit bd5803be7cc328512d0d7817bf38612f524f69ac
Author: Marc Treib <treib@chromium.org>
Date: Mon Jul 10 09:45:57 2017

SearchProviderLogos: Add support for new /ddljson API

Hidden behind a new feature "UseDdljsonApi", available on Android and iOS.

Bug:  690467 
Change-Id: Ia31c1939159a9c1fc093fe478ea6ad91a61d6fe8
Reviewed-on: https://chromium-review.googlesource.com/544869
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485216}
[modify] https://crrev.com/bd5803be7cc328512d0d7817bf38612f524f69ac/chrome/browser/about_flags.cc
[modify] https://crrev.com/bd5803be7cc328512d0d7817bf38612f524f69ac/chrome/browser/android/logo_service.cc
[modify] https://crrev.com/bd5803be7cc328512d0d7817bf38612f524f69ac/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/bd5803be7cc328512d0d7817bf38612f524f69ac/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/bd5803be7cc328512d0d7817bf38612f524f69ac/components/search_provider_logos/BUILD.gn
[add] https://crrev.com/bd5803be7cc328512d0d7817bf38612f524f69ac/components/search_provider_logos/features.cc
[add] https://crrev.com/bd5803be7cc328512d0d7817bf38612f524f69ac/components/search_provider_logos/features.h
[modify] https://crrev.com/bd5803be7cc328512d0d7817bf38612f524f69ac/components/search_provider_logos/google_logo_api.cc
[modify] https://crrev.com/bd5803be7cc328512d0d7817bf38612f524f69ac/components/search_provider_logos/google_logo_api.h
[add] https://crrev.com/bd5803be7cc328512d0d7817bf38612f524f69ac/components/search_provider_logos/google_logo_api_unittest.cc
[modify] https://crrev.com/bd5803be7cc328512d0d7817bf38612f524f69ac/components/search_provider_logos/logo_common.h
[modify] https://crrev.com/bd5803be7cc328512d0d7817bf38612f524f69ac/components/search_provider_logos/logo_tracker.h
[modify] https://crrev.com/bd5803be7cc328512d0d7817bf38612f524f69ac/components/search_provider_logos/logo_tracker_unittest.cc
[modify] https://crrev.com/bd5803be7cc328512d0d7817bf38612f524f69ac/ios/chrome/browser/google/google_logo_service.mm
[modify] https://crrev.com/bd5803be7cc328512d0d7817bf38612f524f69ac/tools/metrics/histograms/enums.xml
Project Member

Comment 41 by bugdroid1@chromium.org, Jul 31 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/354586df155ea4c3a6db76025e4177c930088e58

commit 354586df155ea4c3a6db76025e4177c930088e58
Author: Marc Treib <treib@chromium.org>
Date: Mon Jul 31 14:46:06 2017

SearchProviderLogos: Parse animated logos correctly in new API

Previously, we were detecting them by an "is_animated" flag, but it is
actually called "is_animated_gif".

Bug:  690467 
Change-Id: I15337ac06886fcc737523b70a1d96fb36c19bf82
Reviewed-on: https://chromium-review.googlesource.com/593316
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490762}
[modify] https://crrev.com/354586df155ea4c3a6db76025e4177c930088e58/components/search_provider_logos/google_logo_api.cc
[modify] https://crrev.com/354586df155ea4c3a6db76025e4177c930088e58/components/search_provider_logos/google_logo_api_unittest.cc
Project Member

Comment 42 by bugdroid1@chromium.org, Aug 3 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/18873a37dea920be335c7ede66525b597d251a46

commit 18873a37dea920be335c7ede66525b597d251a46
Author: Marc Treib <treib@chromium.org>
Date: Thu Aug 03 14:31:07 2017

SearchProviderLogos: Add parsing tests for captured ddljson API output

This CL adds a script to download a number of test doodle configs from
the ddljson API. It also adds a captured snapshot of those configs, and
a test that they all get parsed without errors.

Bug:  690467 
Change-Id: Idf8375a4199fc129b905c7f8cd32f12d071389e9
Reviewed-on: https://chromium-review.googlesource.com/597647
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491731}
[modify] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/search_provider_logos/BUILD.gn
[modify] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/search_provider_logos/google_logo_api_unittest.cc
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/README
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/ddljson_android1.json
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/ddljson_android1_fp.json
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/ddljson_android2.json
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/ddljson_android2_fp.json
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/ddljson_android3.json
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/ddljson_android3_fp.json
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/ddljson_android4.json
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/ddljson_android4_fp.json
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/ddljson_ios1.json
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/ddljson_ios1_fp.json
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/ddljson_ios2.json
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/ddljson_ios2_fp.json
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/ddljson_ios3.json
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/ddljson_ios3_fp.json
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/ddljson_ios4.json
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/ddljson_ios4_fp.json
[add] https://crrev.com/18873a37dea920be335c7ede66525b597d251a46/components/test/data/search_provider_logos/download_test_doodles.sh
Project Member

Comment 44 by bugdroid1@chromium.org, Aug 4 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2752e8be828a7a37cf00f3067f26d914351dc688

commit 2752e8be828a7a37cf00f3067f26d914351dc688
Author: Marc Treib <treib@chromium.org>
Date: Fri Aug 04 14:12:09 2017

SearchProviderLogos: Add about:flags entry for UseDdljsonApi on iOS

This is basically the iOS version of
https://chromium-review.googlesource.com/c/600309.
It'll make manual testing of the feature much easier.

Bug:  690467 
Change-Id: I46656106f42821346bb323c655296e63e4ddd2b1
Reviewed-on: https://chromium-review.googlesource.com/602129
Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492020}
[modify] https://crrev.com/2752e8be828a7a37cf00f3067f26d914351dc688/ios/chrome/browser/BUILD.gn
[modify] https://crrev.com/2752e8be828a7a37cf00f3067f26d914351dc688/ios/chrome/browser/about_flags.mm
[modify] https://crrev.com/2752e8be828a7a37cf00f3067f26d914351dc688/ios/chrome/browser/ios_chrome_flag_descriptions.cc
[modify] https://crrev.com/2752e8be828a7a37cf00f3067f26d914351dc688/ios/chrome/browser/ios_chrome_flag_descriptions.h
Project Member

Comment 45 by bugdroid1@chromium.org, Aug 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/741a567e60bb8ac3e62dc534daacaaae63c1321d

commit 741a567e60bb8ac3e62dc534daacaaae63c1321d
Author: Marc Treib <treib@chromium.org>
Date: Mon Aug 07 10:25:56 2017

Add fieldtrial testing config for NTPUseDdljsonApi

Bug:  690467 
Change-Id: I133f7f6c046059e1bec03c41491324d5f0fa886d
Reviewed-on: https://chromium-review.googlesource.com/601988
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492290}
[modify] https://crrev.com/741a567e60bb8ac3e62dc534daacaaae63c1321d/testing/variations/fieldtrial_testing_config.json
Project Member

Comment 46 by bugdroid1@chromium.org, Aug 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/46d33ef8daa9b924b356a1bc75e3eb9acbc62e42

commit 46d33ef8daa9b924b356a1bc75e3eb9acbc62e42
Author: Marc Treib <treib@chromium.org>
Date: Mon Aug 07 15:06:53 2017

UseDdljsonApi: Use same trial name in about:flags and testing config

Before, the field trial was called "NTPUseDdljsonApi" in
fieldtrial_testing_config.json, but "UseDdljsonApi" in the about:flags
override. This caused developer builds to crash on startup if one of
the about:flags overrides was selected.

Bug:  690467 
Change-Id: I2cfcf42360ecfc2b548830df9934df4695dc5d9d
Reviewed-on: https://chromium-review.googlesource.com/603791
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492323}
[modify] https://crrev.com/46d33ef8daa9b924b356a1bc75e3eb9acbc62e42/chrome/browser/about_flags.cc
[modify] https://crrev.com/46d33ef8daa9b924b356a1bc75e3eb9acbc62e42/ios/chrome/browser/about_flags.mm
Project Member

Comment 47 by bugdroid1@chromium.org, Aug 8 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5b41fd7ac7d05d539029c763cc2031639d69d338

commit 5b41fd7ac7d05d539029c763cc2031639d69d338
Author: Marc Treib <treib@chromium.org>
Date: Tue Aug 08 08:26:23 2017

LogoTracker: slightly relax restrictions for using cached logos

This CL removes a restriction of the expiry time of a cached logo not
being too far in the future. This condition triggered for test doodles
with an expiry time that's very far away, which were thus never used
from cache. This wasn't a problem on Android (beyond delaying the test
doodle), but it broke things on iOS, where the additional in-memory
cache in GoogleLogoService got confused.

Bug:  690467 
Change-Id: Id2f3bc2152d2eb38dc25809d9fe3021e86a26a7c
Reviewed-on: https://chromium-review.googlesource.com/603795
Reviewed-by: Chris Pickel <sfiera@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492572}
[modify] https://crrev.com/5b41fd7ac7d05d539029c763cc2031639d69d338/components/search_provider_logos/logo_tracker.cc
Project Member

Comment 48 by bugdroid1@chromium.org, Aug 8 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a79c542cdbf02a7115f7a746e4623c1b8a9fa2e3

commit a79c542cdbf02a7115f7a746e4623c1b8a9fa2e3
Author: Marc Treib <treib@chromium.org>
Date: Tue Aug 08 14:02:34 2017

SearchProviderLogos: Add one more sample doodle to ddljson parsing test

Bug:  690467 
Change-Id: Ieb64e5a334770d3b551c77509614587c058165a5
Reviewed-on: https://chromium-review.googlesource.com/605250
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492613}
[modify] https://crrev.com/a79c542cdbf02a7115f7a746e4623c1b8a9fa2e3/components/search_provider_logos/BUILD.gn
[modify] https://crrev.com/a79c542cdbf02a7115f7a746e4623c1b8a9fa2e3/components/search_provider_logos/google_logo_api_unittest.cc
[add] https://crrev.com/a79c542cdbf02a7115f7a746e4623c1b8a9fa2e3/components/test/data/search_provider_logos/ddljson_android0.json
[add] https://crrev.com/a79c542cdbf02a7115f7a746e4623c1b8a9fa2e3/components/test/data/search_provider_logos/ddljson_android0_fp.json
[add] https://crrev.com/a79c542cdbf02a7115f7a746e4623c1b8a9fa2e3/components/test/data/search_provider_logos/ddljson_ios0.json
[add] https://crrev.com/a79c542cdbf02a7115f7a746e4623c1b8a9fa2e3/components/test/data/search_provider_logos/ddljson_ios0_fp.json
[modify] https://crrev.com/a79c542cdbf02a7115f7a746e4623c1b8a9fa2e3/components/test/data/search_provider_logos/download_test_doodles.sh
Project Member

Comment 49 by bugdroid1@chromium.org, Aug 8 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1aa164b53d55018b1f50d632b4d5fcdbef24fec8

commit 1aa164b53d55018b1f50d632b4d5fcdbef24fec8
Author: Marc Treib <treib@chromium.org>
Date: Tue Aug 08 15:37:40 2017

UseDdljsonApi: Add one more sample doodle to about:flags

Bug:  690467 
Change-Id: Ibbe59de225b07e292622bf5f4d89f61c676f8f9c
Reviewed-on: https://chromium-review.googlesource.com/605667
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492633}
[modify] https://crrev.com/1aa164b53d55018b1f50d632b4d5fcdbef24fec8/chrome/browser/about_flags.cc
[modify] https://crrev.com/1aa164b53d55018b1f50d632b4d5fcdbef24fec8/ios/chrome/browser/about_flags.mm
Project Member

Comment 50 by bugdroid1@chromium.org, Aug 16 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c87be5cc02fd974196c669b251e465028c2287c1

commit c87be5cc02fd974196c669b251e465028c2287c1
Author: Marc Treib <treib@chromium.org>
Date: Wed Aug 16 14:45:51 2017

SearchProviderLogos: don't create StringPiece of temporary

Before this CL, GoogleNewAppendQueryparamsToLogoURL collected the
required query params in a std::vector<base::StringPiece>. However, one
of the params (the fingerprint) was a temporary, leading to a
use-after-free. In practice, this sometimes messed up the request URL by
appending gibberish at the end.

This CL changes from vector<StringPiece> to vector<string> to avoid this
problem.

Bug:  690467 
Change-Id: I45d10e20e32d93d2c95b3bf0b35077e600a7c9b1
Reviewed-on: https://chromium-review.googlesource.com/616643
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494776}
[modify] https://crrev.com/c87be5cc02fd974196c669b251e465028c2287c1/components/search_provider_logos/google_logo_api.cc
[modify] https://crrev.com/c87be5cc02fd974196c669b251e465028c2287c1/components/search_provider_logos/google_logo_api_unittest.cc

Comment 51 by treib@chromium.org, Aug 21 2017

Labels: M-62
Project Member

Comment 52 by bugdroid1@chromium.org, Aug 24 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2c1ee78dc1d9eded9f6d912697489ca4787007d9

commit 2c1ee78dc1d9eded9f6d912697489ca4787007d9
Author: Marc Treib <treib@chromium.org>
Date: Thu Aug 24 08:50:43 2017

Delete components/doodle/

This was abandoned; we're staying with components/search_provider_logos/

Bug:  690467 
Change-Id: I521eeef152f62f39a2a519726d51e1730fdea5f3
Reviewed-on: https://chromium-review.googlesource.com/628418
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496988}
[modify] https://crrev.com/2c1ee78dc1d9eded9f6d912697489ca4787007d9/chrome/browser/BUILD.gn
[modify] https://crrev.com/2c1ee78dc1d9eded9f6d912697489ca4787007d9/chrome/browser/android/DEPS
[modify] https://crrev.com/2c1ee78dc1d9eded9f6d912697489ca4787007d9/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/2c1ee78dc1d9eded9f6d912697489ca4787007d9/components/BUILD.gn
[delete] https://crrev.com/3bca475d1bc6cb1ae4208a69a05199a249730c26/components/doodle/BUILD.gn
[delete] https://crrev.com/3bca475d1bc6cb1ae4208a69a05199a249730c26/components/doodle/DEPS
[delete] https://crrev.com/3bca475d1bc6cb1ae4208a69a05199a249730c26/components/doodle/OWNERS
[delete] https://crrev.com/3bca475d1bc6cb1ae4208a69a05199a249730c26/components/doodle/doodle_fetcher.h
[delete] https://crrev.com/3bca475d1bc6cb1ae4208a69a05199a249730c26/components/doodle/doodle_fetcher_impl.cc
[delete] https://crrev.com/3bca475d1bc6cb1ae4208a69a05199a249730c26/components/doodle/doodle_fetcher_impl.h
[delete] https://crrev.com/3bca475d1bc6cb1ae4208a69a05199a249730c26/components/doodle/doodle_fetcher_impl_unittest.cc
[delete] https://crrev.com/3bca475d1bc6cb1ae4208a69a05199a249730c26/components/doodle/doodle_service.cc
[delete] https://crrev.com/3bca475d1bc6cb1ae4208a69a05199a249730c26/components/doodle/doodle_service.h
[delete] https://crrev.com/3bca475d1bc6cb1ae4208a69a05199a249730c26/components/doodle/doodle_service_unittest.cc
[delete] https://crrev.com/3bca475d1bc6cb1ae4208a69a05199a249730c26/components/doodle/doodle_types.cc
[delete] https://crrev.com/3bca475d1bc6cb1ae4208a69a05199a249730c26/components/doodle/doodle_types.h
[delete] https://crrev.com/3bca475d1bc6cb1ae4208a69a05199a249730c26/components/doodle/doodle_types_unittest.cc
[delete] https://crrev.com/3bca475d1bc6cb1ae4208a69a05199a249730c26/components/doodle/pref_names.cc
[delete] https://crrev.com/3bca475d1bc6cb1ae4208a69a05199a249730c26/components/doodle/pref_names.h
[modify] https://crrev.com/2c1ee78dc1d9eded9f6d912697489ca4787007d9/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/2c1ee78dc1d9eded9f6d912697489ca4787007d9/tools/metrics/histograms/histograms.xml
Project Member

Comment 53 by bugdroid1@chromium.org, Aug 30 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c648abb27310c1bfdfbe3d5526cb78c2c5c6deed

commit c648abb27310c1bfdfbe3d5526cb78c2c5c6deed
Author: Marc Treib <treib@chromium.org>
Date: Wed Aug 30 09:16:31 2017

SearchProviderLogos ddljson API: Force https://, except for .cn

Bug:  690467 
Change-Id: Ifcbd5636a99a8189a8214a91950fd5b7fc3df824
Reviewed-on: https://chromium-review.googlesource.com/637831
Reviewed-by: Chris Palmer <palmer@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498408}
[modify] https://crrev.com/c648abb27310c1bfdfbe3d5526cb78c2c5c6deed/components/search_provider_logos/google_logo_api.cc
[modify] https://crrev.com/c648abb27310c1bfdfbe3d5526cb78c2c5c6deed/components/search_provider_logos/google_logo_api_unittest.cc
Project Member

Comment 54 by bugdroid1@chromium.org, Sep 28 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f9d466f1b3c41f80a842f9eeda0230d546ba32d8

commit f9d466f1b3c41f80a842f9eeda0230d546ba32d8
Author: Marc Treib <treib@chromium.org>
Date: Thu Sep 28 11:17:50 2017

SearchProviderLogos: enable UseDdljsonApi by default, remove fieldtrial config

Bug:  690467 
Change-Id: I6a169ad92f8e9fa2e2c316dc482aec3e621bfdb3
Reviewed-on: https://chromium-review.googlesource.com/628417
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504969}
[modify] https://crrev.com/f9d466f1b3c41f80a842f9eeda0230d546ba32d8/components/search_provider_logos/features.cc
[modify] https://crrev.com/f9d466f1b3c41f80a842f9eeda0230d546ba32d8/testing/variations/fieldtrial_testing_config.json

Comment 55 by treib@chromium.org, Oct 11 2017

Status: Fixed (was: Started)

Comment 56 by treib@chromium.org, Oct 11 2017

Blocking: 773719
►

Sign in to add a comment