New issue
Advanced search Search tips

Issue 778226 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Task



Sign in to add a comment

Remove references to (Web)ToolbarController

Project Member Reported by gambard@chromium.org, Oct 25 2017

Issue description

The ToolbarController and the WebToolbarController are used widely in the app.
Direct references to those classes should be replaced by protocol, or calls to an intermediary object (for now the coordinator for example), allowing an easier refactoring.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 26 2017

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

commit 82c8cc5aab02c4bd2e4504620d7f0414d04d9823
Author: Gauthier Ambard <gambard@chromium.org>
Date: Thu Oct 26 15:59:05 2017

Remove WebToolbarController references to BVC

This CL removes the direct references to the WebToolbarController in
BVC, except for the reference about the UI needed for the iPhone X fixes
which will be removed in a separate CL.

Bug:  778226 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ib5e7c34fc61c7cbeb73e14df88355239a907faa0
Reviewed-on: https://chromium-review.googlesource.com/738197
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511832}
[modify] https://crrev.com/82c8cc5aab02c4bd2e4504620d7f0414d04d9823/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/82c8cc5aab02c4bd2e4504620d7f0414d04d9823/ios/chrome/browser/ui/toolbar/toolbar_coordinator.h
[modify] https://crrev.com/82c8cc5aab02c4bd2e4504620d7f0414d04d9823/ios/chrome/browser/ui/toolbar/toolbar_coordinator.mm

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 27 2017

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

commit 100670f7ab3cdfe04279092652f4db8b3c64bbf5
Author: Gauthier Ambard <gambard@chromium.org>
Date: Fri Oct 27 09:54:27 2017

Remove WebToolbarController UI references in BVC

This CL removes direct reference to WebToolbarController for changing
its UI in BVC. It keeps the compatibility with the iPhone X fixes.

Bug:  778226 ,  778236 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I46dd66b29a5aa2c9c57ed09fe1cc72b562d38158
Reviewed-on: https://chromium-review.googlesource.com/738235
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Jean-François Geyelin <jif@chromium.org>
Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512137}
[modify] https://crrev.com/100670f7ab3cdfe04279092652f4db8b3c64bbf5/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/100670f7ab3cdfe04279092652f4db8b3c64bbf5/ios/chrome/browser/ui/ntp/new_tab_page_header_view.mm
[modify] https://crrev.com/100670f7ab3cdfe04279092652f4db8b3c64bbf5/ios/chrome/browser/ui/stack_view/stack_view_controller.mm
[modify] https://crrev.com/100670f7ab3cdfe04279092652f4db8b3c64bbf5/ios/chrome/browser/ui/toolbar/BUILD.gn
[modify] https://crrev.com/100670f7ab3cdfe04279092652f4db8b3c64bbf5/ios/chrome/browser/ui/toolbar/toolbar_controller+protected.h
[modify] https://crrev.com/100670f7ab3cdfe04279092652f4db8b3c64bbf5/ios/chrome/browser/ui/toolbar/toolbar_controller.h
[modify] https://crrev.com/100670f7ab3cdfe04279092652f4db8b3c64bbf5/ios/chrome/browser/ui/toolbar/toolbar_controller.mm
[modify] https://crrev.com/100670f7ab3cdfe04279092652f4db8b3c64bbf5/ios/chrome/browser/ui/toolbar/toolbar_coordinator.h
[modify] https://crrev.com/100670f7ab3cdfe04279092652f4db8b3c64bbf5/ios/chrome/browser/ui/toolbar/toolbar_coordinator.mm
[add] https://crrev.com/100670f7ab3cdfe04279092652f4db8b3c64bbf5/ios/chrome/browser/ui/toolbar/toolbar_utils.h
[add] https://crrev.com/100670f7ab3cdfe04279092652f4db8b3c64bbf5/ios/chrome/browser/ui/toolbar/toolbar_utils.mm
[modify] https://crrev.com/100670f7ab3cdfe04279092652f4db8b3c64bbf5/ios/chrome/browser/ui/toolbar/web_toolbar_controller.mm

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 30 2017

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

commit 29939db189167f8629ae0f699735c4d12973820a
Author: Gauthier Ambard <gambard@chromium.org>
Date: Mon Oct 30 16:47:31 2017

Remove use of WebToolbarController in Side Swipe

This CL removes the direct use of WebToolbarController to allow easier
refactoring.
As this is removed, it also breaks the functionalities previously
owned by BVC in two protocols implemented by the ToolbarCoordinator.

Bug:  778226 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I75ecd1c34532d2950c22d50bfa746e865084f819
Reviewed-on: https://chromium-review.googlesource.com/738251
Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512516}
[modify] https://crrev.com/29939db189167f8629ae0f699735c4d12973820a/ios/chrome/browser/ui/browser_view_controller.h
[modify] https://crrev.com/29939db189167f8629ae0f699735c4d12973820a/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/29939db189167f8629ae0f699735c4d12973820a/ios/chrome/browser/ui/side_swipe/BUILD.gn
[modify] https://crrev.com/29939db189167f8629ae0f699735c4d12973820a/ios/chrome/browser/ui/side_swipe/card_side_swipe_view.h
[modify] https://crrev.com/29939db189167f8629ae0f699735c4d12973820a/ios/chrome/browser/ui/side_swipe/card_side_swipe_view.mm
[modify] https://crrev.com/29939db189167f8629ae0f699735c4d12973820a/ios/chrome/browser/ui/side_swipe/side_swipe_controller.h
[modify] https://crrev.com/29939db189167f8629ae0f699735c4d12973820a/ios/chrome/browser/ui/side_swipe/side_swipe_controller.mm
[add] https://crrev.com/29939db189167f8629ae0f699735c4d12973820a/ios/chrome/browser/ui/side_swipe/side_swipe_toolbar_interacting.h
[modify] https://crrev.com/29939db189167f8629ae0f699735c4d12973820a/ios/chrome/browser/ui/tab_switcher/BUILD.gn
[modify] https://crrev.com/29939db189167f8629ae0f699735c4d12973820a/ios/chrome/browser/ui/tab_switcher/tab_switcher_transition_context.mm
[modify] https://crrev.com/29939db189167f8629ae0f699735c4d12973820a/ios/chrome/browser/ui/toolbar/BUILD.gn
[modify] https://crrev.com/29939db189167f8629ae0f699735c4d12973820a/ios/chrome/browser/ui/toolbar/toolbar_coordinator.h
[modify] https://crrev.com/29939db189167f8629ae0f699735c4d12973820a/ios/chrome/browser/ui/toolbar/toolbar_coordinator.mm
[add] https://crrev.com/29939db189167f8629ae0f699735c4d12973820a/ios/chrome/browser/ui/toolbar/toolbar_snapshot_providing.h

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 7 2017

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

commit 04ddb5197390dc133e72c031a16ad04e8339a15d
Author: Gauthier Ambard <gambard@chromium.org>
Date: Tue Nov 07 09:14:16 2017

Remove direct use of Toolbar in TabSwitcher

This CL removes the direct use of the Toolbar using the
relinquish/reparent methods. Instead it uses the frame of the toolbar.

Bug:  778226 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I016da12e4550d348c3d39f573629180e0b12d8ce
Reviewed-on: https://chromium-review.googlesource.com/753348
Reviewed-by: Mark Cogan <marq@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514423}
[modify] https://crrev.com/04ddb5197390dc133e72c031a16ad04e8339a15d/ios/chrome/browser/content_suggestions/content_suggestions_header_view_controller.mm
[modify] https://crrev.com/04ddb5197390dc133e72c031a16ad04e8339a15d/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/04ddb5197390dc133e72c031a16ad04e8339a15d/ios/chrome/browser/ui/ntp/google_landing_view_controller.mm
[modify] https://crrev.com/04ddb5197390dc133e72c031a16ad04e8339a15d/ios/chrome/browser/ui/ntp/new_tab_page_controller.mm
[modify] https://crrev.com/04ddb5197390dc133e72c031a16ad04e8339a15d/ios/chrome/browser/ui/ntp/new_tab_page_header_view.mm
[modify] https://crrev.com/04ddb5197390dc133e72c031a16ad04e8339a15d/ios/chrome/browser/ui/tab_switcher/tab_switcher_controller.mm
[modify] https://crrev.com/04ddb5197390dc133e72c031a16ad04e8339a15d/ios/chrome/browser/ui/toolbar/toolbar_owner.h

Project Member

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

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

commit 53351a2dad69fe8d3c825145ac99321dcb66a5d1
Author: Gauthier Ambard <gambard@chromium.org>
Date: Tue Nov 07 09:41:39 2017

Remove public declaration of WebToolbarController

This CL removes the declaration of the WebToolbarController property in
the ToolbarCoordinator. It prevents the application from using it
directly. It will ease the transition to the adaptive toolbar.

Bug:  778226 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ib3332f7481dc01ef066ee20685ce797231810745
Reviewed-on: https://chromium-review.googlesource.com/756738
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514431}
[modify] https://crrev.com/53351a2dad69fe8d3c825145ac99321dcb66a5d1/ios/chrome/browser/ui/toolbar/toolbar_coordinator.h
[modify] https://crrev.com/53351a2dad69fe8d3c825145ac99321dcb66a5d1/ios/chrome/browser/ui/toolbar/toolbar_coordinator.mm

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 14 2017

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

commit 276307e53588c8bbc852283cff8659a1f2124fdf
Author: Gauthier Ambard <gambard@chromium.org>
Date: Tue Nov 14 09:29:14 2017

Remove StackView old animation

This CL removes the code used by the StackView to do the previous
animation when entering/leaving.

Bug:  779514 ,  778226 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: If0b4b0207fcad38c95bb1b46d895354ce623a0d0
Reviewed-on: https://chromium-review.googlesource.com/747421
Reviewed-by: Mark Cogan <marq@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516246}
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/about_flags.mm
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/content_suggestions/content_suggestions_header_view_controller.mm
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ios_chrome_flag_descriptions.cc
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ios_chrome_flag_descriptions.h
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/ntp/google_landing_view_controller.mm
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/ntp/new_tab_page_controller.mm
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/ntp/new_tab_page_header_view.mm
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/omnibox/omnibox_text_field_ios.h
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/omnibox/omnibox_text_field_ios.mm
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/stack_view/stack_view_controller.mm
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/stack_view/stack_view_controller_private.h
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/toolbar/public/toolbar_controller_base_feature.h
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/toolbar/public/toolbar_controller_base_feature.mm
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/toolbar/public/toolbar_controller_constants.h
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/toolbar/public/toolbar_controller_constants.mm
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/toolbar/toolbar_controller+protected.h
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/toolbar/toolbar_controller.h
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/toolbar/toolbar_controller.mm
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/toolbar/toolbar_owner.h
[modify] https://crrev.com/276307e53588c8bbc852283cff8659a1f2124fdf/ios/chrome/browser/ui/toolbar/web_toolbar_controller.mm

Status: Fixed (was: Assigned)

Sign in to add a comment