[ios clean] Use ToolsMenuConfiguration for Menu context.

ios/clean/chrome/browser/ui/toolbar/toolbar_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.
 
 #import "ios/clean/chrome/browser/ui/toolbar/toolbar_coordinator.h"
 
+#import "ios/chrome/browser/ui/tools_menu/tools_menu_configuration.h"

[sczs, 2017/04/12 00:59:26]

Realized I need to update the BUILD.gn file, but importing from
ios/chrome/browser... is OK, right? 

[marq (ping after 24h), 2017/04/12 11:17:07]

Currently the rules are:

/clean can import from /shared and /chrome, but the /chrome imports will need to
be cleaned up later, so they should only be done as part of the "hacking in"
process of refactoring.
/shared can't import from either /chrome or /clean
/chrome can import from /shared but not /clean

So ideally, if you can move the tools menu configuration files into shared/,
that would be better. (and in a separate CL).

[sczs, 2017/04/12 15:06:07]

Understood, will start working on moving that file! 

 #import "ios/clean/chrome/browser/ui/commands/tools_menu_commands.h"
 #import "ios/clean/chrome/browser/ui/omnibox/location_bar_coordinator.h"
 #import "ios/clean/chrome/browser/ui/toolbar/toolbar_mediator.h"
 #import "ios/clean/chrome/browser/ui/toolbar/toolbar_view_controller.h"
 #import "ios/clean/chrome/browser/ui/tools/tools_coordinator.h"
 #import "ios/shared/chrome/browser/ui/browser_list/browser.h"
 #import "ios/shared/chrome/browser/ui/commands/command_dispatcher.h"
 #import "ios/shared/chrome/browser/ui/coordinators/browser_coordinator+internal.h"
 #import "ios/web/public/navigation_manager.h"
 #include "ios/web/public/web_state/web_state.h"
@@ skipping 77 matching lines @@
                            completion:nil];
   }
 }
 
 #pragma mark - ToolsMenuCommands Implementation
 
 - (void)showToolsMenu {
   ToolsCoordinator* toolsCoordinator = [[ToolsCoordinator alloc] init];
   [self addChildCoordinator:toolsCoordinator];
   [toolsCoordinator start];
+  ToolsMenuConfiguration* menuConfiguration =
+      [[ToolsMenuConfiguration alloc] initWithDisplayView:nil];

[sczs, 2017/04/12 00:59:26]

Currently [[ToolsMenuConfiguration alloc] init] is disabled, I will refactor it
a little bit. 

+  menuConfiguration.inTabSwitcher = NO;
+  toolsCoordinator.toolsMenuConfiguration = menuConfiguration;

[sczs, 2017/04/12 00:59:26]

This is my main question, should the Coordinator initializing the ToolsMenu
Coordinator configure the ToolsMenuConfiguration? 
Or should we use the UI Model so the ToolsMenu Coordinator configures its own
ToolsMenuConfiguration object? 

[marq (ping after 24h), 2017/04/12 11:17:07]

Conceptually ToolsMenuConfiguration is model code, so ideally the
ToolsMenuMediator just creates the instance it needs and nobody else knows
anything about it.

[sczs, 2017/04/12 15:06:07]

The problem is that ToolsMenuConfiguration needs to know about the current
context of the UI so we can configure it accordingly.
e.g. It needs to know if we are on the TabGrid or in "navigation/toolbar" mode,
because we will never show the overflow controls on the TabGrid ToolsMenu, even
if we are on compact width. 

[marq (ping after 24h), 2017/04/14 10:01:51]

Ah, OK; I missed that. Then, yes the coordinator that starts the menu
coordinator should pass in a menu config to the menu coordinator, like you are
doing here.

I'd also prefer to see (in another CL is fine) ToolsMenuConfiguration subclasses
for the switcher/toolbar cases rather than a property with conditional logic in
the class. Any time we can extract conditions that won't change at runtime into
separate implementations we should do so (I think).

[sczs, 2017/04/14 22:21:07]

Ack. Will merge this as an initial POC and keep iterating on how the Config is
set up.

   self.toolsMenuCoordinator = toolsCoordinator;
 }
 
 - (void)closeToolsMenu {
   [self.toolsMenuCoordinator stop];
   [self removeChildCoordinator:self.toolsMenuCoordinator];
 }
 
 // PLACEHOLDER: These will move to an object that handles navigation.
 #pragma mark - NavigationCommands
@@ skipping 13 matching lines @@
 - (void)stopLoadingPage {
   self.webState->Stop();
 }
 
 - (void)reloadPage {
   self.webState->GetNavigationManager()->Reload(web::ReloadType::NORMAL,
                                                 false /*check_for_repost*/);
 }
 
 @end