[ios] RootCoordinator and view controller.

[ios] RootCoordinator and view controller.

This CL places a root container at the root of the view controller hierarchy, 
instead of the tab grid. The root container has a content view controller
that can be swapped between regular mode or incognito mode. 
This refactor is required to enable switching to incognito mode.

BUG=686770
Review-Url: https://codereview.chromium.org/2800313002
Cr-Commit-Position: refs/heads/master@{#464420}
Committed: https://chromium.googlesource.com/chromium/src/+/b1f381c92360d29bf4defc89bf4d70af86dd27fb

[edchin, 2017-04-10 04:44:49]

Patchset #1 (id:1) has been deleted

[edchin, 2017-04-10 04:44:55]

Patchset #1 (id:20001) has been deleted

[edchin, 2017-04-10 04:45:03]

Patchset #1 (id:40001) has been deleted

[edchin, 2017-04-10 04:45:13]

Patchset #1 (id:60001) has been deleted

[edchin, 2017-04-10 04:45:42]

Patchset #1 (id:80001) has been deleted

[edchin, 2017-04-10 04:45:57]

Patchset #1 (id:100001) has been deleted

[edchin, 2017-04-10 04:53:55]

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

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

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

[edchin, 2017-04-10 05:01:33]

Description was changed from

==========
[ios] RootContainer.

BUG=
==========

to

==========
[ios] RootContainer.

This CL places a root container at the root of the view controller hierarchy, 
instead of the tab grid. The root container has a content view controller
that can be swapped between regular mode or incognito mode. 
This refactor is required to enable switching to incognito mode.

BUG=686770
==========

[edchin, 2017-04-10 05:01:33]

edchin@chromium.org changed reviewers:
+ lpromero@chromium.org, marq@chromium.org, sczs@chromium.org

[commit-bot: I haz the power, 2017-04-10 05:02:00]

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

[commit-bot: I haz the power, 2017-04-10 05:02:00]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[edchin, 2017-04-10 05:06:56]

Patchset #1 (id:120001) has been deleted

[edchin, 2017-04-10 05:09:23]

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

[commit-bot: I haz the power, 2017-04-10 05:09:35]

Dry run: 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-10 06:07:13]

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

[commit-bot: I haz the power, 2017-04-10 06:07:14]

Dry run: This issue passed the CQ dry run.

[marq (ping after 24h), 2017-04-10 11:04:25]

lgtm

https://codereview.chromium.org/2800313002/diff/140001/ios/clean/chrome/brows...
File ios/clean/chrome/browser/ui/root/root_container_coordinator.h (right):

https://codereview.chromium.org/2800313002/diff/140001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_coordinator.h:12: @interface
RootContainerCoordinator : BrowserCoordinator<URLOpening>
Naming nitpicks: I think just "RootCoordinator" is better, since the fact that
it uses containment is an implementation detail (this is why we have
TabCoordinator vs. TabContainerViewController).

https://codereview.chromium.org/2800313002/diff/140001/ios/clean/chrome/brows...
File ios/clean/chrome/browser/ui/root/root_container_coordinator.mm (right):

https://codereview.chromium.org/2800313002/diff/140001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_coordinator.mm:34: //
CommandDispatcher* dispatcher = self.browser->dispatcher();
Don't leave code commented out. I agree that we want to move URL opening into
the dispatcher, but that's not happening in this CL.

https://codereview.chromium.org/2800313002/diff/140001/ios/clean/chrome/brows...
File ios/clean/chrome/browser/ui/root/root_container_view_controller.mm (right):

https://codereview.chromium.org/2800313002/diff/140001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller.mm:41:
viewController.view.translatesAutoresizingMaskIntoConstraints = YES;
This is the default, no need to set it.

https://codereview.chromium.org/2800313002/diff/140001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller.mm:59: -
(CGRect)rectForZoomWithKey:(NSObject*)key inView:(UIView*)view {
No change for now, but I'd like to figure out a way to not have to forward
protocols like this.

https://codereview.chromium.org/2800313002/diff/140001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller.mm:62: return
[reinterpret_cast<id<ZoomTransitionDelegate>>(
Despite what I might have said in the past, this should be static_cast()

[marq (ping after 24h), 2017-04-10 11:05:01]

Sorry, not lgtm yet -- hit the wrong button.

[edchin, 2017-04-10 16:41:30]

Comments addressed. PTAL.

https://codereview.chromium.org/2800313002/diff/140001/ios/clean/chrome/brows...
File ios/clean/chrome/browser/ui/root/root_container_coordinator.h (right):

https://codereview.chromium.org/2800313002/diff/140001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_coordinator.h:12: @interface
RootContainerCoordinator : BrowserCoordinator<URLOpening>
On 2017/04/10 11:04:25, marq wrote:
> Naming nitpicks: I think just "RootCoordinator" is better, since the fact that
> it uses containment is an implementation detail (this is why we have
> TabCoordinator vs. TabContainerViewController).

Naming collision:
https://cs.chromium.org/chromium/src/ios/chrome/browser/root_coordinator.h?dr...

https://codereview.chromium.org/2800313002/diff/140001/ios/clean/chrome/brows...
File ios/clean/chrome/browser/ui/root/root_container_coordinator.mm (right):

https://codereview.chromium.org/2800313002/diff/140001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_coordinator.mm:34: //
CommandDispatcher* dispatcher = self.browser->dispatcher();
On 2017/04/10 11:04:25, marq wrote:
> Don't leave code commented out. I agree that we want to move URL opening into
> the dispatcher, but that's not happening in this CL.

Oversight on my part. I was originally planning to tackle more in one CL, but
decided 
this is the right amount of changes for clarity. Now updated.

https://codereview.chromium.org/2800313002/diff/140001/ios/clean/chrome/brows...
File ios/clean/chrome/browser/ui/root/root_container_view_controller.mm (right):

https://codereview.chromium.org/2800313002/diff/140001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller.mm:41:
viewController.view.translatesAutoresizingMaskIntoConstraints = YES;
On 2017/04/10 11:04:25, marq wrote:
> This is the default, no need to set it.

Done.

https://codereview.chromium.org/2800313002/diff/140001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller.mm:59: -
(CGRect)rectForZoomWithKey:(NSObject*)key inView:(UIView*)view {
On 2017/04/10 11:04:25, marq wrote:
> No change for now, but I'd like to figure out a way to not have to forward
> protocols like this.

Could not agree more.

https://codereview.chromium.org/2800313002/diff/140001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller.mm:62: return
[reinterpret_cast<id<ZoomTransitionDelegate>>(
On 2017/04/10 11:04:25, marq wrote:
> Despite what I might have said in the past, this should be static_cast()

Done.

[marq (ping after 24h), 2017-04-11 14:15:51]

lgtm

[edchin, 2017-04-11 14:17:05]

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

[commit-bot: I haz the power, 2017-04-11 14:17:21]

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

[lpromero, 2017-04-11 15:20:39]

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/app/s...
File ios/clean/chrome/app/steps/root_container_coordinator+application_step.mm
(right):

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/app/s...
ios/clean/chrome/app/steps/root_container_coordinator+application_step.mm:20:
class RootCoordinatorContainer : public base::SupportsUserData::Data {
The new name is close to this one. Should this one be renamed?

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/brows...
File ios/clean/chrome/browser/ui/root/root_container_view_controller.h (right):

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller.h:12: // View
controller that wholly contains a content view controller.
Does it make sense to add this view controller to Showcase?

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/brows...
File ios/clean/chrome/browser/ui/root/root_container_view_controller.mm (right):

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller.mm:49: if
(viewController.parentViewController != self)
Should this be a DCHECK?

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller.mm:61: return
[static_cast<id<ZoomTransitionDelegate>>(self.contentViewController)
Could you add a unit tests file for all new files?
Here you could test the public API simply by setting a contentViewController
(public property) and testing the ZoomTransitionDelegate protocol conformance
(public too).

[edchin, 2017-04-11 15:52:05]

Replies to comments. Will upload another round of changes with lpromero
suggestions.

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/app/s...
File ios/clean/chrome/app/steps/root_container_coordinator+application_step.mm
(right):

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/app/s...
ios/clean/chrome/app/steps/root_container_coordinator+application_step.mm:20:
class RootCoordinatorContainer : public base::SupportsUserData::Data {
On 2017/04/11 15:20:39, lpromero wrote:
> The new name is close to this one. Should this one be renamed?

The name RootContainerCoordinator should really be RootCoordinator. However,
there is a name
collision at the moment. I'll investigate this a little more and see what is
causing the collision.

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/brows...
File ios/clean/chrome/browser/ui/root/root_container_view_controller.h (right):

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller.h:12: // View
controller that wholly contains a content view controller.
On 2017/04/11 15:20:39, lpromero wrote:
> Does it make sense to add this view controller to Showcase?

Yes it does. Will do.

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/brows...
File ios/clean/chrome/browser/ui/root/root_container_view_controller.mm (right):

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller.mm:49: if
(viewController.parentViewController != self)
On 2017/04/11 15:20:39, lpromero wrote:
> Should this be a DCHECK?

Yes, good suggestion. Will do.

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller.mm:61: return
[static_cast<id<ZoomTransitionDelegate>>(self.contentViewController)
On 2017/04/11 15:20:39, lpromero wrote:
> Could you add a unit tests file for all new files?
> Here you could test the public API simply by setting a contentViewController
> (public property) and testing the ZoomTransitionDelegate protocol conformance
> (public too).

Will do.

[edchin, 2017-04-11 15:55:46]

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/app/s...
File ios/clean/chrome/app/steps/root_container_coordinator+application_step.mm
(right):

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/app/s...
ios/clean/chrome/app/steps/root_container_coordinator+application_step.mm:20:
class RootCoordinatorContainer : public base::SupportsUserData::Data {
On 2017/04/11 15:52:05, edchin wrote:
> On 2017/04/11 15:20:39, lpromero wrote:
> > The new name is close to this one. Should this one be renamed?
> 
> The name RootContainerCoordinator should really be RootCoordinator. However,
> there is a name
> collision at the moment. I'll investigate this a little more and see what is
> causing the collision.

I think my comment was confusing. 
If I changed the name ObjC class name to RootCoordinator, I think this c++
class name would be fine. But as it, it is confusing. 
I'll see if I can change the ObjC name first. 
Otherwise, I would have to change this name to
RootContainerCoordinatorContainer.
That would be unfortunate.

[commit-bot: I haz the power, 2017-04-11 16:25:33]

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

[commit-bot: I haz the power, 2017-04-11 16:25:34]

Dry run: This issue passed the CQ dry run.

[edchin, 2017-04-11 22:22:15]

Patchset #3 (id:180001) has been deleted

[edchin, 2017-04-11 22:22:28]

Patchset #3 (id:200001) has been deleted

[edchin, 2017-04-11 22:26:04]

Patchset #3 (id:220001) has been deleted

[edchin, 2017-04-11 22:57:33]

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

[commit-bot: I haz the power, 2017-04-11 22:58:32]

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

[edchin, 2017-04-11 23:13:12]

Description was changed from

==========
[ios] RootContainer.

This CL places a root container at the root of the view controller hierarchy, 
instead of the tab grid. The root container has a content view controller
that can be swapped between regular mode or incognito mode. 
This refactor is required to enable switching to incognito mode.

BUG=686770
==========

to

==========
[ios] RootCoordinator and view controller.

This CL places a root container at the root of the view controller hierarchy, 
instead of the tab grid. The root container has a content view controller
that can be swapped between regular mode or incognito mode. 
This refactor is required to enable switching to incognito mode.

BUG=686770
==========

[edchin, 2017-04-12 00:09:53]

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

[commit-bot: I haz the power, 2017-04-12 00:10:44]

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

[edchin, 2017-04-12 01:20:48]

Patchset #6 (id:300001) has been deleted

[edchin, 2017-04-12 01:20:58]

Patchset #5 (id:280001) has been deleted

[edchin, 2017-04-12 01:23:26]

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

[commit-bot: I haz the power, 2017-04-12 01:24:16]

Dry run: 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-12 01:32:56]

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

[commit-bot: I haz the power, 2017-04-12 01:32:57]

Dry run: Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[edchin, 2017-04-12 07:58:08]

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

[commit-bot: I haz the power, 2017-04-12 07:58:32]

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

[edchin, 2017-04-12 08:01:39]

Patchset #5 (id:320001) has been deleted

[edchin, 2017-04-12 08:01:49]

Patchset #5 (id:340001) has been deleted

[edchin, 2017-04-12 08:10:52]

Addressed all of lpromero@'s comments. PTAL.
 
- The coordinator is now properly named RootCoordinator. The was
a naming collision, but the old chrome class has been renamed to 
ChromeRootCoordinator. 
- Unittests added. One is a placeholder. Others have tests.
- Added to showcase, including egtest.

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/app/s...
File ios/clean/chrome/app/steps/root_container_coordinator+application_step.mm
(right):

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/app/s...
ios/clean/chrome/app/steps/root_container_coordinator+application_step.mm:20:
class RootCoordinatorContainer : public base::SupportsUserData::Data {
On 2017/04/11 15:20:39, lpromero wrote:
> The new name is close to this one. Should this one be renamed?

The new ObjC class name is now RootCoordinator. This C++ container object
name now seems fine to me, since it contains the RootCoordinator.

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/brows...
File ios/clean/chrome/browser/ui/root/root_container_view_controller.h (right):

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller.h:12: // View
controller that wholly contains a content view controller.
On 2017/04/11 15:20:39, lpromero wrote:
> Does it make sense to add this view controller to Showcase?

Done.

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/brows...
File ios/clean/chrome/browser/ui/root/root_container_view_controller.mm (right):

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller.mm:49: if
(viewController.parentViewController != self)
On 2017/04/11 15:20:39, lpromero wrote:
> Should this be a DCHECK?

Done.

https://codereview.chromium.org/2800313002/diff/160001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller.mm:61: return
[static_cast<id<ZoomTransitionDelegate>>(self.contentViewController)
On 2017/04/11 15:20:39, lpromero wrote:
> Could you add a unit tests file for all new files?
> Here you could test the public API simply by setting a contentViewController
> (public property) and testing the ZoomTransitionDelegate protocol conformance
> (public too).

Done.

[lpromero, 2017-04-12 08:52:27]

I am sorry to be a pain, but could the renaming of the old class be split in
another CL?

[commit-bot: I haz the power, 2017-04-12 08:57:16]

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

[commit-bot: I haz the power, 2017-04-12 08:57:17]

Dry run: This issue passed the CQ dry run.

[lpromero, 2017-04-12 08:58:49]

Thanks for the changes!

https://codereview.chromium.org/2800313002/diff/360001/ios/clean/chrome/app/s...
File ios/clean/chrome/app/steps/root_coordinator+application_step.h (right):

https://codereview.chromium.org/2800313002/diff/360001/ios/clean/chrome/app/s...
ios/clean/chrome/app/steps/root_coordinator+application_step.h:13: // Creates
the main window and makes it key, but doesn't make it visible yet.
Maybe this is an assumption that could be unit-tested?

https://codereview.chromium.org/2800313002/diff/360001/ios/clean/chrome/brows...
File ios/clean/chrome/browser/ui/root/root_container_view_controller_unittest.mm
(right):

https://codereview.chromium.org/2800313002/diff/360001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller_unittest.mm:34: 
Could you add a test for the switching of contentViewController? Making sure the
new one is added, and the old one removed?

https://codereview.chromium.org/2800313002/diff/360001/ios/clean/chrome/brows...
File ios/clean/chrome/browser/ui/root/root_coordinator_unittest.mm (right):

https://codereview.chromium.org/2800313002/diff/360001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_coordinator_unittest.mm:20: 
You could add a test that starting the coordinator installs a
contentViewController on its controller?

openURL: is harder to test though.

[lpromero, 2017-04-12 08:59:17]

And forgot the lgtm :)

[edchin, 2017-04-12 18:38:32]

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

[commit-bot: I haz the power, 2017-04-12 18:39:22]

Dry run: 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-12 19:44:38]

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

[commit-bot: I haz the power, 2017-04-12 19:44:39]

Dry run: This issue passed the CQ dry run.

[edchin, 2017-04-13 07:31:36]

Patchset #7 (id:400001) has been deleted

[edchin, 2017-04-13 07:31:50]

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

[commit-bot: I haz the power, 2017-04-13 07:32:39]

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

[edchin, 2017-04-13 07:39:15]

Several more unit tests were added. Coordinator unit tests can
be improved with a real browser state object in the future.
This will be landed soon.

https://codereview.chromium.org/2800313002/diff/360001/ios/clean/chrome/app/s...
File ios/clean/chrome/app/steps/root_coordinator+application_step.h (right):

https://codereview.chromium.org/2800313002/diff/360001/ios/clean/chrome/app/s...
ios/clean/chrome/app/steps/root_coordinator+application_step.h:13: // Creates
the main window and makes it key, but doesn't make it visible yet.
On 2017/04/12 08:58:49, lpromero wrote:
> Maybe this is an assumption that could be unit-tested?

Additional tests are added, but this whole assumption is not unit tested. 
We need a real browser state in order to test this better. I will update
with more tests once we have better examples of coordinator unittests.

https://codereview.chromium.org/2800313002/diff/360001/ios/clean/chrome/brows...
File ios/clean/chrome/browser/ui/root/root_container_view_controller_unittest.mm
(right):

https://codereview.chromium.org/2800313002/diff/360001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_container_view_controller_unittest.mm:34: 
On 2017/04/12 08:58:49, lpromero wrote:
> Could you add a test for the switching of contentViewController? Making sure
the
> new one is added, and the old one removed?

Done.

https://codereview.chromium.org/2800313002/diff/360001/ios/clean/chrome/brows...
File ios/clean/chrome/browser/ui/root/root_coordinator_unittest.mm (right):

https://codereview.chromium.org/2800313002/diff/360001/ios/clean/chrome/brows...
ios/clean/chrome/browser/ui/root/root_coordinator_unittest.mm:20: 
On 2017/04/12 08:58:49, lpromero wrote:
> You could add a test that starting the coordinator installs a
> contentViewController on its controller?
> 
> openURL: is harder to test though.

In order to test the coordinator's start method, 
we need a real browser state. I'll create more tests
once we have better examples of coordinator unittests.

[edchin, 2017-04-13 07:58:12]

Patchset #3 (id:240001) has been deleted

[edchin, 2017-04-13 07:58:30]

Patchset #3 (id:260001) has been deleted

[edchin, 2017-04-13 07:58:58]

Patchset #4 (id:380001) has been deleted

[edchin, 2017-04-13 08:00:07]

On 2017/04/12 08:52:27, lpromero wrote:
> I am sorry to be a pain, but could the renaming of the old class be split in
> another CL?

Per your suggestion, the old class was renamed in another CL which has 
already landed.

[commit-bot: I haz the power, 2017-04-13 08:58:51]

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

[commit-bot: I haz the power, 2017-04-13 08:58:53]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[edchin, 2017-04-13 14:59:42]

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

[commit-bot: I haz the power, 2017-04-13 15:00:14]

Dry run: 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-13 15:36:39]

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

[commit-bot: I haz the power, 2017-04-13 15:36:40]

Dry run: This issue passed the CQ dry run.

[edchin, 2017-04-13 15:39:17]

The CQ bit was checked by edchin@chromium.org

[edchin, 2017-04-13 15:39:18]

The patchset sent to the CQ was uploaded after l-g-t-m from marq@chromium.org,
lpromero@chromium.org
Link to the patchset: https://codereview.chromium.org/2800313002/#ps420001
(title: "More unittests and cleanup.")

[commit-bot: I haz the power, 2017-04-13 15:39:51]

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-13 15:47:52]

CQ is committing da patch.

Bot data: {"patchset_id": 420001, "attempt_start_ts": 1492097957906680,
"parent_rev": "7887732326e9a545585ece8147cd2fe88a080f01", "commit_rev":
"b1f381c92360d29bf4defc89bf4d70af86dd27fb"}

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

Description was changed from

==========
[ios] RootCoordinator and view controller.

This CL places a root container at the root of the view controller hierarchy, 
instead of the tab grid. The root container has a content view controller
that can be swapped between regular mode or incognito mode. 
This refactor is required to enable switching to incognito mode.

BUG=686770
==========

to

==========
[ios] RootCoordinator and view controller.

This CL places a root container at the root of the view controller hierarchy, 
instead of the tab grid. The root container has a content view controller
that can be swapped between regular mode or incognito mode. 
This refactor is required to enable switching to incognito mode.

BUG=686770

Review-Url: https://codereview.chromium.org/2800313002
Cr-Commit-Position: refs/heads/master@{#464420}
Committed:
https://chromium.googlesource.com/chromium/src/+/b1f381c92360d29bf4defc89bf4d...
==========

[commit-bot: I haz the power, 2017-04-13 15:48:50]

Committed patchset #4 (id:420001) as
https://chromium.googlesource.com/chromium/src/+/b1f381c92360d29bf4defc89bf4d...