[Mac] Don't close the Wrench menu after using the zoom buttons if the menu was opened sticky.

This runs the Task that updates the zoom information in the UI thread outside of the
normal MessageLoop and instead runs it on the native run loop.

R=mark
BUG=48679
TEST=Open Wrench menu with a click. Use zoom buttons. Menu stays open and page zooms. Other buttons still close menu.
TEST=Open Wrench menu with click-hold-drag. Hover and release on zoom buttons. Menu closes and page zooms.

Review URL: http://codereview.chromium.org/3183013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@56566 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 200 insertions, 10 deletions

diff --git a/chrome/browser/cocoa/menu_tracked_button.h b/chrome/browser/cocoa/menu_tracked_button.h
index 00dc78a..84ed154 100644
--- a/chrome/browser/cocoa/menu_tracked_button.h
+++ b/chrome/browser/cocoa/menu_tracked_button.h
@@ -36,6 +36,8 @@
   NSTrackingRectTag trackingTag_;
 }
 
+@property (nonatomic, readonly, getter=isTracking) BOOL tracking;
+
 @end
 
 #endif  // CHROME_BROWSER_COCOA_MENU_TRACKED_BUTTON_H_
diff --git a/chrome/browser/cocoa/menu_tracked_button.mm b/chrome/browser/cocoa/menu_tracked_button.mm
index aabef67..247417d 100644
--- a/chrome/browser/cocoa/menu_tracked_button.mm
+++ b/chrome/browser/cocoa/menu_tracked_button.mm
@@ -15,6 +15,8 @@
 
 @implementation MenuTrackedButton
 
+@synthesize tracking = tracking_;
+
 - (void)updateTrackingAreas {
   [super updateTrackingAreas];
   [self removeTrackingRect:trackingTag_];
@@ -71,6 +73,7 @@
     return [super mouseUp:theEvent];
   }
   [self performClick:self];
+  tracking_ = NO;
 }
 
 - (void)doHighlight:(BOOL)highlight {
diff --git a/chrome/browser/cocoa/wrench_menu_controller.h b/chrome/browser/cocoa/wrench_menu_controller.h
index ba5051c..99f3743 100644
--- a/chrome/browser/cocoa/wrench_menu_controller.h
+++ b/chrome/browser/cocoa/wrench_menu_controller.h
@@ -9,12 +9,17 @@
 #import <Cocoa/Cocoa.h>
 
 #import "base/cocoa_protocols_mac.h"
+#include "base/scoped_ptr.h"
 #import "chrome/browser/cocoa/menu_controller.h"
 
 @class MenuTrackedRootView;
 @class ToolbarController;
 class WrenchMenuModel;
 
+namespace WrenchMenuControllerInternal {
+class ZoomLevelObserver;
+}  // namespace WrenchMenuControllerInternal
+
 // The Wrench menu has a creative layout, with buttons in menu items. There is
 // a cross-platform model for this special menu, but on the Mac it's easier to
 // get spacing and alignment precisely right using a NIB. To do that, we
@@ -34,6 +39,8 @@ class WrenchMenuModel;
   IBOutlet NSButton* zoomDisplay_;
   IBOutlet NSButton* zoomMinus_;
   IBOutlet NSButton* zoomFullScreen_;
+
+  scoped_ptr<WrenchMenuControllerInternal::ZoomLevelObserver> observer_;
 }
 
 // Designated initializer; called within the NIB.
diff --git a/chrome/browser/cocoa/wrench_menu_controller.mm b/chrome/browser/cocoa/wrench_menu_controller.mm
index cb0059a..4966157 100644
--- a/chrome/browser/cocoa/wrench_menu_controller.mm
+++ b/chrome/browser/cocoa/wrench_menu_controller.mm
@@ -13,18 +13,53 @@
 #import "chrome/browser/cocoa/menu_tracked_root_view.h"
 #import "chrome/browser/cocoa/toolbar_controller.h"
 #include "chrome/browser/wrench_menu_model.h"
+#include "chrome/common/notification_observer.h"
+#include "chrome/common/notification_service.h"
+#include "chrome/common/notification_source.h"
+#include "chrome/common/notification_type.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 
 @interface WrenchMenuController (Private)
 - (void)adjustPositioning;
 - (void)performCommandDispatch:(NSNumber*)tag;
+- (NSButton*)zoomDisplay;
 @end
 
+namespace WrenchMenuControllerInternal {
+
+class ZoomLevelObserver : public NotificationObserver {
+ public:
+  explicit ZoomLevelObserver(WrenchMenuController* controller)
+      : controller_(controller) {
+    registrar_.Add(this, NotificationType::ZOOM_LEVEL_CHANGED,
+                   NotificationService::AllSources());
+  }
+
+  void Observe(NotificationType type,
+               const NotificationSource& source,
+               const NotificationDetails& details) {
+    DCHECK_EQ(type.value, NotificationType::ZOOM_LEVEL_CHANGED);
+    WrenchMenuModel* wrenchMenuModel = [controller_ wrenchMenuModel];
+    wrenchMenuModel->UpdateZoomControls();
+    const string16 level =
+        wrenchMenuModel->GetLabelForCommandId(IDC_ZOOM_PERCENT_DISPLAY);
+    [[controller_ zoomDisplay] setTitle:SysUTF16ToNSString(level)];
+  }
+
+ private:
+  NotificationRegistrar registrar_;
+  WrenchMenuController* controller_;  // Weak; owns this.
+};
+
+}  // namespace WrenchMenuControllerInternal
+
 @implementation WrenchMenuController
 
 - (id)init {
-  self = [super init];
+  if ((self = [super init])) {
+    observer_.reset(new WrenchMenuControllerInternal::ZoomLevelObserver(self));
+  }
   return self;
 }
 
@@ -98,12 +133,22 @@
 // NSCarbonMenuWindow; this screws up the typical |-commandDispatch:| system.
 - (IBAction)dispatchWrenchMenuCommand:(id)sender {
   NSInteger tag = [sender tag];
-  // The custom views within the Wrench menu are abnormal and keep the menu open
-  // after a target-action. Close the menu manually.
-  // TODO(rsesek): It'd be great if the zoom buttons didn't have to close the
-  // menu. See http://crbug.com/48679 for more info.
-  [menu_ cancelTracking];
-  [self dispatchCommandInternal:tag];
+  if (sender == zoomPlus_ || sender == zoomMinus_) {
+    // Do a direct dispatch rather than scheduling on the outermost run loop,
+    // which would not get hit until after the menu had closed.
+    [self performCommandDispatch:[NSNumber numberWithInt:tag]];
+
+    // The zoom buttons should not close the menu if opened sticky.
+    if ([sender respondsToSelector:@selector(isTracking)] &&
+        [sender performSelector:@selector(isTracking)]) {
+      [menu_ cancelTracking];
+    }
+  } else {
+    // The custom views within the Wrench menu are abnormal and keep the menu
+    // open after a target-action.  Close the menu manually.
+    [menu_ cancelTracking];
+    [self dispatchCommandInternal:tag];
+  }
 }
 
 - (void)dispatchCommandInternal:(NSInteger)tag {
@@ -197,4 +242,8 @@
   [[editCut_ superview] setFrame:parentFrame];
 }
 
+- (NSButton*)zoomDisplay {
+  return zoomDisplay_;
+}
+
 @end  // @implementation WrenchMenuController
diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc
index 87f4cff..500212c 100644
--- a/chrome/browser/renderer_host/resource_message_filter.cc
+++ b/chrome/browser/renderer_host/resource_message_filter.cc
@@ -54,6 +54,7 @@
 #include "chrome/browser/speech/speech_input_dispatcher_host.h"
 #include "chrome/browser/spellchecker_platform_engine.h"
 #include "chrome/browser/task_manager.h"
+#include "chrome/browser/ui_thread_helpers.h"
 #include "chrome/browser/worker_host/message_port_dispatcher.h"
 #include "chrome/browser/worker_host/worker_service.h"
 #include "chrome/common/child_process_host.h"
@@ -1030,7 +1031,7 @@ void ResourceMessageFilter::OnV8HeapStatsOnUIThread(
 
 void ResourceMessageFilter::OnDidZoomURL(const GURL& url,
                                          int zoom_level) {
-  ChromeThread::PostTask(ChromeThread::UI, FROM_HERE,
+  ui_thread_helpers::PostTaskWhileRunningMenu(FROM_HERE,
       NewRunnableMethod(this,
                         &ResourceMessageFilter::UpdateHostZoomLevelsOnUIThread,
                         url, zoom_level));
diff --git a/chrome/browser/ui_thread_helpers.h b/chrome/browser/ui_thread_helpers.h
new file mode 100644
index 0000000..90e2c4e
--- /dev/null
+++ b/chrome/browser/ui_thread_helpers.h
@@ -0,0 +1,32 @@
+// Copyright (c) 2010 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.
+
+#ifndef CHROME_BROWSER_UI_THREAD_HELPERS_H_
+#define CHROME_BROWSER_UI_THREAD_HELPERS_H_
+#pragma once
+
+class Task;
+
+namespace tracked_objects {
+class Location;
+}  // namespace tracked_objects
+
+namespace ui_thread_helpers {
+
+// This can be used in place of ChromeThread::PostTask(ChromeThread::UI, ...).
+// The purpose of this function is to be able to execute Chrome work alongside
+// native work when a message loop is running nested or, in the case of Mac,
+// in a different mode.  Currently this is used for updating the HostZoomMap
+// while the Wrench menu is open, allowing for the zoom display to update.  See
+// http://crbug.com/48679 for the full rationale.
+//
+// CAVEAT EMPTOR: This function's implementation is different across platforms
+// and may run the Task in a way that differs from the stock MessageLoop.  You
+// should check the behavior on all platforms if you use this.
+bool PostTaskWhileRunningMenu(const tracked_objects::Location& from_here,
+                              Task* task);
+
+}  // namespace ui_thread_helpers
+
+#endif  // CHROME_BROWSER_UI_THREAD_HELPERS_H_
diff --git a/chrome/browser/ui_thread_helpers_linux.cc b/chrome/browser/ui_thread_helpers_linux.cc
new file mode 100644
index 0000000..41721c3
--- /dev/null
+++ b/chrome/browser/ui_thread_helpers_linux.cc
@@ -0,0 +1,18 @@
+// Copyright (c) 2010 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.
+
+#include "chrome/browser/ui_thread_helpers.h"
+
+#include "base/task.h"
+#include "chrome/browser/chrome_thread.h"
+
+namespace ui_thread_helpers {
+
+bool PostTaskWhileRunningMenu(const tracked_objects::Location& from_here,
+                              Task* task) {
+  // TODO(rsesek): Implementing Linux behavior may fix http://crbug.com/48240
+  return ChromeThread::PostTask(ChromeThread::UI, from_here, task);
+}
+
+}  // namespace ui_thread_helpers
diff --git a/chrome/browser/ui_thread_helpers_mac.mm b/chrome/browser/ui_thread_helpers_mac.mm
new file mode 100644
index 0000000..c6efd3c
--- /dev/null
+++ b/chrome/browser/ui_thread_helpers_mac.mm
@@ -0,0 +1,59 @@
+// Copyright (c) 2010 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.
+
+#include "chrome/browser/ui_thread_helpers.h"
+
+#import <Cocoa/Cocoa.h>
+
+#include "base/scoped_ptr.h"
+#include "base/task.h"
+#include "chrome/browser/chrome_thread.h"
+
+// This is a wrapper for running Chrome Task objects from within a native run
+// loop.  A typical use case is when Chrome work needs to get done but the main
+// message loop is blocked by a nested run loop (like an event tracking one).
+// This can run specific tasks in that nested loop.  This owns the task and will
+// delete it and itself when done.
+@interface UITaskHelperMac : NSObject {
+ @private
+  scoped_ptr<Task> task_;
+}
+- (id)initWithTask:(Task*)task;
+- (void)runTask;
+@end
+
+@implementation UITaskHelperMac
+- (id)initWithTask:(Task*)task {
+  if ((self = [super init])) {
+    task_.reset(task);
+  }
+  return self;
+}
+
+- (void)runTask {
+  task_->Run();
+  [self autorelease];
+}
+@end
+
+namespace ui_thread_helpers {
+
+bool PostTaskWhileRunningMenu(const tracked_objects::Location& from_here,
+                              Task* task) {
+  // This deletes itself and the task after the task runs.
+  UITaskHelperMac* runner = [[UITaskHelperMac alloc] initWithTask:task];
+
+  // Schedule the selector in multiple modes in case this was called while a
+  // menu was not running.
+  NSArray* modes = [NSArray arrayWithObjects:NSEventTrackingRunLoopMode,
+                                             NSDefaultRunLoopMode,
+                                             nil];
+  [runner performSelectorOnMainThread:@selector(runTask)
+                           withObject:nil
+                        waitUntilDone:NO
+                                modes:modes];
+  return true;
+}
+
+}  // namespace ui_thread_helpers
diff --git a/chrome/browser/ui_thread_helpers_win.cc b/chrome/browser/ui_thread_helpers_win.cc
new file mode 100644
index 0000000..170ecdb
--- /dev/null
+++ b/chrome/browser/ui_thread_helpers_win.cc
@@ -0,0 +1,17 @@
+// Copyright (c) 2010 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.
+
+#include "chrome/browser/ui_thread_helpers.h"
+
+#include "base/task.h"
+#include "chrome/browser/chrome_thread.h"
+
+namespace ui_thread_helpers {
+
+bool PostTaskWhileRunningMenu(const tracked_objects::Location& from_here,
+                              Task* task) {
+  return ChromeThread::PostTask(ChromeThread::UI, from_here, task);
+}
+
+}  // namespace ui_thread_helpers
diff --git a/chrome/browser/wrench_menu_model.h b/chrome/browser/wrench_menu_model.h
index 911f333..5b92362 100644
--- a/chrome/browser/wrench_menu_model.h
+++ b/chrome/browser/wrench_menu_model.h
@@ -106,6 +106,9 @@ class WrenchMenuModel : public menus::SimpleMenuModel,
   // Getters.
   Browser* browser() const { return browser_; }
 
+  // Calculates |zoom_label_| in response to a zoom change.
+  void UpdateZoomControls();
+
  private:
   // Testing constructor used for mocking.
   friend class ::MockWrenchMenuModel;
@@ -118,8 +121,7 @@ class WrenchMenuModel : public menus::SimpleMenuModel,
   void CreateCutCopyPaste();
   void CreateZoomFullscreen();
 
-  // Calculates |zoom_label_| in response to a zoom change.
-  void UpdateZoomControls();
+  // Gets the current zoom information from the renderer.
   double GetZoom(bool* enable_increment, bool* enable_decrement);
 
   string16 GetSyncMenuLabel() const;