New issue
Advanced search Search tips

Issue 686402 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

Do not use DomDistillerStore in ReadingList

Project Member Reported by olivierrobin@chromium.org, Jan 28 2017

Issue description

The service is not needed as it mainly handles caches that we do not want.
We should directly use the distiller.
 
Summary: Do not use DomDistillerStore in ReadingList (was: Do not use DomDistillerService in ReadingList)
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 1 2017

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

commit b227a4e9a84a3faae02a125085a49906e1b4c54a
Author: olivierrobin <olivierrobin@chromium.org>
Date: Wed Feb 01 17:43:52 2017

Allow DomDistillerService to have no store.

We do not want storage or sync of distilled versions of page as we store
them in files with ReadingListItems.
Do not instantiate the store in ReadingList for iOS.

BUG= 686402 

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

[modify] https://crrev.com/b227a4e9a84a3faae02a125085a49906e1b4c54a/components/dom_distiller/core/dom_distiller_service.cc
[modify] https://crrev.com/b227a4e9a84a3faae02a125085a49906e1b4c54a/ios/chrome/browser/dom_distiller/dom_distiller_service_factory.cc

Labels: M-57 Merge-Request-57
Status: Fixed (was: Assigned)
Project Member

Comment 4 by sheriffbot@chromium.org, Feb 1 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

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

Comment 5 by bugdroid1@chromium.org, Feb 2 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b39f83e80dfceebfedc15294355e931bc564736e

commit b39f83e80dfceebfedc15294355e931bc564736e
Author: Olivier Robin <olivierrobin@chromium.org>
Date: Thu Feb 02 09:08:50 2017

Allow DomDistillerService to have no store.

We do not want storage or sync of distilled versions of page as we store
them in files with ReadingListItems.
Do not instantiate the store in ReadingList for iOS.

BUG= 686402 

Review-Url: https://codereview.chromium.org/2667533003
Cr-Commit-Position: refs/heads/master@{#447551}
(cherry picked from commit b227a4e9a84a3faae02a125085a49906e1b4c54a)

Review-Url: https://codereview.chromium.org/2673603002 .
Cr-Commit-Position: refs/branch-heads/2987@{#268}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/b39f83e80dfceebfedc15294355e931bc564736e/components/dom_distiller/core/dom_distiller_service.cc
[modify] https://crrev.com/b39f83e80dfceebfedc15294355e931bc564736e/ios/chrome/browser/dom_distiller/dom_distiller_service_factory.cc

Sign in to add a comment