Add ContextMenuCoordinator.

ios/chrome/browser/ui/context_menu/context_menu_wrangler.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/chrome/browser/ui/context_menu/context_menu_wrangler.h"
+
+#include "base/ios/weak_nsobject.h"
+#include "base/strings/sys_string_conversions.h"
+#import "ios/web/public/web_state/context_menu_params.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/strings/grit/ui_strings.h"
+
+@interface ContextMenuWrangler () {
+  // Underlying system alert.
+  base::WeakNSObject<UIAlertController> _alertController;

[marq (ping after 24h), 2016/05/18 10:38:10]

Should this really be weak?

[michaeldo, 2016/05/18 15:22:04]

This should NOT be weak, since it may be deallocated after some buttons are
added to it, but before it is presented. This could cause some "lost buttons.

My mistake was that it is weak in the existing ContextMenuController class,
which is where much of this logic is modeled after.

+  // View controller which will be used to present the |_alertController|.
+  base::WeakNSObject<UIViewController> _presentingViewController;
+  // Parameters that define what is shown in the context menu.
+  web::ContextMenuParams _params;
+}
+// Redefined to readwrite.
+@property(nonatomic, readwrite, getter=isVisible) BOOL visible;
+// Lazy initializer to create the |_alert|.
+@property(nonatomic, readonly) UIAlertController* alertController;
+// Called when the alert is dismissed to perform cleanup.
+- (void)alertDismissed;
+@end
+
+@implementation ContextMenuWrangler
+@synthesize visible = _visible;

[marq (ping after 24h), 2016/05/18 10:38:10]

Doesn't this property end up just being YES if _alertController is nonnull and
NO otherwise? If so, do you need to track it in a separate variable?

[michaeldo, 2016/05/18 15:22:04]

It doesn't quite follow _alertController. This is set to yes when start is
called, and set to NO once dismissed. The exact use case where we use this
externally should be evaluated. Maybe at that location we can just check if a
ContextMenuCoordinator exists or not.

+
+- (instancetype)initWithViewController:(UIViewController*)viewController
+                                params:(const web::ContextMenuParams&)params {
+  self = [super init];
+  if (self) {
+    _params = params;
+    _presentingViewController.reset(viewController);
+  }
+  return self;
+}
+

[marq (ping after 24h), 2016/05/18 10:38:10]

#pragma mark - Object lifecycle

[michaeldo, 2016/05/18 15:22:05]

Done.

+- (void)dealloc {
+  [_alertController dismissViewControllerAnimated:NO completion:nil];

[marq (ping after 24h), 2016/05/18 10:38:10]

I'd prefer to have this call [self stop] instead of duplicating code.

[michaeldo, 2016/05/18 15:22:04]

Done.

+  [super dealloc];
+}
+
+- (void)addItemWithTitle:(NSString*)title action:(ProceduralBlock)actionBlock {

[marq (ping after 24h), 2016/05/18 10:38:09]

#pragma mark - Public methods

[michaeldo, 2016/05/18 15:22:05]

Done.

+  base::WeakNSObject<ContextMenuWrangler> weakSelf(self);
+  void (^actionHandler)(UIAlertAction*) = ^(UIAlertAction*) {
+    [weakSelf alertDismissed];
+    actionBlock();
+  };
+  [self.alertController
+      addAction:[UIAlertAction actionWithTitle:title
+                                         style:UIAlertActionStyleDefault
+                                       handler:actionHandler]];
+}
+
+- (void)start {
+  // Check that the view is still visible on screen, otherwise just return and
+  // don't show the context menu.
+  if (![_params.view window] &&
+      ![_params.view isKindOfClass:[UIWindow class]]) {
+    _alertController.reset();

[marq (ping after 24h), 2016/05/18 10:38:10]

This case destroys all of the config information created by calls to
-addItemWithTitle. That should probably be documented somewhere.

[michaeldo, 2016/05/18 15:22:04]

I agree, but maybe it isn't really necessary to destroy it? That was the prior
logic, but I'd prefer to simply early return.

+    return;
+  }
+
+  [_presentingViewController presentViewController:_alertController

[marq (ping after 24h), 2016/05/18 10:38:09]

_alertController or self.alertController?

[michaeldo, 2016/05/18 15:22:04]

I changed this to self.alertController. I think it is better, although if it
hasn't been created until it is presented, it will only have a cancel button.

+                                          animated:YES
+                                        completion:nil];
+  self.visible = YES;
+}
+
+- (void)stop {
+  [_alertController dismissViewControllerAnimated:NO completion:nil];
+  [self setVisible:NO];
+  _alertController.reset();
+}
+
+- (UIAlertController*)alertController {

[marq (ping after 24h), 2016/05/18 10:38:10]

#pragma mark - Property implementation

[michaeldo, 2016/05/18 15:22:04]

Done.

+  if (!_alertController) {
+    DCHECK([_params.view isDescendantOfView:[_presentingViewController view]]);
+    UIAlertController* alert = [UIAlertController
+        alertControllerWithTitle:_params.menu_title
+                         message:nil
+                  preferredStyle:UIAlertControllerStyleActionSheet];
+    alert.popoverPresentationController.sourceView = _params.view;
+    alert.popoverPresentationController.sourceRect =
+        CGRectMake(_params.location.x, _params.location.y, 1.0, 1.0);
+
+    base::WeakNSObject<ContextMenuWrangler> weakSelf(self);
+    UIAlertAction* cancelAction =
+        [UIAlertAction actionWithTitle:l10n_util::GetNSString(IDS_APP_CANCEL)
+                                 style:UIAlertActionStyleCancel
+                               handler:^(UIAlertAction*) {
+                                 [weakSelf alertDismissed];
+                               }];
+    [alert addAction:cancelAction];
+
+    _alertController.reset([alert retain]);
+  }
+  return _alertController;
+}
+
+- (void)alertDismissed {

[marq (ping after 24h), 2016/05/18 10:38:10]

#pragma mark - Private methods

[michaeldo, 2016/05/18 15:22:04]

Done.

+  [self setVisible:NO];

[marq (ping after 24h), 2016/05/18 10:38:09]

Please be consistent with how you assign to this property; I'd prefer dot
notation everywhere. (self.visible = NO).

[michaeldo, 2016/05/18 15:22:04]

agreed! Done.

+  _alertController.reset();
+}
+
+@end