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

Issue 733166 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Feature



Sign in to add a comment

Remove custom hooks in GoogleLandingViewController

Project Member Reported by gambard@chromium.org, Jun 14 2017

Issue description

GoogleLandingViewController uses some custom hooks: it overrides -view to pass its own view.
This view resizes the subviews of the GoogleLandingViewController at every redrawing, updating their frames directly. This is because of historical reasons.

This will be done in multiple steps:
- Converting the frames resizing to autolayout
- Using the ViewController builtin resizing hooks
- Removing the customs view
 
As part of this refactoring, the NTP UI will slightly changes: the doodle frame will be 10 points higher and the fakeomnibox 26 points higher. It will match the constraints for the landscape format.
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 20 2017

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

commit d0a82df53c829fe6b9768c98aa1a86936b2cd774
Author: Gregory Chatzinoff <gchatz@chromium.org>
Date: Tue Jun 20 22:00:55 2017

Revert "Remove custom resizing hooks in GoogleLandingVC"

This reverts commit f6e429ff2786b7d1d2c16bd4902fa4b4b9ee5ce5.

Reason for revert: This CL broke a variety of external url
tests on ipad 32-bit configs.

Original change's description:
> Remove custom resizing hooks in GoogleLandingVC
> 
> GoogleLandingViewController used to have custom hooks to detect resizing events
> or orientation changes. This CL removes them and only use the generic
> UIViewController hooks.
> It also change the NTP constraints for the logo and the omnibox: the constraint
> and positioning is the same in landscape and portrait. The omnibox and doodle
> are now higher in portrait.
> 
> TEST= This CL refactors the NTP. Some regression testing should be done, in particular on rotations.
> Issues for which specific fixes have been removed: crbug.com/491131
> On iPad and in fullscreen, the collection view's inset is very large. When Chrome enters slide over mode,
> the previously set inset is larger than the newly set collection view's width, which makes the collection view throw an exception.
> 
> Bug:  733166 
> Change-Id: I2f6faf2468196cdd0a2faa191634b4da51254079
> Reviewed-on: https://chromium-review.googlesource.com/536915
> Commit-Queue: Gauthier Ambard <gambard@chromium.org>
> Reviewed-by: Justin Cohen <justincohen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#480453}

TBR=justincohen@chromium.org,gambard@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  733166 
Change-Id: I2aebc9327489d4a89861507ae9dedb166054cf4b
Reviewed-on: https://chromium-review.googlesource.com/541761
Reviewed-by: Gregory Chatzinoff <gchatz@chromium.org>
Commit-Queue: Gregory Chatzinoff <gchatz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#480979}
[modify] https://crrev.com/d0a82df53c829fe6b9768c98aa1a86936b2cd774/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils.mm
[modify] https://crrev.com/d0a82df53c829fe6b9768c98aa1a86936b2cd774/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils_unittest.mm
[modify] https://crrev.com/d0a82df53c829fe6b9768c98aa1a86936b2cd774/ios/chrome/browser/ui/ntp/google_landing_view_controller.mm
[modify] https://crrev.com/d0a82df53c829fe6b9768c98aa1a86936b2cd774/ios/chrome/browser/ui/ntp/new_tab_page_header_view.mm
[modify] https://crrev.com/d0a82df53c829fe6b9768c98aa1a86936b2cd774/ios/chrome/browser/ui/ntp/whats_new_header_view.h
[modify] https://crrev.com/d0a82df53c829fe6b9768c98aa1a86936b2cd774/ios/chrome/browser/ui/ntp/whats_new_header_view.mm

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 21 2017

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

commit fc0f3fca9419c2dfd3c342dc9ce892913b09b225
Author: gambard <gambard@chromium.org>
Date: Wed Jun 21 14:29:41 2017

Reland of "Remove custom resizing hooks in GoogleLandingVC"

GoogleLandingViewController used to have custom hooks to detect resizing events
or orientation changes. This CL removes them and only use the generic
UIViewController hooks.
It also change the NTP constraints for the logo and the omnibox: the constraint
and positioning is the same in landscape and portrait. The omnibox and doodle
are now higher in portrait.

TEST= This CL refactors the NTP. Some regression testing should be done, in particular on rotations.
Issues for which specific fixes have been removed: crbug.com/491131
On iPad and in fullscreen, the collection view's inset is very large. When Chrome enters slide over mode,
the previously set inset is larger than the newly set collection view's width, which makes the collection view throw an exception.

Bug:  733166 , 735026
Change-Id: Iaae7f5518aced14545fed33c0138024907fbe1e7
Reviewed-on: https://chromium-review.googlesource.com/542736
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481201}
[modify] https://crrev.com/fc0f3fca9419c2dfd3c342dc9ce892913b09b225/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils.mm
[modify] https://crrev.com/fc0f3fca9419c2dfd3c342dc9ce892913b09b225/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils_unittest.mm
[modify] https://crrev.com/fc0f3fca9419c2dfd3c342dc9ce892913b09b225/ios/chrome/browser/ui/ntp/google_landing_view_controller.mm
[modify] https://crrev.com/fc0f3fca9419c2dfd3c342dc9ce892913b09b225/ios/chrome/browser/ui/ntp/new_tab_page_header_view.mm
[modify] https://crrev.com/fc0f3fca9419c2dfd3c342dc9ce892913b09b225/ios/chrome/browser/ui/ntp/new_tab_page_toolbar_controller.mm
[modify] https://crrev.com/fc0f3fca9419c2dfd3c342dc9ce892913b09b225/ios/chrome/browser/ui/ntp/whats_new_header_view.h
[modify] https://crrev.com/fc0f3fca9419c2dfd3c342dc9ce892913b09b225/ios/chrome/browser/ui/ntp/whats_new_header_view.mm

Status: Fixed (was: Assigned)

Sign in to add a comment