New issue
Advanced search Search tips

Issue 620939 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 616244



Sign in to add a comment

ios_internal/chrome/browser/ui/side_swipe code should not use CRWWebController

Project Member Reported by eugene...@chromium.org, Jun 17 2016

Issue description

CRWWebController is a private class for web// and web::WebState should be used instead.
 
Cc: justincohen@chromium.org
Components: UI>Browser>Navigation
Labels: -Restrict-View-Google -Type-Bug -Pri-2 Pri-3 Type-Feature
Labels: -Type-Feature Type-Task
Labels: Proj-Not-WKBackForwardList
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 10 2017

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

commit b56ecad0783a7c0ced174012852c2596b9025001
Author: Eugene But <eugenebut@google.com>
Date: Thu Aug 10 21:09:52 2017

Removed CRWWebController.usePlaceholderOverlay property.

Use PagePlaceholderTabHelper instead.

Bug:  620939 ,620480
Change-Id: I00b42a2483bf1b4f94ffa42c221531473988d3ce
Reviewed-on: https://chromium-review.googlesource.com/604708
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493544}
[modify] https://crrev.com/b56ecad0783a7c0ced174012852c2596b9025001/ios/chrome/browser/tabs/tab_model.mm
[modify] https://crrev.com/b56ecad0783a7c0ced174012852c2596b9025001/ios/chrome/browser/ui/side_swipe/BUILD.gn
[modify] https://crrev.com/b56ecad0783a7c0ced174012852c2596b9025001/ios/chrome/browser/ui/side_swipe/card_side_swipe_view.mm
[modify] https://crrev.com/b56ecad0783a7c0ced174012852c2596b9025001/ios/chrome/browser/ui/side_swipe/side_swipe_controller.mm
[modify] https://crrev.com/b56ecad0783a7c0ced174012852c2596b9025001/ios/chrome/browser/web/page_placeholder_tab_helper.h
[modify] https://crrev.com/b56ecad0783a7c0ced174012852c2596b9025001/ios/chrome/browser/web/page_placeholder_tab_helper_unittest.mm
[modify] https://crrev.com/b56ecad0783a7c0ced174012852c2596b9025001/ios/web/web_state/ui/crw_web_controller.h
[modify] https://crrev.com/b56ecad0783a7c0ced174012852c2596b9025001/ios/web/web_state/ui/crw_web_controller.mm

Components: -UI>Browser>Navigation UI>Browser>Navigation>GestureNav
Cc: danyao@chromium.org
Labels: -Proj-Not-WKBackForwardList Proj-WKBackForwardList
ios/chrome/browser/ui/side_swipe will be unnecessary after switching to WKBackForwardList. In hat case Chrome can use WKWebView side swipe UI for the navigation.
ios/chrome/browser/ui/side_swipe is also used for 'toolbar swipe to change tabs'.  This functionality is unrelated to WKWebView side swipe for UI for navigation, therefor I expect side_swipe to live in post WKBackForwardList, albeit in a more simplified manner. 
Labels: -Proj-WKBackForwardList
Thanks Justin! Removing Proj-WKBackForwardList label.
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 27 2017

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

commit 44835133e5dae4bbc458a7f75a0caa453791342a
Author: Sylvain Defresne <sdefresne@chromium.org>
Date: Wed Dec 27 12:27:10 2017

Use PagePlaceholderTabHelper to present overlay.

USe PagePlaceholderTabHelper to present the grey image overlay instead
of the CRWWebController implementation.

Bug: 616244,  620939 
Change-Id: I60589825448892ff25edd37b6cdf681b3c253892
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/836989
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526219}
[modify] https://crrev.com/44835133e5dae4bbc458a7f75a0caa453791342a/ios/chrome/browser/tabs/tab.mm
[modify] https://crrev.com/44835133e5dae4bbc458a7f75a0caa453791342a/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/44835133e5dae4bbc458a7f75a0caa453791342a/ios/chrome/browser/ui/side_swipe/DEPS
[modify] https://crrev.com/44835133e5dae4bbc458a7f75a0caa453791342a/ios/chrome/browser/ui/side_swipe/side_swipe_controller.mm
[modify] https://crrev.com/44835133e5dae4bbc458a7f75a0caa453791342a/ios/chrome/browser/ui/tab_switcher/BUILD.gn
[modify] https://crrev.com/44835133e5dae4bbc458a7f75a0caa453791342a/ios/chrome/browser/ui/tab_switcher/tab_switcher_cache.mm
[modify] https://crrev.com/44835133e5dae4bbc458a7f75a0caa453791342a/ios/chrome/browser/web/page_placeholder_tab_helper.h
[modify] https://crrev.com/44835133e5dae4bbc458a7f75a0caa453791342a/ios/chrome/browser/web/page_placeholder_tab_helper.mm
[modify] https://crrev.com/44835133e5dae4bbc458a7f75a0caa453791342a/ios/chrome/browser/web/page_placeholder_tab_helper_unittest.mm
[modify] https://crrev.com/44835133e5dae4bbc458a7f75a0caa453791342a/ios/web/public/web_state/ui/crw_web_delegate.h

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 28 2017

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

commit 703d781779c52f2b7a77d7b4e44d0c85a1bc1997
Author: Sylvain Defresne <sdefresne@chromium.org>
Date: Thu Dec 28 17:45:49 2017

Remove code to display overlay from CRWWebController.

Remove unused code from CRWWebController now that chrome uses
PagePlaceholderTabHelper to display the gray overlay image.

Move the -defaultSnapshotImage implementation to Tab. It will
be moved to SnapshotTabHelper in a followup CL.

Bug:  620939 ,  621250 
Change-Id: I3eede1448ad24a3cc0ec1f6bb257a9d074f917d9
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/842546
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526303}
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/chrome/browser/tabs/tab.mm
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/chrome/browser/ui/side_swipe/BUILD.gn
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/chrome/browser/ui/side_swipe/DEPS
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/chrome/browser/ui/side_swipe/card_side_swipe_view.mm
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/chrome/browser/ui/stack_view/BUILD.gn
[delete] https://crrev.com/ebaae6e504b2a989d2b970b7e8b3be467effc7a8/ios/chrome/browser/ui/stack_view/DEPS
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/chrome/browser/ui/stack_view/card_set.mm
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/chrome/browser/ui/stack_view/card_set_unittest.mm
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/web/public/test/fakes/test_web_state.h
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/web/public/test/fakes/test_web_state.mm
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/web/public/web_state/ui/crw_web_delegate.h
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/web/public/web_state/web_state.h
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/web/web_state/ui/crw_web_controller.h
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/web/web_state/ui/crw_web_controller.mm
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/web/web_state/web_state_impl.h
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/web/web_state/web_state_impl.mm
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/web/web_state/web_state_impl_unittest.mm
[modify] https://crrev.com/703d781779c52f2b7a77d7b4e44d0c85a1bc1997/ios/web/web_state/web_state_unittest.mm

Status: Fixed (was: Available)

Sign in to add a comment