MacViews: Fix failing Menu Runner Cocoa Tests.

ui/views/controls/menu/menu_runner_impl_cocoa.mm

 // Copyright 2014 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 "ui/views/controls/menu/menu_runner_impl_cocoa.h"
 
 #include "base/mac/sdk_forward_declarations.h"
 #import "ui/base/cocoa/menu_controller.h"
 #include "ui/base/models/menu_model.h"
 #include "ui/events/event_utils.h"
@@ skipping 75 matching lines @@
     int32_t run_types) {
   if ((run_types & kNativeRunTypes) != 0 &&
       (run_types & MenuRunner::IS_NESTED) == 0) {
     return new MenuRunnerImplCocoa(menu_model);
   }
 
   return new MenuRunnerImplAdapter(menu_model);
 }
 
 MenuRunnerImplCocoa::MenuRunnerImplCocoa(ui::MenuModel* menu)
-    : running_(false),
+    : in_run_menu_at_(false),
       delete_after_run_(false),
       closing_event_time_(base::TimeDelta()) {
   menu_controller_.reset(
       [[MenuController alloc] initWithModel:menu useWithPopUpButtonCell:NO]);
 }
 
 bool MenuRunnerImplCocoa::IsRunning() const {
-  return running_;
+  return [menu_controller_ isMenuOpen];

[tapted, 2016/05/06 00:37:56]

I think this is different to what menu_runner_impl.cc does, but the codepaths
are complicated :/. Do you think the tests need to be updated instead?

E.g. IsRunning() says "Returns true if we're in a nested message loop running
the menu.". Can you check whether the non-native menu controlled by
menu_runner_impl.cc returns false immediately after a cancel? We should try to
be consistent with that unless other stuff breaks outside of tests.

[karandeepb, 2016/05/25 04:19:33]

Not sure if my understanding is correct, but on Mac, calling Cancel() would call

[menu_ cancelTracking] which should end the nested run loop spun by Cocoa. So
this implementation is consistent with the comments on IsRunning().

As far as consistency with the implementation on other platforms is concerned,
calling
MenuController::Cancel() calls delegate_->OnMenuClosed(), which would make
IsRunning() return false immediately. Example see -
https://code.google.com/p/chromium/codesearch#chromium/src/ui/views/controls/....
But this is only true for async and non
blocking runs(MenuRunner::FOR_DROP). For a sync menu, calling Cancel() 
would cause the run loop termination, hence causing IsRunning() to return false,
but only after the last run loop iteration completes. (like our current
implementation). Hence IsRunning() would not return false immediately after a
Cancel() for this case.

[tapted, 2016/05/25 05:07:19]

yup - but with this change Cocoa *would* return false for this case (right?)

The problem is when code *inside* the nested runloop calls Cancel() immediately
followed something else. Like you say - Cancel() doesn't immediately abort the
nested runloop - it just marks it to stop iterating. In
https://codereview.chromium.org/1876013002/ the problem was code that did
something like

HandleMenuActionFoo() {
  ..
  MenuRunner::Cancel();
  MenuRunner::Release();
}

MenuRunnerCocoaTest.DestroyAfterCanceling was added to cover this.

We need to compare code that does

HandleMenuActionFoo() {
  ..
  MenuRunner::Cancel();
  if (MenuRunner::IsRunning()) {
    // something.
  }
}

This should behave the same regardless of whether
MenuRunnerImplInterface::Create(..) returns a `new MenuRunnerImplCocoa` or a
`new MenuRunnerImplAdapter`.

I suspect that with this change, MenuRunner::IsRunning() will return false for
Cocoa, but true for the Adapter.

Perhaps we can add a new test to cover this -- menu_runner_cocoa_unittest.mm
doesn't currently try to perform tests using a MenuRunnerImpl

 }
 
 void MenuRunnerImplCocoa::Release() {
-  if (IsRunning()) {
+  if (in_run_menu_at_) {
     if (delete_after_run_)
       return;  // We already canceled.
 
     delete_after_run_ = true;
     [menu_controller_ cancel];
   } else {
     delete this;
   }
 }
 
 MenuRunner::RunResult MenuRunnerImplCocoa::RunMenuAt(Widget* parent,
                                                      MenuButton* button,
                                                      const gfx::Rect& bounds,
                                                      MenuAnchorPosition anchor,
                                                      int32_t run_types) {
+  in_run_menu_at_ = true;
   DCHECK(run_types & kNativeRunTypes);
   DCHECK(!IsRunning());
   DCHECK(parent);
   closing_event_time_ = base::TimeDelta();
-  running_ = true;
 
   if (run_types & MenuRunner::CONTEXT_MENU) {
     [NSMenu popUpContextMenu:[menu_controller_ menu]
                    withEvent:[NSApp currentEvent]
                      forView:parent->GetNativeView()];
   } else if (run_types & MenuRunner::COMBOBOX) {
     NSMenuItem* checked_item = FirstCheckedItem(menu_controller_);
     base::scoped_nsobject<NSView> anchor_view(
         CreateMenuAnchorView(parent->GetNativeWindow(), bounds, checked_item));
     NSMenu* menu = [menu_controller_ menu];
     [menu setMinimumWidth:bounds.width() + kNativeCheckmarkWidth];
     [menu popUpMenuPositioningItem:checked_item
                         atLocation:NSZeroPoint
                             inView:anchor_view];
     [anchor_view removeFromSuperview];
   } else {
     NOTREACHED();
   }
 
   closing_event_time_ = ui::EventTimeForNow();
-  running_ = false;
+  in_run_menu_at_ = false;
 
   if (delete_after_run_) {
     delete this;
     return MenuRunner::MENU_DELETED;
   }
 
   return MenuRunner::NORMAL_EXIT;
 }
 
 void MenuRunnerImplCocoa::Cancel() {
   [menu_controller_ cancel];
 }
 
 base::TimeDelta MenuRunnerImplCocoa::GetClosingEventTime() const {
   return closing_event_time_;
 }
 
 MenuRunnerImplCocoa::~MenuRunnerImplCocoa() {
 }
 
 }  // namespace internal
 }  // namespace views