Convert main NTP panel to UIViewController.

Convert main NTP panel to UIViewController.

To ease modernizing the NTP only the internal panels are being converted
to UIViewControllers.  This means all the plumbing between the
BrowserViewController and the internal NTP panels (WebController, NTP)
hierarchy is skipped.  While normally the logic to push and pop a view
controller would be owned by a coordinator, in this case the old NTP
controller adds and removes child view controllers itself when a load
is initiated, and when WebController calls -willBeDismissed.

BUG=708319
Review-Url: https://codereview.chromium.org/2806153004
Cr-Commit-Position: refs/heads/master@{#464850}
Committed: https://chromium.googlesource.com/chromium/src/+/1f6e1cbafd32c584fe6197eaefd4edd2f08e1024

[justincohen, 2017-04-10 18:37:28]

The CQ bit was checked by justincohen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-10 18:38:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[justincohen, 2017-04-10 18:41:42]

The CQ bit was checked by justincohen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-10 18:41:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[justincohen, 2017-04-10 18:43:07]

justincohen@chromium.org changed reviewers:
+ rohitrao@chromium.org

[justincohen, 2017-04-10 18:43:07]

Thoughts?

[commit-bot: I haz the power, 2017-04-10 18:54:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-10 18:54:41]

Dry run: This issue passed the CQ dry run.

[rohitrao (ping after 24h), 2017-04-12 12:45:09]

https://codereview.chromium.org/2806153004/diff/40001/ios/chrome/browser/ui/n...
File ios/chrome/browser/ui/ntp/google_landing_controller.mm (right):

https://codereview.chromium.org/2806153004/diff/40001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/google_landing_controller.mm:408:
[(GoogleLandingView*)self.view setFrameDelegate:self];
You can redeclare the |view| property to be a GoogleLandingView*, to avoid
casting everywhere.

https://codereview.chromium.org/2806153004/diff/40001/ios/chrome/browser/ui/n...
File ios/chrome/browser/ui/ntp/new_tab_page_controller.h (right):

https://codereview.chromium.org/2806153004/diff/40001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/new_tab_page_controller.h:95: @property(nonatomic,
assign) UIViewController* parentViewController;
Blank line after.

https://codereview.chromium.org/2806153004/diff/40001/ios/chrome/browser/ui/n...
File ios/chrome/browser/ui/ntp/new_tab_page_controller.mm (right):

https://codereview.chromium.org/2806153004/diff/40001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/new_tab_page_controller.mm:285:
[googleLandingController_ removeFromParentViewController];
Why is this not covered by the willBeDismissed code below?

https://codereview.chromium.org/2806153004/diff/40001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/new_tab_page_controller.mm:496: UIView* view;
Consider changing this to:

UIView* panelView = nil;
UIViewController* panelViewController = nil;

Then you can set the two variables as needed in the conditional blocks, and if
|panelViewController| is non-nil at line 552, you can add it as a child.

This also gives us a place to leave a gigantic comment about what's happening
and why we're doing it this way.

[justincohen, 2017-04-12 14:22:21]

The CQ bit was checked by justincohen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-12 14:22:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[justincohen, 2017-04-12 14:23:12]

ptal

https://codereview.chromium.org/2806153004/diff/40001/ios/chrome/browser/ui/n...
File ios/chrome/browser/ui/ntp/google_landing_controller.mm (right):

https://codereview.chromium.org/2806153004/diff/40001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/google_landing_controller.mm:408:
[(GoogleLandingView*)self.view setFrameDelegate:self];
On 2017/04/12 12:45:09, rohitrao wrote:
> You can redeclare the |view| property to be a GoogleLandingView*, to avoid
> casting everywhere.

Done.

https://codereview.chromium.org/2806153004/diff/40001/ios/chrome/browser/ui/n...
File ios/chrome/browser/ui/ntp/new_tab_page_controller.h (right):

https://codereview.chromium.org/2806153004/diff/40001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/new_tab_page_controller.h:95: @property(nonatomic,
assign) UIViewController* parentViewController;
On 2017/04/12 12:45:09, rohitrao wrote:
> Blank line after.

Done.

https://codereview.chromium.org/2806153004/diff/40001/ios/chrome/browser/ui/n...
File ios/chrome/browser/ui/ntp/new_tab_page_controller.mm (right):

https://codereview.chromium.org/2806153004/diff/40001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/new_tab_page_controller.mm:285:
[googleLandingController_ removeFromParentViewController];
On 2017/04/12 12:45:09, rohitrao wrote:
> Why is this not covered by the willBeDismissed code below?

-willMoveToParentViewController is called before the view is removed.

-removeFromParentViewController is called after the view is removed.

https://codereview.chromium.org/2806153004/diff/40001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/new_tab_page_controller.mm:496: UIView* view;
On 2017/04/12 12:45:09, rohitrao wrote:
> Consider changing this to:
> 
> UIView* panelView = nil;
> UIViewController* panelViewController = nil;
> 
> Then you can set the two variables as needed in the conditional blocks, and if
> |panelViewController| is non-nil at line 552, you can add it as a child.
> 
> This also gives us a place to leave a gigantic comment about what's happening
> and why we're doing it this way.

I was planning on doing this when adding the other view controllers, but I can
do it now too.

[commit-bot: I haz the power, 2017-04-12 14:34:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-12 14:34:21]

Dry run: This issue passed the CQ dry run.

[rohitrao (ping after 24h), 2017-04-13 12:48:52]

lgtm

https://codereview.chromium.org/2806153004/diff/60001/ios/chrome/browser/ui/n...
File ios/chrome/browser/ui/ntp/new_tab_page_controller.mm (right):

https://codereview.chromium.org/2806153004/diff/60001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/new_tab_page_controller.mm:296: -
(void)willBeDismissed {
Explain that this is called by //web immediately before this controller's view
is removed from the view hierarchy.

[justincohen, 2017-04-13 12:54:06]

Description was changed from

==========
Convert main NTP panel to UIViewController.

BUG=708319
==========

to

==========
Convert main NTP panel to UIViewController.

To ease modernizing the NTP only the internal panels are being converted
to UIViewControllers.  This means all the plumbing between the
BrowserViewController and the internal NTP panels (WebController, NTP)
hierarchy is skipped.  While normally the logic to push and pop a view
controller would be owned by a coordinator, in this case the old NTP
controller adds and removes child view controllers itself when a load
is initiated, and when WebController calls -willBeDismissed.

BUG=708319
==========

[justincohen, 2017-04-13 13:57:28]

The CQ bit was checked by justincohen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-13 13:57:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[justincohen, 2017-04-13 13:58:02]

I will land this after M59 branches.

https://codereview.chromium.org/2806153004/diff/60001/ios/chrome/browser/ui/n...
File ios/chrome/browser/ui/ntp/new_tab_page_controller.mm (right):

https://codereview.chromium.org/2806153004/diff/60001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/new_tab_page_controller.mm:296: -
(void)willBeDismissed {
On 2017/04/13 12:48:52, rohitrao wrote:
> Explain that this is called by //web immediately before this controller's view
> is removed from the view hierarchy.

Done.

[commit-bot: I haz the power, 2017-04-13 14:08:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-13 14:08:27]

Dry run: This issue passed the CQ dry run.

[justincohen, 2017-04-15 01:36:30]

The CQ bit was checked by justincohen@chromium.org

[justincohen, 2017-04-15 01:36:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
rohitrao@chromium.org
Link to the patchset: https://codereview.chromium.org/2806153004/#ps80001
(title: "Update comments")

[commit-bot: I haz the power, 2017-04-15 01:36:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-15 01:48:21]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1492220190942950,
"parent_rev": "73f0465777d91348d35874b5d2ebd28e5dfff138", "commit_rev":
"1f6e1cbafd32c584fe6197eaefd4edd2f08e1024"}

[commit-bot: I haz the power, 2017-04-15 01:49:15]

Description was changed from

==========
Convert main NTP panel to UIViewController.

To ease modernizing the NTP only the internal panels are being converted
to UIViewControllers.  This means all the plumbing between the
BrowserViewController and the internal NTP panels (WebController, NTP)
hierarchy is skipped.  While normally the logic to push and pop a view
controller would be owned by a coordinator, in this case the old NTP
controller adds and removes child view controllers itself when a load
is initiated, and when WebController calls -willBeDismissed.

BUG=708319
==========

to

==========
Convert main NTP panel to UIViewController.

To ease modernizing the NTP only the internal panels are being converted
to UIViewControllers.  This means all the plumbing between the
BrowserViewController and the internal NTP panels (WebController, NTP)
hierarchy is skipped.  While normally the logic to push and pop a view
controller would be owned by a coordinator, in this case the old NTP
controller adds and removes child view controllers itself when a load
is initiated, and when WebController calls -willBeDismissed.

BUG=708319

Review-Url: https://codereview.chromium.org/2806153004
Cr-Commit-Position: refs/heads/master@{#464850}
Committed:
https://chromium.googlesource.com/chromium/src/+/1f6e1cbafd32c584fe6197eaefd4...
==========

[commit-bot: I haz the power, 2017-04-15 01:49:16]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/1f6e1cbafd32c584fe6197eaefd4...