RenderWidget popup should be a NSWindow so it can go outside the main window.

content/browser/renderer_host/render_widget_host_view_mac.mm

 // Copyright (c) 2012 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 "content/browser/renderer_host/render_widget_host_view_mac.h"
 
 #include <QuartzCore/QuartzCore.h>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 22 matching lines @@
 #import "content/browser/renderer_host/render_widget_host_view_mac_editcommand_helper.h"
 #import "content/browser/renderer_host/text_input_client_mac.h"
 #include "content/common/accessibility_messages.h"
 #include "content/common/edit_command.h"
 #include "content/common/gpu/gpu_messages.h"
 #include "content/common/plugin_messages.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/native_web_keyboard_event.h"
 #import "content/public/browser/render_widget_host_view_mac_delegate.h"
+#include "content/public/browser/web_contents_view_delegate.h"
 #include "skia/ext/platform_canvas.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebInputEvent.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebScreenInfo.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/mac/WebInputEventFactory.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/mac/WebScreenInfoFactory.h"
 #import "third_party/mozilla/ComplexTextInputPanel.h"
 #include "third_party/skia/include/core/SkColor.h"
+#import "ui/base/cocoa/event_hook_application.h"
 #import "ui/base/cocoa/fullscreen_window_manager.h"
 #import "ui/base/cocoa/underlay_opengl_hosting_window.h"
 #include "ui/base/keycodes/keyboard_codes.h"
 #include "ui/base/layout.h"
 #include "ui/gfx/point.h"
 #include "ui/gfx/rect_conversions.h"
 #include "ui/gfx/size_conversions.h"
 #include "ui/gfx/scoped_ns_graphics_context_save_gstate_mac.h"
+#include "ui/gfx/native_widget_types.h"

[Avi (use Gerrit), 2012/12/12 15:59:49]

in alphabetical order

[keishi, 2012/12/13 07:51:12]

Didn't need this. Removed.

 #include "ui/surface/io_surface_support_mac.h"
 #include "webkit/plugins/npapi/webplugin.h"
 
 using content::BackingStoreMac;
 using content::BrowserAccessibility;
 using content::BrowserAccessibilityManager;
 using content::EditCommand;
 using content::NativeWebKeyboardEvent;
 using content::RenderViewHostImpl;
 using content::RenderWidgetHostImpl;
@@ skipping 92 matching lines @@
 - (BOOL)canBecomeKeyWindow {
   return YES;
 }
 
 - (BOOL)canBecomeMainWindow {
   return YES;
 }
 
 @end
 
+@interface RenderWidgetPopupWindow : NSWindow<CrEventHookProtocol>
+@end
+
+@implementation RenderWidgetPopupWindow
+
+- (id)initWithContentRect:(NSRect)contentRect
+                styleMask:(NSUInteger)windowStyle
+                  backing:(NSBackingStoreType)bufferingType
+                    defer:(BOOL)deferCreation {
+  if (self = [super initWithContentRect:contentRect
+                              styleMask:windowStyle
+                                backing:bufferingType
+                                  defer:deferCreation]) {
+    [self setOpaque:NO];
+    [self setBackgroundColor:[NSColor clearColor]];
+    [self startObservingClick];
+  }
+  return self;
+}
+
+- (void)close {
+  [self stopObservingClick];
+  [super close];
+}
+
+// Gets called for all events in application. Watching for a click outside the
+// window so we can close.
+- (void)hookForEvent:(NSEvent*)theEvent {
+  if ([theEvent window] == self)
+    return;
+  NSEventType eventType = [theEvent type];
+  if (eventType == NSLeftMouseDown || eventType == NSRightMouseDown) {
+    [self close];
+  }

[Avi (use Gerrit), 2012/12/12 15:59:49]

You don't need {} for a one-liner.

I'm still curious about this. For a popup, the view has closeOnDeactivate_ ==
YES, which in its -resignFirstResponder: should cause it to kill itself on
deactivation. Why is this event hooking necessary?

In addition, what seems to me to be necessary (and what isn't here) is handling
scroll events outside the window. Currently, scroll events in the parent RWHV
kill all child RWHVs. Since we're using child windows, this no longer happens.
See my comment below.

[keishi, 2012/12/13 07:51:12]

resignFirstResponder isn't enough. For example it isn't called when dragging the
window title bar(because it doesn't become the first responder). It is also not
called when clicking on windows for other applications.

+}
+
+// Gets called when the menubar is clicked.
+// Needed because the hookForEvent method doesn't see the click on the menubar.
+- (void)begunTracking:(NSNotification *)notification {

[Avi (use Gerrit), 2012/12/12 15:59:49]

No space before *.

[keishi, 2012/12/13 07:51:12]

Done.

+  [self close];
+}
+
+// Install the callback.
+- (void)startObservingClick {

[Avi (use Gerrit), 2012/12/12 15:59:49]

-startObservingClicks is more grammatical. (And -stopObservingClicks too.)

[keishi, 2012/12/13 07:51:12]

Done.

+  CrEventHookApplication* app = static_cast<CrEventHookApplication*>(
+      [CrEventHookApplication sharedApplication]);
+  [app addEventHook:self];
+
+  NSNotificationCenter *nc = [NSNotificationCenter defaultCenter];

[Avi (use Gerrit), 2012/12/12 15:59:49]

Space after, not before *.

[keishi, 2012/12/13 07:51:12]

Done.

+  [nc addObserver:self
+         selector:@selector(begunTracking:)
+             name:NSMenuDidBeginTrackingNotification
+           object:[NSApp mainMenu]];
+}
+
+// Remove the callback.
+- (void)stopObservingClick {
+  CrEventHookApplication* app = static_cast<CrEventHookApplication*>(
+      [CrEventHookApplication sharedApplication]);
+  [app removeEventHook:self];
+
+  NSNotificationCenter *nc = [NSNotificationCenter defaultCenter];
+  [nc removeObserver:self
+                name:NSMenuDidBeginTrackingNotification
+              object:[NSApp mainMenu]];
+}
+
+@end
+
 namespace {
 
 // Maximum number of characters we allow in a tooltip.
 const size_t kMaxTooltipLength = 1024;
 
 // TODO(suzhe): Upstream this function.
 WebKit::WebColor WebColorFromNSColor(NSColor *color) {
   CGFloat r, g, b, a;
   [color getRed:&r green:&g blue:&b alpha:&a];
 
@@ skipping 142 matching lines @@
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // RenderWidgetHostViewMac, RenderWidgetHostView implementation:
 
 void RenderWidgetHostViewMac::InitAsChild(
     gfx::NativeView parent_view) {
 }
 
 void RenderWidgetHostViewMac::InitAsPopup(
-    RenderWidgetHostView* parent_host_view,
-    const gfx::Rect& pos) {
+    RenderWidgetHostView* parent_host_view, const gfx::Rect& pos) {
   bool activatable = popup_type_ == WebKit::WebPopupTypeNone;
   [cocoa_view_ setCloseOnDeactivate:YES];
   [cocoa_view_ setCanBeKeyView:activatable ? YES : NO];
   [parent_host_view->GetNativeView() addSubview:cocoa_view_];

[Avi (use Gerrit), 2012/12/12 15:59:49]

This line doesn't make sense any more. If we're installing cocoa_view_ in its
own window, why do we make it a subview of the parent only to immediately remove
it below?

[keishi, 2012/12/13 07:51:12]

Removed.

 
   NSPoint origin_global = NSPointFromCGPoint(pos.origin().ToCGPoint());
   if ([[NSScreen screens] count] > 0) {
     origin_global.y = [[[NSScreen screens] objectAtIndex:0] frame].size.height -
         pos.height() - origin_global.y;
   }
-  NSPoint origin_window =
-      [[cocoa_view_ window] convertScreenToBase:origin_global];
-  NSPoint origin_view =
-      [cocoa_view_ convertPoint:origin_window fromView:nil];
-  NSRect initial_frame = NSMakeRect(origin_view.x,
-                                    origin_view.y,
-                                    pos.width(),
-                                    pos.height());
-  [cocoa_view_ setFrame:initial_frame];
+
+  popup_window_.reset([[RenderWidgetPopupWindow alloc]
+      initWithContentRect:NSMakeRect(origin_global.x, origin_global.y,
+                                     pos.width(), pos.height())
+                styleMask:NSBorderlessWindowMask
+                  backing:NSBackingStoreBuffered
+                    defer:NO]);
+  [popup_window_ setLevel:NSPopUpMenuWindowLevel];
+  [popup_window_ setReleasedWhenClosed:NO];
+  [popup_window_ makeKeyAndOrderFront:nil];
+  [[popup_window_ contentView] addSubview:cocoa_view_];
+  [cocoa_view_ setFrame:[[popup_window_ contentView] bounds]];
+  [cocoa_view_ setAutoresizingMask:NSViewWidthSizable | NSViewHeightSizable];
+  [[NSNotificationCenter defaultCenter]
+      addObserver:cocoa_view_
+         selector:@selector(popupWindowWillClose:)
+             name:NSWindowWillCloseNotification
+           object:popup_window_];

[Avi (use Gerrit), 2012/12/12 15:59:49]

What is the case that you are trying to catch here? The window is borderless, so
you should be the only one closing it.

[keishi, 2012/12/13 07:51:12]

When a click is observed RenderWidgetPopupWindow will close itself. cocoa_view_
will get notified of this and kill the RenderWidgetHostViewMac.

 }
 
 // This function creates the fullscreen window and hides the dock and menubar if
 // necessary. Note, this codepath is only used for pepper flash when
 // pp::FlashFullScreen::SetFullscreen() is called. If
 // pp::FullScreen::SetFullscreen() is called then the entire browser window
 // will enter fullscreen instead.
 void RenderWidgetHostViewMac::InitAsFullscreen(
     RenderWidgetHostView* reference_host_view) {
   fullscreen_parent_host_view_ =
@@ skipping 88 matching lines @@
   if (rect.size().IsEmpty())
     return;
 
   // Ignore the position of |rect| for non-popup rwhvs. This is because
   // background tabs do not have a window, but the window is required for the
   // coordinate conversions. Popups are always for a visible tab.
   if (IsPopup()) {
     // The position of |rect| is screen coordinate system and we have to
     // consider Cocoa coordinate system is upside-down and also multi-screen.
     NSPoint origin_global = NSPointFromCGPoint(rect.origin().ToCGPoint());
+    NSSize size = NSMakeSize(rect.width(), rect.height());
+    size = [cocoa_view_ convertSize:size toView:nil];
     if ([[NSScreen screens] count] > 0) {
-      NSSize size = NSMakeSize(rect.width(), rect.height());
-      size = [cocoa_view_ convertSize:size toView:nil];
       NSScreen* screen =
           static_cast<NSScreen*>([[NSScreen screens] objectAtIndex:0]);
       origin_global.y =
           NSHeight([screen frame]) - size.height - origin_global.y;
     }
-
-    // Then |origin_global| is converted to client coordinate system.
-    DCHECK([cocoa_view_ window]);
-    NSPoint origin_window =
-        [[cocoa_view_ window] convertScreenToBase:origin_global];
-    NSPoint origin_view =
-        [[cocoa_view_ superview] convertPoint:origin_window fromView:nil];
-    NSRect frame = NSMakeRect(origin_view.x, origin_view.y,
-                              rect.width(), rect.height());
-    [cocoa_view_ setFrame:frame];
+    [popup_window_ setFrame:NSMakeRect(origin_global.x, origin_global.y,
+                                       size.width, size.height)
+                    display:YES];
   } else {
     DCHECK([[cocoa_view_ superview] isKindOfClass:[BaseView class]]);
     BaseView* superview = static_cast<BaseView*>([cocoa_view_ superview]);
     gfx::Rect rect2 = [superview flipNSRectToRect:[cocoa_view_ frame]];
     rect2.set_width(rect.width());
     rect2.set_height(rect.height());
     [cocoa_view_ setFrame:[superview flipRectToNSRect:rect2]];
   }
 }
 
@@ skipping 229 matching lines @@
   for (NSView* subview in [cocoa_view_ subviews]) {
     if ([subview isKindOfClass:[RenderWidgetHostViewCocoa class]]) {
       [static_cast<RenderWidgetHostViewCocoa*>(subview)
           renderWidgetHostViewMac]->ShutdownHost();
     } else if ([subview isKindOfClass:[AcceleratedPluginView class]]) {
       [static_cast<AcceleratedPluginView*>(subview)
           onRenderWidgetHostViewGone];
     }
   }
 
+  [[NSNotificationCenter defaultCenter]
+      removeObserver:cocoa_view_
+                name:NSWindowWillCloseNotification
+              object:popup_window_];
+
   // We've been told to destroy.
   [cocoa_view_ retain];
   [cocoa_view_ removeFromSuperview];
   [cocoa_view_ autorelease];
 
+  [popup_window_ close];
+  popup_window_.autorelease();
+
   [fullscreen_window_manager_ exitFullscreenMode];
   fullscreen_window_manager_.reset();
   [pepper_fullscreen_window_ close];
 
   // This can be called as part of processing the window's responder
   // chain, for instance |-performKeyEquivalent:|.  In that case the
   // object needs to survive until the stack unwinds.
   pepper_fullscreen_window_.autorelease();
 
   // We get this call just before |render_widget_host_| deletes
@@ skipping 1354 matching lines @@
   }
 
   // Possibly autohide the cursor.
   if ([RenderWidgetHostViewCocoa shouldAutohideCursorForEvent:theEvent])
     [NSCursor setHiddenUntilMouseMoves:YES];
 }
 
 - (void)scrollWheel:(NSEvent*)theEvent {
   // Cancel popups before calling the delegate because even if the delegate eats
   // the event, it's still an explicit user action outside the popup.
   [self cancelChildPopups];

[Avi (use Gerrit), 2012/12/12 15:59:49]

How does this code work?

As I noted above, it used to be the job of the root RWHV, when receiving a
scroll event, to kill any children. This code won't function now, because child
RWHVs are now in their own windows, so in -cancelChildPopups nothing will be
found.

If this code is useless, remove it.

[keishi, 2012/12/13 07:51:12]

You're right. We can't do this if we use NSWindow. It turns out we want to close
popups on mouse when event for all platforms so I will change
WebKit::WebViewImpl to do it.

+bool WebViewImpl::handleMouseWheel(Frame& mainFrame, const WebMouseWheelEvent&
event)
+{
+    hidePopups();
+    return PageWidgetEventHandler::handleMouseWheel(mainFrame, event);
+}
+

 
   if (delegate_ && [delegate_ respondsToSelector:@selector(handleEvent:)]) {
     BOOL handled = [delegate_ handleEvent:theEvent];
     if (handled)
       return;
   }
 
   const WebMouseWheelEvent& event =
       WebInputEventFactory::mouseWheelEvent(theEvent, self);
   if (renderWidgetHostView_->render_widget_host_)
@@ skipping 1228 matching lines @@
 }
 
 - (void)updateCursor:(NSCursor*)cursor {
   if (currentCursor_ == cursor)
     return;
 
   currentCursor_.reset(cursor, base::scoped_policy::RETAIN);
   [[self window] invalidateCursorRectsForView:self];
 }
 
+- (void)popupWindowWillClose:(NSNotification *)notification {
+  [self resignFirstResponder];

[keishi, 2012/12/13 07:51:12]

Changing this line because cocoa_view_ for a popup doesn't become first
responder and we don't have text fields in popups.

+}
+
 @end
 
 //
 // Supporting application services
 //
 @implementation RenderWidgetHostViewCocoa(NSServicesRequests)
 
 - (BOOL)writeSelectionToPasteboard:(NSPasteboard*)pboard
                              types:(NSArray*)types {
   const std::string& str = renderWidgetHostView_->selected_text();
@@ skipping 11 matching lines @@
   if (!string) return NO;
 
   // If the user is currently using an IME, confirm the IME input,
   // and then insert the text from the service, the same as TextEdit and Safari.
   [self confirmComposition];
   [self insertText:string];
   return YES;
 }
 
 @end