New issue
Advanced search Search tips

Issue 661642 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 3
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Task



Sign in to add a comment

Refactor the similarities between WebControllerSnapshotHelper and SnapshotManager

Project Member Reported by stkhapugin@chromium.org, Nov 2 2016

Issue description

Address TODO in web_controller_snapshot_helper.h.
 
Cc: eugene...@chromium.org
Cc: jif@chromium.org
Labels: -Type-Bug Type-Feature
Status: Available (was: Unconfirmed)
jif@ do you know who inherited snapshotting ownership from JB?

Comment 3 by jif@chromium.org, Nov 2 2016

I did.
Components: UI>Browser>Mobile>TabSwitcher
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 7 2017

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

commit be8ca3128c6f8302dfd37ba3610b1eb53087dd4e
Author: Sylvain Defresne <sdefresne@chromium.org>
Date: Thu Dec 07 09:41:31 2017

Delete SnapshotManager class.

Refactor Tab (and its unit test) and WebControllerSnapshotHelper to
directly use SnapshotCache (accessed via the SnapshotCacheFactory).

Move -generateSnapshotForView:withRect:overlays: method (and helper
functions) to WebControllerSnapshotHelper.

Bug:  661642 , 620465
Change-Id: Ie8cde629b99ca39b30cd1bc3354dac441cd5e562
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/810768
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522388}
[modify] https://crrev.com/be8ca3128c6f8302dfd37ba3610b1eb53087dd4e/ios/chrome/browser/snapshots/BUILD.gn
[modify] https://crrev.com/be8ca3128c6f8302dfd37ba3610b1eb53087dd4e/ios/chrome/browser/snapshots/snapshot_cache_factory.h
[modify] https://crrev.com/be8ca3128c6f8302dfd37ba3610b1eb53087dd4e/ios/chrome/browser/snapshots/snapshot_cache_factory.mm
[delete] https://crrev.com/840d3b217376f3ee5366501f948ee12b0499784b/ios/chrome/browser/snapshots/snapshot_manager.h
[delete] https://crrev.com/840d3b217376f3ee5366501f948ee12b0499784b/ios/chrome/browser/snapshots/snapshot_manager.mm
[modify] https://crrev.com/be8ca3128c6f8302dfd37ba3610b1eb53087dd4e/ios/chrome/browser/snapshots/web_controller_snapshot_helper.h
[modify] https://crrev.com/be8ca3128c6f8302dfd37ba3610b1eb53087dd4e/ios/chrome/browser/snapshots/web_controller_snapshot_helper.mm
[modify] https://crrev.com/be8ca3128c6f8302dfd37ba3610b1eb53087dd4e/ios/chrome/browser/tabs/tab.mm
[modify] https://crrev.com/be8ca3128c6f8302dfd37ba3610b1eb53087dd4e/ios/chrome/browser/tabs/tab_unittest.mm
[modify] https://crrev.com/be8ca3128c6f8302dfd37ba3610b1eb53087dd4e/ios/chrome/browser/ui/stack_view/stack_view_controller_perftest.mm

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 8 2017

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

commit 836829eb1399f6f977d0df3fd2789767ff1adc9b
Author: Sylvain Defresne <sdefresne@chromium.org>
Date: Fri Dec 08 10:28:25 2017

Clean WebControllerSnapshotHelper API.

As the WebControllerSnapshotHelper keep a reference to the Tab and
the CRWWebController, clean the API removing all references to the
CRWWebController class.

Remove the sessionID parameter from WebControllerSnapshotHelper
methods as this accessible via the Tab.

Remove duplicated method +defaultSnapshotImage calling the method
from CRWWebController.

Bug:  661641 ,  661642 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I179bafb4199eab72474e30ef22be953bbf60e3b6
Reviewed-on: https://chromium-review.googlesource.com/814396
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522753}
[modify] https://crrev.com/836829eb1399f6f977d0df3fd2789767ff1adc9b/ios/chrome/browser/snapshots/web_controller_snapshot_helper.h
[modify] https://crrev.com/836829eb1399f6f977d0df3fd2789767ff1adc9b/ios/chrome/browser/snapshots/web_controller_snapshot_helper.mm
[modify] https://crrev.com/836829eb1399f6f977d0df3fd2789767ff1adc9b/ios/chrome/browser/tabs/tab.mm

Labels: -Type-Feature Type-Task
Cc: -jif@chromium.org
Owner: edchin@chromium.org
Status: Assigned (was: Available)
To edchin@ for assessment; is this still relevant?
I could not find WebControllerSnapshotHelper class in Chrome for iOS codebase. Did we remove WebControllerSnapshotHelper?
Status: Fixed (was: Assigned)
WebControllerSnapshotHelper was renamed to SnapshotGenerator. 
https://chromium-review.googlesource.com/c/chromium/src/+/814535

Also, SnapshotManager class was deleted and the code was moved elsewhere. 
https://chromium-review.googlesource.com/c/chromium/src/+/810768

So this is done.

Sign in to add a comment