New issue
Advanced search Search tips

Issue 751628 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 27
Components:
EstimatedDays: ----
NextAction: 2018-11-27
OS: Android
Pri: 3
Type: Task
q4

Blocked on:
issue 714109



Sign in to add a comment

[Suggestions favicons] Clear up code for old favicon server when not needed any more

Project Member Reported by jkrcal@chromium.org, Aug 2 2017

Issue description

org.chromium.chrome.browser.suggestions.ImageFetcher has two ways of fetching favicons:
 - from the old (social) favicon server
 - from the new favicon server v2.

When we completely switch to the new server, we should remove the previous code:
 - simplify org.chromium.chrome.browser.suggestions.ImageFetcher
 - remove ensureIconIsAvailable from FaviconHelper.

This is unlikely to happen before M62 hits Stable.
 

Comment 1 by dgn@chromium.org, Aug 4 2017

Cc: galinap@google.com
Components: UI>Browser>NewTabPage
Labels: zine-triaged zine-client

Comment 2 by jkrcal@chromium.org, Nov 28 2017

Cc: -galinap@google.com jkrcal@chromium.org
Labels: target-jardin
Owner: ----
Status: Available (was: Assigned)
This probably goes away by itself with Jardin. Keeping around for the improbable case that it does not.

Update about the situation:
 - Due to jardin, we haven't switched to the new server on Android (because of process work that would bring no significant improvement).
 - If we still leave the old Zine interface on tablets or other rare-case situations, it seems reasonable to switch to the new server on Android and clean this up.

Comment 3 by jkrcal@chromium.org, Nov 28 2017

Components: -UI>Browser>NewTabPage UI>Browser>ContentSuggestions
Cc: -dgn@chromium.org -jkrcal@chromium.org
Components: -UI>Browser>ContentSuggestions UI>Browser>ContentSuggestions>Feed
Labels: KR-Simplified-NTP medium q4 O-Zine-Improvements
Owner: fgor...@chromium.org
Status: Assigned (was: Available)
Moving to the current milestone as a clean up after issue 714109
Blockedon: 714109
NextAction: 2018-10-23
Labels: -medium small
The patch is out for review.
https://chromium-review.googlesource.com/c/chromium/src/+/1291840

Turns out this was small.
The NextAction date has arrived: 2018-10-23
Status: Started (was: Assigned)
NextAction: 2018-11-20
The NextAction date has arrived: 2018-11-20
NextAction: 2018-11-27
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 26

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

commit 7e14a87d7043513ff90c65e60c5f2e8a61175c04
Author: Filip Gorski <fgorski@chromium.org>
Date: Mon Nov 26 22:33:29 2018

[Zine] Old favicon code clean up

Bug:  751628 
Change-Id: Ief50f500266330c5ecceff718001f92bd7d4f467
Reviewed-on: https://chromium-review.googlesource.com/c/1291840
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Filip Gorski <fgorski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610962}
[modify] https://crrev.com/7e14a87d7043513ff90c65e60c5f2e8a61175c04/chrome/android/java/src/org/chromium/chrome/browser/suggestions/ImageFetcher.java
[modify] https://crrev.com/7e14a87d7043513ff90c65e60c5f2e8a61175c04/chrome/android/java/src/org/chromium/chrome/browser/suggestions/SuggestionsBinder.java
[modify] https://crrev.com/7e14a87d7043513ff90c65e60c5f2e8a61175c04/chrome/android/java/src/org/chromium/chrome/browser/suggestions/SuggestionsUiDelegateImpl.java
[modify] https://crrev.com/7e14a87d7043513ff90c65e60c5f2e8a61175c04/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/snippets/ArticleSnippetsTest.java
[modify] https://crrev.com/7e14a87d7043513ff90c65e60c5f2e8a61175c04/chrome/android/junit/src/org/chromium/chrome/browser/suggestions/ImageFetcherTest.java
[modify] https://crrev.com/7e14a87d7043513ff90c65e60c5f2e8a61175c04/chrome/android/junit/src/org/chromium/chrome/browser/suggestions/TileGroupUnitTest.java

The NextAction date has arrived: 2018-11-27
Status: Closed (was: Started)
Status: Fixed (was: Closed)

Sign in to add a comment