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

Issue 794828 link

Starred by 2 users
Status: Fixed
Owner:
█ harringtond@chromium.org
Last visit > 30 days ago
Closed: May 2018
Cc:
mard...@chromium.org
█ aboss@chromium.org
dewittj@chromium.org
█ dim...@chromium.org
gambard@chromium.org
vitaliii@chromium.org
janicewong@chromium.org
█ bauerb@chromium.org
carlosk@chromium.org
█ chili@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Missing thumbnails from Zine suggestions

Project Member Reported by mdw@chromium.org, Dec 14 2017 
Chrome Version: 64.0.3281.0
OS: Android 8.1.0
Device: Pixel 2

What steps will reproduce the problem?
(1) Go into airplane mode or otherwise be offline
(2) Scroll up on Chrome Home

Many of the Zine article suggestions, even those that have been prefetched for offline reading, are missing image thumbnails.

See the attached screenshot.

I have had this happen to me several times so it seems to be a persistent bug, not just a transient issue.

I would expect that if Chrome has the headline and link, that the image thumbnails are also available offline. This should particularly be the case for any Zine suggestions that have been prefetched.
Screenshot_20171210-034528.png
155 KB View Download

Comment 1 by fhorschig@chromium.org, Dec 14 2017

Labels: -Pri-2 M-64 Needs-Feedback zine-triaged Pri-1
Has this worked in previous versions?
(Just to figure out whether it's a regression.)

Does this happen on iOS, too?

Comment 2 by tschumann@chromium.org, Dec 14 2017

Cc: vitaliii@chromium.org
This seems like an existing bug in the offline integration.
Zine manages the thumbnails itself, separately of the page contents.
My assumption is that we simply don't prefetch the thumbnail when we offline the article.

Comment 3 by mard...@chromium.org, Dec 14 2017

Cc: gambard@chromium.org
On iOS, sometimes some thumbnails don’t show if you are offline. This is not consistent though. iOS doesn’t prefetch though (I think).
Adding Gauthier who can confirm the expected behavior.
138D9286-BAD7-426E-8E17-F1DCC201B019.png
145 KB View Download

Comment 4 by vitaliii@chromium.org, Dec 14 2017

Cc: dewittj@chromium.org
To #2: that's correct. Only article content is prefetched so far.

Comment 5 by gambard@chromium.org, Dec 14 2017 

On iOS there is no prefetching on images. I would expect to have 0 image is there is no network (obviously you will have images for articles you have already seen while online).

Comment 6 by mdw@chromium.org, Dec 15 2017 

I'd like to tease apart two things: Offlining the article content vs. fetching the thumbnails.

I cannot tell for sure, but it seems that Zine (on Android at least) fetches the thumbnails separately from the article metadata (headline, link, etc.) - is that true? This would mean that you might have article suggestions being shown with no thumbnails, since the client might have fetched the former but not the latter.

If not already the case, my suggestion would be to embed the thumbnail into the metadata response from the Zine server, e.g., as base64 encoded data, so that whenever the client has the article metadata it also has the thumbnail. Has this been considered?

Even if we are unable to do this, however, I think we should consider embedding the thumbnail into the *prefetched article content* so that at least those articles that have been prefetched will have thumbnails. And we might consider a client change whereby only offlined articles are shown on the NTP when the user is offline.

Comment 7 by tschumann@chromium.org, Dec 15 2017 

Matt, you are correct, thumbnails are being fetched separately.
We considered embedding them into the response, but decided not to for data size reasons. Compared to thumbnails, the metadata is tiny and by separating them, we don't have to worry about data usage of more frequent background fetch intervals.
[i can dig out the precise numbers if you're curious].

What we should do for prefetching is at the time we decide to download the ready bundle, we should also request the thumbnails.

Vitalii, that should be relatively straight forward, right?

Comment 8 by vitaliii@chromium.org, Dec 15 2017

Cc: dim...@chromium.org
#7: this should be somewhat straight forward. We do not listen to prefetching, it happens transparently. The change will require us listening. Also, the prefetching may start much later, thus, we will have to get back the list of prefetched suggestions to know which thumbnails we need to download. 

Adding dimich@ for evaluation on their side.

Comment 9 by tschumann@chromium.org, Dec 15 2017 

Well, the alternative is that the prefetching code simply calls  FetchSuggestionImage() on the ContentSuggestionsService (or if that's tricky we provide a better suited hook) instead. Might make the interactions simpler and avoids another layer of indirection. That said, I don't know to which parts of our system the prefetching code already has access to. My preference is to not itnroduce more complexity and keep things as simple as they can be (ie. avoid regressions or unintended side effects :)).

Leaving this up to you vitalii and the prefetching team.

Comment 10 by vitaliii@chromium.org, Dec 15 2017 

#9: Calling FetchSuggestionImage() from prefetching code would simplify things a lot. This seems possible (they observer ContentSuggestionsService, which exposes FetchSuggestionImage). I just need to verify that it will be cached on our side and to double check when it is removed.

dimich@, does this sound good to you?

Comment 11 by vitaliii@chromium.org, Jan 3 2018

Owner: dim...@chromium.org
dimich@, ping.

I still think that it would be infeasible to trigger caching of images on our side, because currently the information flows only to the prefetching infrastructure and on our side we do not know when prefetch happens. As it was said above, we cache images on our side if they are requested, thus, it is enough to just request them during the prefetch.

Comment 12 by vitaliii@chromium.org, Jan 5 2018

Cc: carlosk@chromium.org

Comment 13 by dim...@chromium.org, Jan 5 2018 

Having thumbnails for prefetched articles (at least) would be nice. We are planning work in Q1 to fix the lack of thumbnails in other place - in Download Home. For that we'd want the access to the thumbnail as well. 

Also, there is a separate issue - when prefetch is enabled, device accumulates ~150 articles in a few days and keeps them rotated for freshness. However, Zine feed is normally only shows ~3-10 prefetched articles since it rotates at different schedule Even if we get a lot of thumbnails driven by prefetch, we might not solve it for Zine.

Our team will spin up an effort to design this soon, so stay tuned.

Comment 14 by carlosk@chromium.org, Jan 5 2018

Cc: chili@chromium.org

Comment 15 by vitaliii@chromium.org, Jan 16 2018 

To #13: dimich@, it looks like to have thumbnails in Zine, we don't need to cache thumbnails on your side (prefetching). We already have thumbnails caching in Zine and it looks possible to utilize it. 

Basically, the fix is to call ContentSuggestionsService::FetchSuggestionImage for all suggestions you prefetch when you prefetch them. This will cache thumbnails on Zine's side.

So it is up to you, whether you want to have this Zine cache based solution temporarily at first or to go for fully designed approach for both Downloads Home and Zine on your side.

Comment 16 by fhorschig@chromium.org, Jan 18 2018 

We are close to M-64 Release: will we move this to M-65?

Comment 17 by dim...@chromium.org, Jan 18 2018 

to #15: we'd be glad to use Zine thumbnail fetch/caching facility. I think this would also be compatible with future refactorings that are planned. we didn't play with that yet but we will, it's our near-term plan to have those thumbnails in Download Home. 

Vitalii, is there something that indicates/gcontrols a lifetime of the suggestion in Zine (and also of corresponding thumbnail)? We have pretty long expiration time for articles we download (since we already spent some resources to download them, they become a bit more valuable). We would like the thumbnail lifetime to ideally match our downloaded article lifetime. There is KeepPrefetchedContentSuggestions flag and corresponding code, but I believe it simply prolongs the lifetime  of a suggestion, there is no way to notify the Zine about downloaded article expiration and/or other way around, to make sure the suggestion and its downloaded snapshot have same lifetime... We can talk about it offline.

Comment 18 by dim...@chromium.org, Jan 18 2018

Cc: aboss@chromium.org

Comment 19 by vitaliii@chromium.org, Jan 19 2018 

#17: 

> it's our near-term plan to have those thumbnails in Download Home. 

Please note that I originally meant only Zine suggestions thumbnails. This case is extremely easy to support with the current setup.

Reusing Zine thumbnails fetching/caching for Download Home thumbnails will require more changes, but should be doable.

> is there something that indicates/gcontrols a lifetime of the suggestion in Zine 

Each suggestion has |expiry_date|. After this point in time the suggestion must not be shown to the user. However, it may get removed earlier, e.g. because of the fresher set of suggestion fetched. At the moment, |expiry_date| is set to |creation_date| + 3 days on the server for all suggestions.

> (and also of corresponding thumbnail)?

Thumbnail lifetime is completely defined by suggestion lifetime. As long as a suggestion can be shown to the user, the thumbnail is cached. 

> We would like the thumbnail lifetime to ideally match our downloaded article lifetime.

This requires quite some changes, but sounds doable. However, I am not sure whether this is optimal.

> KeepPrefetchedContentSuggestions flag and corresponding code, but I believe it simply prolongs the lifetime  of a suggestion, 

True, moreover, the prolongation works in a very limited manner. Without this flag, all suggestions are deleted when a new set is fetched. With this flag, some (at most 5) prefetched suggestions are kept when a new set is fetched.

> there is no way to notify the Zine about downloaded article expiration and/or other way around

True, at the moment there is no way.

> We can talk about it offline.

Sure. You should probably create a separate bug for Download Home thumbnails, because this bug is about Zine suggestion thumbnails. 

Do I understand right that you would rather have more complex solution working for both Download Home and Zine suggestions than an easy fix for Zine suggestions only?

Comment 20 by zea@chromium.org, Feb 13 2018

Labels: -Pri-1 Pri-2
Given this isn't a regression, lowering priority to P2.

Comment 21 by harringtond@chromium.org, Mar 8 2018

Owner: harringtond@chromium.org
Project Member

Comment 22 by bugdroid1@chromium.org, Mar 19 2018 

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

commit 343f6d9f447374dd8b96539d9b7a42c96efb0cea
Author: Dan Harrington <harringtond@chromium.org>
Date: Mon Mar 19 19:11:16 2018

Change ImageFetcher API

There is a need to fetch images without decoding them, so the
ImageFetcher's API was modified to allow callbacks for
image data and/or the decoded image. This made the ImageFetcherDelegate
superfluous and confusing, so it was removed.

Bug:  794828 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I809a692f5ee55fbe101adbb33210f456a4fdb27d
Reviewed-on: https://chromium-review.googlesource.com/955958
Commit-Queue: Dan H <harringtond@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544100}
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/chrome/browser/ntp_snippets/download_suggestions_provider.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/chrome/browser/search/suggestions/image_fetcher_impl_browsertest.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/favicon/core/BUILD.gn
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/favicon/core/large_icon_service.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/favicon/core/large_icon_service_unittest.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/image_fetcher/core/BUILD.gn
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/image_fetcher/core/image_fetcher.h
[delete] https://crrev.com/96f094cab85aa5a68ef871b548bd23c4c49ec6be/components/image_fetcher/core/image_fetcher_delegate.h
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/image_fetcher/core/image_fetcher_impl.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/image_fetcher/core/image_fetcher_impl.h
[add] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/image_fetcher/core/mock_image_fetcher.cc
[add] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/image_fetcher/core/mock_image_fetcher.h
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/ntp_snippets/BUILD.gn
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/ntp_snippets/callbacks.h
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/ntp_snippets/contextual/contextual_content_suggestions_service.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/ntp_snippets/contextual/contextual_content_suggestions_service_unittest.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/ntp_snippets/remote/cached_image_fetcher.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/ntp_snippets/remote/cached_image_fetcher.h
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/ntp_snippets/remote/cached_image_fetcher_unittest.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/ntp_snippets/remote/remote_suggestions_database.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/ntp_snippets/remote/remote_suggestions_database.h
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/ntp_tiles/BUILD.gn
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/ntp_tiles/icon_cacher_impl.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/ntp_tiles/icon_cacher_impl_unittest.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/signin/core/browser/account_fetcher_service.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/signin/core/browser/account_fetcher_service.h
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/suggestions/BUILD.gn
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/suggestions/image_manager.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/suggestions/image_manager.h
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/components/suggestions/image_manager_unittest.cc
[modify] https://crrev.com/343f6d9f447374dd8b96539d9b7a42c96efb0cea/ios/chrome/browser/passwords/notify_auto_signin_view_controller.mm
Project Member

Comment 23 by bugdroid1@chromium.org, Mar 20 2018 

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

commit 6eb74dc867e7bbff531394a5e78f13b7f212202b
Author: Dan Harrington <harringtond@chromium.org>
Date: Tue Mar 20 17:39:18 2018

Add FetchSuggestionImageData to ContentSuggestionsProvider

This allows fetching raw image data without decoding the image.

Bug:  794828 
Change-Id: I42f060d2e582a745862f8e63fefd9e2d9243f514
Reviewed-on: https://chromium-review.googlesource.com/969454
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Commit-Queue: Dan H <harringtond@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544427}
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/chrome/browser/android/ntp/content_suggestions_notifier_service_unittest.cc
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/chrome/browser/ntp_snippets/download_suggestions_provider.cc
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/chrome/browser/ntp_snippets/download_suggestions_provider.h
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/bookmarks/bookmark_suggestions_provider.cc
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/bookmarks/bookmark_suggestions_provider.h
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/content_suggestions_provider.h
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/content_suggestions_service.cc
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/content_suggestions_service.h
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/mock_content_suggestions_provider.cc
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/mock_content_suggestions_provider.h
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/offline_pages/recent_tab_suggestions_provider.cc
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/offline_pages/recent_tab_suggestions_provider.h
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.cc
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.h
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/reading_list/reading_list_suggestions_provider.h
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/remote/remote_suggestions_scheduler_impl_unittest.cc
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/sessions/foreign_sessions_suggestions_provider.cc
[modify] https://crrev.com/6eb74dc867e7bbff531394a5e78f13b7f212202b/components/ntp_snippets/sessions/foreign_sessions_suggestions_provider.h
Project Member

Comment 25 by bugdroid1@chromium.org, Mar 30 2018 

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

commit c68101a5a6c59d76fed85084b526817ccb9736a4
Author: Dan Harrington <harringtond@chromium.org>
Date: Fri Mar 30 02:36:41 2018

Add page_thumbnails table and OfflinePageThumbnail

These pieces will be used to store/represent page thumbnails.

Bug:  794828 
Change-Id: I242ec8376a36b2b41590bf89fb166a924aa13ca7
Reviewed-on: https://chromium-review.googlesource.com/984952
Commit-Queue: Dan H <harringtond@chromium.org>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547077}
[modify] https://crrev.com/c68101a5a6c59d76fed85084b526817ccb9736a4/components/offline_pages/core/BUILD.gn
[modify] https://crrev.com/c68101a5a6c59d76fed85084b526817ccb9736a4/components/offline_pages/core/offline_page_metadata_store_sql.cc
[modify] https://crrev.com/c68101a5a6c59d76fed85084b526817ccb9736a4/components/offline_pages/core/offline_page_metadata_store_sql.h
[modify] https://crrev.com/c68101a5a6c59d76fed85084b526817ccb9736a4/components/offline_pages/core/offline_page_metadata_store_unittest.cc
[add] https://crrev.com/c68101a5a6c59d76fed85084b526817ccb9736a4/components/offline_pages/core/offline_page_thumbnail.cc
[add] https://crrev.com/c68101a5a6c59d76fed85084b526817ccb9736a4/components/offline_pages/core/offline_page_thumbnail.h
[add] https://crrev.com/c68101a5a6c59d76fed85084b526817ccb9736a4/components/offline_pages/core/offline_page_thumbnail_unittest.cc
Project Member

Comment 26 by bugdroid1@chromium.org, Apr 2 2018 

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

commit f076278cf74e4bc0400c0b996ce0fad227c4fd7d
Author: Dan Harrington <harringtond@chromium.org>
Date: Mon Apr 02 16:20:56 2018

Add offline pages model tasks for thumbnails

These are all the tasks required to save, load, and cleanup
thumbnails.

base change: https://chromium-review.googlesource.com/c/chromium/src/+/984952

Bug:  794828 
Change-Id: I1ce081f53913a9b4c24b7f00a9d173c3fc061696
Reviewed-on: https://chromium-review.googlesource.com/985014
Commit-Queue: Dan H <harringtond@chromium.org>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547444}
[modify] https://crrev.com/f076278cf74e4bc0400c0b996ce0fad227c4fd7d/components/offline_pages/core/BUILD.gn
[add] https://crrev.com/f076278cf74e4bc0400c0b996ce0fad227c4fd7d/components/offline_pages/core/model/cleanup_thumbnails_task.cc
[add] https://crrev.com/f076278cf74e4bc0400c0b996ce0fad227c4fd7d/components/offline_pages/core/model/cleanup_thumbnails_task.h
[add] https://crrev.com/f076278cf74e4bc0400c0b996ce0fad227c4fd7d/components/offline_pages/core/model/cleanup_thumbnails_task_unittest.cc
[add] https://crrev.com/f076278cf74e4bc0400c0b996ce0fad227c4fd7d/components/offline_pages/core/model/get_thumbnail_task.cc
[add] https://crrev.com/f076278cf74e4bc0400c0b996ce0fad227c4fd7d/components/offline_pages/core/model/get_thumbnail_task.h
[add] https://crrev.com/f076278cf74e4bc0400c0b996ce0fad227c4fd7d/components/offline_pages/core/model/get_thumbnail_task_unittest.cc
[add] https://crrev.com/f076278cf74e4bc0400c0b996ce0fad227c4fd7d/components/offline_pages/core/model/store_thumbnail_task.cc
[add] https://crrev.com/f076278cf74e4bc0400c0b996ce0fad227c4fd7d/components/offline_pages/core/model/store_thumbnail_task.h
[add] https://crrev.com/f076278cf74e4bc0400c0b996ce0fad227c4fd7d/components/offline_pages/core/model/store_thumbnail_task_unittest.cc
[modify] https://crrev.com/f076278cf74e4bc0400c0b996ce0fad227c4fd7d/components/offline_pages/core/offline_page_types.h
Project Member

Comment 27 by bugdroid1@chromium.org, Apr 4 2018 

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

commit ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a
Author: Dan Harrington <harringtond@chromium.org>
Date: Wed Apr 04 00:48:57 2018

Download thumbnails

Thumbnails are downloaded when a prefetched page finishes downloading.
This CL also adds thumbnail methods to OfflinePageModel.

Bug:  794828 
Change-Id: I91263c58028d2b18dba22d33ee9964e909a0823e
Reviewed-on: https://chromium-review.googlesource.com/985205
Commit-Queue: Dan H <harringtond@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547897}
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/chrome/browser/BUILD.gn
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/chrome/browser/offline_pages/prefetch/prefetch_service_factory.cc
[add] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/chrome/browser/offline_pages/prefetch/thumbnail_fetcher_impl.cc
[add] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/chrome/browser/offline_pages/prefetch/thumbnail_fetcher_impl.h
[add] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/chrome/browser/offline_pages/prefetch/thumbnail_fetcher_impl_unittest.cc
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/chrome/test/BUILD.gn
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/ntp_snippets/mock_content_suggestions_provider.cc
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/ntp_snippets/mock_content_suggestions_provider.h
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/model/offline_page_model_taskified.cc
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/model/offline_page_model_taskified.h
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/model/offline_page_model_taskified_unittest.cc
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/offline_page_model.h
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/BUILD.gn
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/download_completed_task.cc
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/download_completed_task.h
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/download_completed_task_unittest.cc
[add] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/mock_thumbnail_fetcher.cc
[add] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/mock_thumbnail_fetcher.h
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/prefetch_dispatcher.h
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/prefetch_dispatcher_impl.cc
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/prefetch_dispatcher_impl.h
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/prefetch_dispatcher_impl_unittest.cc
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/prefetch_download_flow_unittest.cc
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/prefetch_service.h
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/prefetch_service_impl.cc
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/prefetch_service_impl.h
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/prefetch_service_test_taco.cc
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/prefetch_service_test_taco.h
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/test_prefetch_dispatcher.cc
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/test_prefetch_dispatcher.h
[add] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/prefetch/thumbnail_fetcher.h
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/stub_offline_page_model.cc
[modify] https://crrev.com/ee5e69dfa6e5ea57eb9d5ccde3dc6767f9872d9a/components/offline_pages/core/stub_offline_page_model.h
Project Member

Comment 28 by bugdroid1@chromium.org, Apr 9 2018 

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

commit 7f92cb25cfe74606148982f31804e08c49785ac6
Author: Dan Harrington <harringtond@chromium.org>
Date: Mon Apr 09 22:10:19 2018

Provide thumbnails in DownloadUIAdapter

This makes thumbnails show up in downloads home and notifications
for prefetched pages.

Bug:  794828 
Change-Id: I77a1dd34d71c6b56531e31325ee7a13edc992229
Reviewed-on: https://chromium-review.googlesource.com/990201
Commit-Queue: Dan H <harringtond@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549295}
[modify] https://crrev.com/7f92cb25cfe74606148982f31804e08c49785ac6/chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadItemView.java
[modify] https://crrev.com/7f92cb25cfe74606148982f31804e08c49785ac6/chrome/browser/BUILD.gn
[modify] https://crrev.com/7f92cb25cfe74606148982f31804e08c49785ac6/chrome/browser/offline_pages/android/downloads/offline_page_download_bridge.cc
[add] https://crrev.com/7f92cb25cfe74606148982f31804e08c49785ac6/chrome/browser/offline_pages/thumbnail_decoder_impl.cc
[add] https://crrev.com/7f92cb25cfe74606148982f31804e08c49785ac6/chrome/browser/offline_pages/thumbnail_decoder_impl.h
[add] https://crrev.com/7f92cb25cfe74606148982f31804e08c49785ac6/chrome/browser/offline_pages/thumbnail_decoder_impl_unittest.cc
[modify] https://crrev.com/7f92cb25cfe74606148982f31804e08c49785ac6/chrome/test/BUILD.gn
[modify] https://crrev.com/7f92cb25cfe74606148982f31804e08c49785ac6/components/offline_pages/core/DEPS
[modify] https://crrev.com/7f92cb25cfe74606148982f31804e08c49785ac6/components/offline_pages/core/downloads/BUILD.gn
[modify] https://crrev.com/7f92cb25cfe74606148982f31804e08c49785ac6/components/offline_pages/core/downloads/download_ui_adapter.cc
[modify] https://crrev.com/7f92cb25cfe74606148982f31804e08c49785ac6/components/offline_pages/core/downloads/download_ui_adapter.h
[modify] https://crrev.com/7f92cb25cfe74606148982f31804e08c49785ac6/components/offline_pages/core/downloads/download_ui_adapter_unittest.cc
[modify] https://crrev.com/7f92cb25cfe74606148982f31804e08c49785ac6/components/offline_pages/core/recent_tabs/recent_tabs_ui_adapter_delegate.cc
[modify] https://crrev.com/7f92cb25cfe74606148982f31804e08c49785ac6/components/offline_pages/core/recent_tabs/recent_tabs_ui_adapter_delegate_unittest.cc
[add] https://crrev.com/7f92cb25cfe74606148982f31804e08c49785ac6/components/offline_pages/core/thumbnail_decoder.h
Project Member

Comment 29 by bugdroid1@chromium.org, Apr 10 2018 

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

commit 33f4e02722685b77fe52e36ef486a125bc96bafd
Author: Dan Harrington <harringtond@chromium.org>
Date: Tue Apr 10 01:36:02 2018

Add UMA for thumbnail fetching/cleanup

Bug:  794828 
Change-Id: I6264f900967b9fc995b250c2968b299a31bb81db
Reviewed-on: https://chromium-review.googlesource.com/988937
Commit-Queue: Dan H <harringtond@chromium.org>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549370}
[modify] https://crrev.com/33f4e02722685b77fe52e36ef486a125bc96bafd/chrome/browser/offline_pages/prefetch/thumbnail_fetcher_impl.cc
[modify] https://crrev.com/33f4e02722685b77fe52e36ef486a125bc96bafd/chrome/browser/offline_pages/prefetch/thumbnail_fetcher_impl_unittest.cc
[modify] https://crrev.com/33f4e02722685b77fe52e36ef486a125bc96bafd/components/offline_pages/core/model/cleanup_thumbnails_task.cc
[modify] https://crrev.com/33f4e02722685b77fe52e36ef486a125bc96bafd/components/offline_pages/core/model/cleanup_thumbnails_task_unittest.cc
[modify] https://crrev.com/33f4e02722685b77fe52e36ef486a125bc96bafd/components/offline_pages/core/model/offline_page_model_taskified_unittest.cc
[modify] https://crrev.com/33f4e02722685b77fe52e36ef486a125bc96bafd/components/offline_pages/core/prefetch/prefetch_dispatcher_impl.cc
[modify] https://crrev.com/33f4e02722685b77fe52e36ef486a125bc96bafd/components/offline_pages/core/prefetch/prefetch_dispatcher_impl.h
[modify] https://crrev.com/33f4e02722685b77fe52e36ef486a125bc96bafd/components/offline_pages/core/prefetch/prefetch_dispatcher_impl_unittest.cc
[modify] https://crrev.com/33f4e02722685b77fe52e36ef486a125bc96bafd/components/offline_pages/core/prefetch/thumbnail_fetcher.h
[modify] https://crrev.com/33f4e02722685b77fe52e36ef486a125bc96bafd/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/33f4e02722685b77fe52e36ef486a125bc96bafd/tools/metrics/histograms/histograms.xml
Project Member

Comment 30 by bugdroid1@chromium.org, Apr 25 2018 

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

commit 43f4559e6f7232740882012711aec30543360b01
Author: Dan Harrington <harringtond@chromium.org>
Date: Wed Apr 25 18:56:29 2018

Add OfflinePages.DownloadUI.PrefetchedItemHasThumbnail UMA

This will help us understand how many articles show
up with a thumbnail in downloads home.

Bug:  794828 
Change-Id: I4576620f877147e0d897e29af11186150ddcf816
Reviewed-on: https://chromium-review.googlesource.com/1024639
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Commit-Queue: Dan H <harringtond@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553684}
[modify] https://crrev.com/43f4559e6f7232740882012711aec30543360b01/components/offline_pages/core/downloads/download_ui_adapter.cc
[modify] https://crrev.com/43f4559e6f7232740882012711aec30543360b01/components/offline_pages/core/downloads/download_ui_adapter.h
[modify] https://crrev.com/43f4559e6f7232740882012711aec30543360b01/components/offline_pages/core/downloads/download_ui_adapter_unittest.cc
[modify] https://crrev.com/43f4559e6f7232740882012711aec30543360b01/tools/metrics/histograms/histograms.xml
Project Member

Comment 31 by bugdroid1@chromium.org, May 9 2018 

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

commit 3570d56cb11bdccedd4e71598d0c598fd965a881
Author: Carlos Knippschild <carlosk@chromium.org>
Date: Wed May 09 02:36:00 2018

Download thumbnails when GeneratePageBundle is requested

This change adds a new trigger to the downloading of thumbnails for
prefetched offline articles right after GeneratePageBundle is
requested. This is in addition to the later attempt made when a
prefetched article is successfully downloaded and aims at improving the
availability of article thumbnails.

Bug:  794828 
Change-Id: Ic857ab91f4f2befb327e91300005c59c10bc417d
Reviewed-on: https://chromium-review.googlesource.com/1047991
Commit-Queue: Carlos Knippschild <carlosk@chromium.org>
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557069}
[modify] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/BUILD.gn
[add] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/model/has_thumbnail_task.cc
[add] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/model/has_thumbnail_task.h
[add] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/model/has_thumbnail_task_unittest.cc
[modify] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/model/offline_page_model_taskified.cc
[modify] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/model/offline_page_model_taskified.h
[modify] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/model/offline_page_model_taskified_unittest.cc
[modify] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/offline_page_model.h
[modify] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/prefetch/generate_page_bundle_task.cc
[modify] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/prefetch/generate_page_bundle_task.h
[modify] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/prefetch/generate_page_bundle_task_unittest.cc
[modify] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/prefetch/prefetch_dispatcher.h
[modify] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/prefetch/prefetch_dispatcher_impl.cc
[modify] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/prefetch/prefetch_dispatcher_impl.h
[modify] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/prefetch/prefetch_dispatcher_impl_unittest.cc
[modify] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/prefetch/test_prefetch_dispatcher.cc
[modify] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/prefetch/test_prefetch_dispatcher.h
[modify] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/stub_offline_page_model.cc
[modify] https://crrev.com/3570d56cb11bdccedd4e71598d0c598fd965a881/components/offline_pages/core/stub_offline_page_model.h

Comment 32 by carlosk@chromium.org, May 9 2018

Status: Fixed (was: Assigned)
With the changes listed above Prefetching requests Zine to download thumbnails for articles when there's a high chance that they will actually be made available offline. 

This favors:
a) Avoiding downloading thumbnails for the ~40% of suggested articles that are never prefetched, and
b) Using the users data only when they are most likely to be on WiFi.

The compromise is that these requests might take up to 2 days to happen and by that time the respective Zine entry might have already been removed and then no thumbnail will be available.

Moreover, for the presentation in the DH we are also relying on a separate storage for the thumbnail data. So Zine entry expiration won't affect the prefetched article presentation at that other UI.

I'm marking this issue as fixed as we don't currently intend to work on further improvements for now. See issue 841522 (view restricted; sorry!).
Project Member

Comment 33 by bugdroid1@chromium.org, May 22 2018 

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

commit e4d10fd312fb132c5ac2cf14b19e9e9e077ef506
Author: Carlos Knippschild <carlosk@chromium.org>
Date: Tue May 22 00:18:56 2018

Update prefetch thumbnail download metrics

With the change to have an earlier attempt at downloading thumbnails for
prefetched articles, the completeness metric that was in place became
imprecise by mixing results from both attempts in the same buckets. This
change address that by adding more values to the existing tracking enum
that differentiate each of the attempts.

There also some minor, related changes to thumbnail fetching and
histogram testing.

Bug:  794828 
Change-Id: Ife84446aa3083d4d21cc1e2713fdf5f6d65ec82e
Reviewed-on: https://chromium-review.googlesource.com/1053355
Commit-Queue: Carlos Knippschild <carlosk@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560429}
[modify] https://crrev.com/e4d10fd312fb132c5ac2cf14b19e9e9e077ef506/base/test/histogram_tester.h
[modify] https://crrev.com/e4d10fd312fb132c5ac2cf14b19e9e9e077ef506/chrome/browser/offline_pages/prefetch/thumbnail_fetcher_impl.cc
[modify] https://crrev.com/e4d10fd312fb132c5ac2cf14b19e9e9e077ef506/chrome/browser/offline_pages/prefetch/thumbnail_fetcher_impl.h
[modify] https://crrev.com/e4d10fd312fb132c5ac2cf14b19e9e9e077ef506/chrome/browser/offline_pages/prefetch/thumbnail_fetcher_impl_unittest.cc
[modify] https://crrev.com/e4d10fd312fb132c5ac2cf14b19e9e9e077ef506/components/offline_pages/core/prefetch/mock_thumbnail_fetcher.h
[modify] https://crrev.com/e4d10fd312fb132c5ac2cf14b19e9e9e077ef506/components/offline_pages/core/prefetch/prefetch_dispatcher_impl.cc
[modify] https://crrev.com/e4d10fd312fb132c5ac2cf14b19e9e9e077ef506/components/offline_pages/core/prefetch/prefetch_dispatcher_impl.h
[modify] https://crrev.com/e4d10fd312fb132c5ac2cf14b19e9e9e077ef506/components/offline_pages/core/prefetch/prefetch_dispatcher_impl_unittest.cc
[modify] https://crrev.com/e4d10fd312fb132c5ac2cf14b19e9e9e077ef506/components/offline_pages/core/prefetch/thumbnail_fetcher.h
[modify] https://crrev.com/e4d10fd312fb132c5ac2cf14b19e9e9e077ef506/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/e4d10fd312fb132c5ac2cf14b19e9e9e077ef506/tools/metrics/histograms/histograms.xml

Comment 34 by krav...@chromium.org, May 22 2018 

Not see this issue on latest M68 builds
Project Member

Comment 35 by bugdroid1@chromium.org, May 25 2018 

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

commit 443760805c1a3305e9bd53de0a4ec43ac9351861
Author: Carlos Knippschild <carlosk@chromium.org>
Date: Fri May 25 22:04:35 2018

Fix histogram using attribute "units" when it should be "enum"

This fixes the presentation of this histogram in the UMA dashboards
where it currently shows as representing a numeric value.

Bug:  794828 
Change-Id: I549d7460440435861cb4364f303845cfcac5d2e0
Reviewed-on: https://chromium-review.googlesource.com/1072712
Reviewed-by: Brian White <bcwhite@chromium.org>
Commit-Queue: Carlos Knippschild <carlosk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562033}
[modify] https://crrev.com/443760805c1a3305e9bd53de0a4ec43ac9351861/tools/metrics/histograms/histograms.xml
►

Sign in to add a comment