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

Issue 605870 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 584429
issue 594122

Blocking:
issue 603267



Sign in to add a comment

Cache downloaded images for snippets on disk using LevelDB

Project Member Reported by bauerb@chromium.org, Apr 22 2016

Issue description

Cache downloaded images for snippets on disk.
 

Comment 1 by bauerb@chromium.org, Apr 22 2016

Blockedon: 594122
Blocking: -594122

Comment 2 by fi...@chromium.org, Apr 22 2016

isn't that the same as  issue 584429 ?

Comment 3 by bauerb@chromium.org, Apr 22 2016

No, that is just about doing the download on the native side. (The initial description also mentioned caching, but it makes sense to separate those.)

Comment 4 by finkm@google.com, Apr 22 2016

Status: Available (was: Untriaged)

Comment 5 by treib@chromium.org, May 2 2016

Labels: -Pri-2 Pri-1

Comment 6 by treib@chromium.org, May 3 2016

Cc: sfiera@chromium.org
Labels: zine-mr-iter-14
Owner: treib@chromium.org
Status: Assigned (was: Available)

Comment 7 by treib@chromium.org, May 3 2016

Labels: zine-mr-iter-13

Comment 8 by treib@chromium.org, May 4 2016

Summary: Cache downloaded images for snippets on disk using LevelDB (was: Cache downloaded images for snippets on disk)
Per investigation on  bug 594122 : We'll use LevelDB, with the existing proto wrappers.
Blocking: 603267

Comment 10 by treib@chromium.org, May 10 2016

Status: Started (was: Assigned)
Cc: markusheintz@chromium.org

Comment 12 by treib@chromium.org, May 12 2016

Labels: zine-mr-iter-15
Status: Work in progress. This will take a while, there are yaks to be shaved related to image fetching.
First step will be to create a LevelDB to store snippets *without* their thumbnails.

Comment 13 by treib@chromium.org, May 19 2016

Labels: zine-mr-iter-16
Status: Still WIP. I hope to get the first CL (LevelDB for snippets, but not thumbnails yet) out this week.

Comment 14 by treib@chromium.org, May 27 2016

Labels: zine-mr-iter-17
Status: First CL in review (not much progress this week).
Project Member

Comment 15 by bugdroid1@chromium.org, Jun 2 2016

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

commit 4db94acd2c51b5c67cc00175b5a0dad4f865eb03
Author: treib <treib@chromium.org>
Date: Thu Jun 02 11:32:42 2016

[NTP Snippets] Persist snippets in a LevelDB instead of prefs

BUG= 605870 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

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

[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/chrome/browser/ntp_snippets/ntp_snippets_service_factory.cc
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/chrome/browser/resources/snippets_internals.js
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/chrome/browser/ui/webui/snippets_internals_message_handler.cc
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/chrome/browser/ui/webui/snippets_internals_message_handler.h
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/components_tests.gyp
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets.gypi
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets/BUILD.gn
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets/DEPS
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets/ntp_snippet.cc
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets/ntp_snippet.h
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets/ntp_snippets_constants.cc
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets/ntp_snippets_constants.h
[add] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets/ntp_snippets_database.cc
[add] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets/ntp_snippets_database.h
[add] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets/ntp_snippets_database_unittest.cc
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets/ntp_snippets_service.cc
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets/ntp_snippets_service.h
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets/ntp_snippets_service_unittest.cc
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets/pref_names.cc
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets/pref_names.h
[add] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets/proto/BUILD.gn
[add] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/components/ntp_snippets/proto/ntp_snippets.proto
[modify] https://crrev.com/4db94acd2c51b5c67cc00175b5a0dad4f865eb03/ios/chrome/browser/ntp_snippets/ios_chrome_ntp_snippets_service_factory.cc

Labels: zine-mr-iter-18
First CL landed (LevelDB for snippets, but not images yet). Continuing...
Project Member

Comment 17 by bugdroid1@chromium.org, Jun 3 2016

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

commit 65f9bbedd611ba2fb9b1156014518111de3abf77
Author: treib <treib@chromium.org>
Date: Fri Jun 03 08:13:38 2016

NTPSnippetsDatabase: support multiple concurrent loads
(follow-up to https://codereview.chromium.org/1987333003)

So far, there are no multiple loads (yet), but this removes a weird wrinkle
from the API and doesn't really make anything any more complex.

BUG= 605870 

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

[modify] https://crrev.com/65f9bbedd611ba2fb9b1156014518111de3abf77/components/ntp_snippets/ntp_snippets_database.cc
[modify] https://crrev.com/65f9bbedd611ba2fb9b1156014518111de3abf77/components/ntp_snippets/ntp_snippets_database.h

Labels: M-53

Comment 19 by treib@chromium.org, Jun 10 2016

Labels: zine-mr-iter-19
Prototype of image caching done; waiting for image_fetcher changes.

Comment 21 by treib@chromium.org, Jun 17 2016

Labels: zine-mr-iter-20
First part landed, second part in review and hopefully landing soon.
Project Member

Comment 22 by bugdroid1@chromium.org, Jun 17 2016

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

commit e195cf32ea015eb62340da83540ae5f90f6b74a4
Author: treib <treib@chromium.org>
Date: Fri Jun 17 16:11:43 2016

[NTP Snippets] Cache images in a LevelDB

TBRing a trivial change to the iOS factory
TBR=noyau

BUG= 605870 

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

[modify] https://crrev.com/e195cf32ea015eb62340da83540ae5f90f6b74a4/chrome/browser/ntp_snippets/ntp_snippets_service_factory.cc
[modify] https://crrev.com/e195cf32ea015eb62340da83540ae5f90f6b74a4/chrome/browser/search/suggestions/image_fetcher_impl.cc
[modify] https://crrev.com/e195cf32ea015eb62340da83540ae5f90f6b74a4/components/image_fetcher/image_fetcher_delegate.h
[modify] https://crrev.com/e195cf32ea015eb62340da83540ae5f90f6b74a4/components/ntp_snippets/ntp_snippets_database.cc
[modify] https://crrev.com/e195cf32ea015eb62340da83540ae5f90f6b74a4/components/ntp_snippets/ntp_snippets_database.h
[modify] https://crrev.com/e195cf32ea015eb62340da83540ae5f90f6b74a4/components/ntp_snippets/ntp_snippets_database_unittest.cc
[modify] https://crrev.com/e195cf32ea015eb62340da83540ae5f90f6b74a4/components/ntp_snippets/ntp_snippets_service.cc
[modify] https://crrev.com/e195cf32ea015eb62340da83540ae5f90f6b74a4/components/ntp_snippets/ntp_snippets_service.h
[modify] https://crrev.com/e195cf32ea015eb62340da83540ae5f90f6b74a4/components/ntp_snippets/ntp_snippets_service_unittest.cc
[modify] https://crrev.com/e195cf32ea015eb62340da83540ae5f90f6b74a4/components/ntp_snippets/proto/ntp_snippets.proto
[modify] https://crrev.com/e195cf32ea015eb62340da83540ae5f90f6b74a4/ios/chrome/browser/ntp_snippets/ios_chrome_ntp_snippets_service_factory.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Jun 20 2016

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

commit 509a7f00acc736e6e528642ed1c982295ad69c25
Author: treib <treib@chromium.org>
Date: Mon Jun 20 11:12:49 2016

[NTP Snippets] Extend NTPSnippetsDatabase tests for images
Also clean up the existing tests a bit

BUG= 605870 

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

[modify] https://crrev.com/509a7f00acc736e6e528642ed1c982295ad69c25/components/ntp_snippets/ntp_snippets_database_unittest.cc

Comment 25 by treib@chromium.org, Jun 21 2016

Status: Fixed (was: Started)
Labels: zine-mr-MVP

Comment 27 by finkm@google.com, Jul 1 2016

Labels: -zine-mr-mvp
Labels: zine-mr-MVP

Sign in to add a comment