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

Issue 778236 link

Starred by 1 user
Status: Fixed
Owner:
gambard@chromium.org
Closed: Jan 2018
Cc:
sczs@chromium.org
gambard@chromium.org
justincohen@chromium.org
█ lod@chromium.org
ios-bugs-priority@chromium.org
ios-bugs@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Task



Sign in to add a comment

Remove declaration of viewSafeAreaInsetsDidChange in ToolbarController's interface

Project Member Reported by gambard@chromium.org, Oct 25 2017 
ToolbarController is declaring the viewSafeAreaInsetsDidChange method in its interface as it is not a UIViewController.
Once it is a real UIViewController, remove the declaration of this method.
Its call can also be removed once it is a contained ViewController.
Project Member

Comment 1 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

Comment 2 by justincohen@chromium.org, Nov 20 2017

Cc: gambard@chromium.org justincohen@chromium.org
gambard@ can these be removed now, right now we are calling these methods twice.

Comment 3 by gambard@chromium.org, Nov 21 2017 

If the toolbarViewController is a child view controller, I think it is fine.

Comment 4 by justincohen@chromium.org, Nov 21 2017

Cc: sczs@chromium.org
Owner: lod@chromium.org
lod@ I think you are making changes in this area now -- perhaps you can take this while sczs@ is out.
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 9 2018 

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

commit 2094a8b25cb85f6422101584112502c80a9cef5c
Author: Gauthier Ambard <gambard@chromium.org>
Date: Tue Jan 09 14:13:31 2018

Remove legacy code

This CL removes legacy code added when the toolbar wasn't a
ViewController.
As it is now a child ViewController, the call to
viewSafeAreaInsetsDidChange is correctly propagated.

Bug:  778236 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ic552e43c3f6ed0daa50fb92ba9dd75f9e2573252
Reviewed-on: https://chromium-review.googlesource.com/856979
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527984}
[modify] https://crrev.com/2094a8b25cb85f6422101584112502c80a9cef5c/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/2094a8b25cb85f6422101584112502c80a9cef5c/ios/chrome/browser/ui/toolbar/web_toolbar_controller.mm

Comment 6 by gambard@chromium.org, Jan 9 2018

Cc: lod@chromium.org
Owner: gambard@chromium.org
Status: Fixed (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 10 2018 

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

commit d544d115bf1ecfe7687f4b2a448b4f24fbfabc5f
Author: Gauthier Ambard <gambard@chromium.org>
Date: Wed Jan 10 10:26:04 2018

Fix the WebToolbar

The viewSafeAreaInsetsDidChange method was calling a method specific
to the SafeArea toolbar.
This CL check the flag is enabled before calling the method.

Bug:  778236 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ia8fa9ec124f6f700823d2498e041d94b22301de3
Reviewed-on: https://chromium-review.googlesource.com/859237
Commit-Queue: Louis Romero <lpromero@chromium.org>
Reviewed-by: Louis Romero <lpromero@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528274}
[modify] https://crrev.com/d544d115bf1ecfe7687f4b2a448b4f24fbfabc5f/ios/chrome/browser/ui/toolbar/web_toolbar_controller.mm
►

Sign in to add a comment