☂ Umbrella bug for removing StatusBarHeight() |
|||||||||
Issue descriptionUmbrella bug for removing StatusBarHeight hack once and for all. iOS 11 or newer devices should probably use something along the lines of |safeAreaInsets.top| or similar. Places where StatusBarHeight() is referenced: snapshot_generator (for kBrowserContainerFullscreen only) bookmark_utils_ios uses it's own broken version browser_view_controller for fullscreen and header sizing ntp/content suggestions find_bar_controller_ios history_search_view incognito_view recent_tabs/panel_bar_view overscroll_actions (for kBrowserContainerFullscreen only) payment_request_picker_view_controller card_side_swipe_view stack_view_controller (deprecated?) tab_switcher_controller legacy/toolbar_controller It seems like BVC (toolbar placement, fullscreen) is the worst offender, with NTP/ContentSuggestions coming in at a close second. But there's a long tail of usage.
,
Sep 13
,
Sep 13
Just to identify ones that we don't have to touch, panel_bar_view and history_search_view look like they will be removed as part of the Bijou cleanup process.
,
Sep 18
,
Sep 20
,
Sep 20
,
Oct 1
,
Oct 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8c3ace002f93d1a61985012ba31a7dc082cb159c commit 8c3ace002f93d1a61985012ba31a7dc082cb159c Author: Justin Cohen <justincohen@google.com> Date: Wed Oct 03 14:59:54 2018 [ios] Remove StatusBarHeight from ContentSuggestionsCollectionUtils. Bug: 883770 Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs Change-Id: I2b7a57e57047677304ba572b9648e998563a8d39 Reviewed-on: https://chromium-review.googlesource.com/c/1256442 Commit-Queue: Justin Cohen <justincohen@chromium.org> Reviewed-by: Gauthier Ambard <gambard@chromium.org> Cr-Commit-Position: refs/heads/master@{#596211} [modify] https://crrev.com/8c3ace002f93d1a61985012ba31a7dc082cb159c/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils.h [modify] https://crrev.com/8c3ace002f93d1a61985012ba31a7dc082cb159c/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils.mm [modify] https://crrev.com/8c3ace002f93d1a61985012ba31a7dc082cb159c/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils_unittest.mm [modify] https://crrev.com/8c3ace002f93d1a61985012ba31a7dc082cb159c/ios/chrome/browser/ui/content_suggestions/content_suggestions_header_view_controller.mm [modify] https://crrev.com/8c3ace002f93d1a61985012ba31a7dc082cb159c/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm [modify] https://crrev.com/8c3ace002f93d1a61985012ba31a7dc082cb159c/ios/chrome/browser/ui/ntp/incognito_view.mm
,
Oct 3
,
Oct 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0940e06356b92d52b815c2664ba94e32eeed3c6e commit 0940e06356b92d52b815c2664ba94e32eeed3c6e Author: Justin Cohen <justincohen@google.com> Date: Sat Oct 06 17:46:18 2018 [ios] Remove remaining usage of StatusBarHeight. This should remove the remaining few calls to StatusBarHeight in >=iOS11 and with -kBrowserContainerFullscreen and kOutOfWebFullscreen enabled. StatusBarHeight() will now DCHECK if called on >=iOS11 with those features enabled. Bug: 883770 Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs Change-Id: Icd933faa6e223122da46d0e856600df86fd2e5e1 Reviewed-on: https://chromium-review.googlesource.com/c/1259344 Commit-Queue: Justin Cohen <justincohen@chromium.org> Reviewed-by: Peter Lee <pkl@chromium.org> Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org> Reviewed-by: Gauthier Ambard <gambard@chromium.org> Cr-Commit-Position: refs/heads/master@{#597437} [modify] https://crrev.com/0940e06356b92d52b815c2664ba94e32eeed3c6e/ios/chrome/browser/ui/browser_view_controller.mm [modify] https://crrev.com/0940e06356b92d52b815c2664ba94e32eeed3c6e/ios/chrome/browser/ui/content_suggestions/content_suggestions_egtest.mm [modify] https://crrev.com/0940e06356b92d52b815c2664ba94e32eeed3c6e/ios/chrome/browser/ui/content_suggestions/ntp_home_egtest.mm [modify] https://crrev.com/0940e06356b92d52b815c2664ba94e32eeed3c6e/ios/chrome/browser/ui/overscroll_actions/overscroll_actions_controller.mm [modify] https://crrev.com/0940e06356b92d52b815c2664ba94e32eeed3c6e/ios/chrome/browser/ui/payments/payment_request_picker_view_controller.mm [modify] https://crrev.com/0940e06356b92d52b815c2664ba94e32eeed3c6e/ios/chrome/browser/ui/side_swipe/card_side_swipe_view.mm [modify] https://crrev.com/0940e06356b92d52b815c2664ba94e32eeed3c6e/ios/chrome/browser/ui/ui_util.mm
,
Oct 26
Now that kBrowserContainerFullscreen and kOutOfWebFullscreen are defaulted on, and iOS10 will (soon) be deprecated, any call to StatusBarHeight will DCHECK. Removing RBS and moving the milestone to M72. The remainder of this bug will be to remove the -StatusBarHeight method, but that's just cleanup and should not block.
,
Nov 5
,
Nov 14
The only remaining usage of StatusBarHeight is behind the kBrowserContainerFullscreen flag. kkhorimoto@ can you let me know when we remove that flag?
,
Nov 15
It's enabled on trunk by default, and will be rolled out over M71. We should be able to delete it after we branch for M72, assuming the rollout goes fine. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by justincohen@chromium.org
, Sep 13