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

Issue 705572 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Feature

Blocked on:
issue 692975
issue 709498
issue 714025



Sign in to add a comment

[Tracking bug] Experiment with fetching publisher's favicons from new Google favicon server.

Project Member Reported by jkrcal@chromium.org, Mar 27 2017

Issue description

We should:
 - implement a c++ functionality for fetching publisher's favicons to allow for platform independence;
 - create a field trial feature;
 - based on the feature, switch in the java code between the old java code to access the old Google favicon server and the new c++ code to access the new Google favicon server;
 - measure UMA success metrics for both code paths (in java);
 - perform an experiment to compare success rate of the two services.
 

Comment 1 by jkrcal@chromium.org, Mar 27 2017

Blockedon: 692975
Project Member

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

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

commit 18514e66cea2de4f902f300d6b136f4bb9b18b6b
Author: jkrcal <jkrcal@chromium.org>
Date: Thu Mar 30 06:12:28 2017

[Content suggestions] Add a feature to fetch favicons from a new server

Together with CL 2781583002, this CL prepares fetching publishers
favicons in c++, using the new API function of LargeIconService.

In follow-up CLs, I will:
 - implement the actual functionality in ContentSuggestionsService,
 - switch in SnippetArticleViewHolder between the old and the new code
   path, based on the feature introduced in this CL.

BUG= 705572 

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

[modify] https://crrev.com/18514e66cea2de4f902f300d6b136f4bb9b18b6b/chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java
[modify] https://crrev.com/18514e66cea2de4f902f300d6b136f4bb9b18b6b/chrome/browser/about_flags.cc
[modify] https://crrev.com/18514e66cea2de4f902f300d6b136f4bb9b18b6b/chrome/browser/android/chrome_feature_list.cc
[modify] https://crrev.com/18514e66cea2de4f902f300d6b136f4bb9b18b6b/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/18514e66cea2de4f902f300d6b136f4bb9b18b6b/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/18514e66cea2de4f902f300d6b136f4bb9b18b6b/components/ntp_snippets/features.cc
[modify] https://crrev.com/18514e66cea2de4f902f300d6b136f4bb9b18b6b/components/ntp_snippets/features.h
[modify] https://crrev.com/18514e66cea2de4f902f300d6b136f4bb9b18b6b/tools/metrics/histograms/histograms.xml

Blockedon: 709498

Comment 4 by jkrcal@chromium.org, Apr 21 2017

Blockedon: 714025
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 25 2017

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

commit 99b05cd8620a446c0e84fefbf28e8db907b44bdc
Author: jkrcal <jkrcal@chromium.org>
Date: Tue Apr 25 18:27:08 2017

[LargeIconService] Allow overriding request URL by a field trial param

This CL integrates last minute comments from product on the first
experiment with fetch favicons on demand - for "Articles for you" on the
New Tab Page.

This CL further allows to override to request URL format using a field
trial parameter so that operating that experiment is more flexible.

BUG= 705572 

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

[modify] https://crrev.com/99b05cd8620a446c0e84fefbf28e8db907b44bdc/components/favicon/core/large_icon_service.cc
[modify] https://crrev.com/99b05cd8620a446c0e84fefbf28e8db907b44bdc/components/favicon/core/large_icon_service_unittest.cc

Comment 6 by jkrcal@chromium.org, Apr 27 2017

Labels: Merge-Request-59
Status: Verified (was: Started)
The last CL has been verified on 60.0.3081.0.
Project Member

Comment 7 by sheriffbot@chromium.org, Apr 27 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 28 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/258b6493bff7901661c887ac3b08e5d2a83e2ad7

commit 258b6493bff7901661c887ac3b08e5d2a83e2ad7
Author: Jan Krcal <jkrcal@chromium.org>
Date: Fri Apr 28 11:39:46 2017

[LargeIconService] Allow overriding request URL by a field trial param

This CL integrates last minute comments from product on the first
experiment with fetch favicons on demand - for "Articles for you" on the
New Tab Page.

This CL further allows to override to request URL format using a field
trial parameter so that operating that experiment is more flexible.

BUG= 705572 

Review-Url: https://codereview.chromium.org/2839653002
Cr-Commit-Position: refs/heads/master@{#467051}
(cherry picked from commit 99b05cd8620a446c0e84fefbf28e8db907b44bdc)

Review-Url: https://codereview.chromium.org/2849813002 .
Cr-Commit-Position: refs/branch-heads/3071@{#290}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/258b6493bff7901661c887ac3b08e5d2a83e2ad7/components/favicon/core/large_icon_service.cc
[modify] https://crrev.com/258b6493bff7901661c887ac3b08e5d2a83e2ad7/components/favicon/core/large_icon_service_unittest.cc

Sign in to add a comment