[Clean] Add unittest target and BrowserCoordinator tests.

ios/clean/chrome/browser/browser_coordinator.mm

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // ======                        New Architecture                         =====
 // =         This code is only used in the new iOS Chrome architecture.       =
 // ============================================================================
 
 #import "base/logging.h"
 #import "ios/clean/chrome/browser/browser_coordinator+internal.h"
@@ skipping 45 matching lines @@
 
 @implementation BrowserCoordinator (Internal)
 // Concrete implementations must implement a |viewController| property.
 @dynamic viewController;
 
 - (NSSet*)children {
   return [self.childCoordinators copy];
 }
 
 - (void)addChildCoordinator:(BrowserCoordinator*)coordinator {
+  CHECK([self respondsToSelector:@selector(viewController)])
+      << "BrowserCoordinator implementations must provide a viewController "
+         "property.";
   [self.childCoordinators addObject:coordinator];
+  coordinator.parentCoordinator = self;
   coordinator.browserState = self.browserState;
-  coordinator.parentCoordinator = self;
   coordinator.rootViewController = self.viewController;
 }
 
 - (BrowserCoordinator*)overlayCoordinator {
   if (self.overlaying)
     return self;
   for (BrowserCoordinator* child in self.children) {
     BrowserCoordinator* overlay = child.overlayCoordinator;
     if (overlay)
       return overlay;
   }
   return nil;
 }
 
 - (void)addOverlayCoordinator:(BrowserCoordinator*)overlayCoordinator {
   // If this object has no children, then add |overlayCoordinator| as a child
   // and mark it as such.
   if ([self canAddOverlayCoordinator:overlayCoordinator]) {
     [self addChildCoordinator:overlayCoordinator];
     overlayCoordinator.overlaying = YES;
   } else if (self.childCoordinators.count == 1) {
     [[self.childCoordinators anyObject]
         addOverlayCoordinator:overlayCoordinator];
-  } else {
+  } else if (self.childCoordinators.count > 1) {
     CHECK(NO) << "Coordinators with multiple children must explicitly "
               << "handle -addOverlayCoordinator: or return NO to "
               << "-canAddOverlayCoordinator:";
   }
+  // If control reaches here, the terminal child of the coordinator hierarchy
+  // has returned NO to -canAddOverlayCoordinator, so no overlay can be added.
+  // This is by default a silent no-op.
 }
 
 - (void)removeOverlayCoordinator {
   BrowserCoordinator* overlay = self.overlayCoordinator;
   [overlay.parentCoordinator removeChildCoordinator:overlay];
+  overlay.overlaying = NO;
 }
 
 - (BOOL)canAddOverlayCoordinator:(BrowserCoordinator*)overlayCoordinator {
+  // By default, a hierarchy with an overlay can't add a new one.
+  // By default, coordinators with parents can't be added as overlays.
   // By default, coordinators with no other children can add an overlay.

[lpromero, 2017/01/11 13:29:25]

All this logic should be documented in a header or design doc? 

[marq (ping after 24h), 2017/01/11 14:06:25]

I'll update the header.

-  return self.childCoordinators.count == 0;
+  return self.overlayCoordinator == nil &&
+         overlayCoordinator.parentCoordinator == nil &&
+         self.childCoordinators.count == 0;
 }
 
 - (void)removeChildCoordinator:(BrowserCoordinator*)coordinator {
+  if (![self.childCoordinators containsObject:coordinator])
+    return;
   [self.childCoordinators removeObject:coordinator];
+  coordinator.parentCoordinator = nil;
 }
 
 @end