Consider the popup window position when the window shows upward. This patch depends on WebKit patch.

chrome/browser/renderer_host/render_widget_host_view_mac.mm

 // 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 <QuartzCore/QuartzCore.h>
 
 #include "chrome/browser/renderer_host/render_widget_host_view_mac.h"
 
 #include "app/app_switches.h"
 #include "app/surface/io_surface_support_mac.h"
@@ skipping 648 matching lines @@
   // ignore them so we don't re-allocate the backing store.  We will paint
   // everything again when we become selected again.
   is_hidden_ = true;
 
   // If we have a renderer, then inform it that we are being hidden so it can
   // reduce its resource utilization.
   render_widget_host_->WasHidden();
 }
 
 void RenderWidgetHostViewMac::SetSize(const gfx::Size& size) {
+  gfx::Rect rect = GetViewBounds();
+  rect.set_size(size);
+  SetBounds(rect);
+}
+
+void RenderWidgetHostViewMac::SetBounds(const gfx::Rect& rect) {
   if (is_hidden_)
     return;
 
   // During the initial creation of the RenderWidgetHostView in
   // TabContents::CreateRenderViewForRenderManager, SetSize is called with an
   // empty size. In the Windows code flow, it is not ignored because subsequent
   // sizing calls from the OS flow through TCVW::WasSized which calls SetSize()
   // again. On Cocoa, we rely on the Cocoa view struture and resizer flags to
   // keep things sized properly. On the other hand, if the size is not empty
   // then this is a valid request for a pop-up.
-  if (size.IsEmpty())
+  if (rect.size().IsEmpty())
     return;
 
-  // Do conversions to upper-left origin, as "set size" means keep the
-  // upper-left corner pinned. If the new size is valid, this is a popup whose
-  // superview is another RenderWidgetHostViewCocoa, but even if it's directly
-  // in a TabContentsViewCocoa, they're both BaseViews.
-  DCHECK([[cocoa_view_ superview] isKindOfClass:[BaseView class]]);
-  gfx::Rect rect =
-      [(BaseView*)[cocoa_view_ superview] flipNSRectToRect:[cocoa_view_ frame]];
-  rect.set_width(size.width());
-  rect.set_height(size.height());
-  [cocoa_view_ setFrame:
-      [(BaseView*)[cocoa_view_ superview] flipRectToNSRect:rect]];
+  // The position of |rect| is screen coordnate system and we have to consider
+  // Cocoa coordinate system is upside-down and also muti screen.
+  NSPoint global_origin = NSPointFromCGPoint(rect.origin().ToCGPoint());
+  if ([[NSScreen screens] count] > 0) {
+    NSRect bounds = NSMakeRect(0, 0, rect.width(), rect.height());
+    bounds = [cocoa_view_ convertRect:bounds toView:nil];

[Nico, 2011/02/16 18:18:45]

is convertSize:toView: enough?

+    global_origin.y = [[[NSScreen screens] objectAtIndex:0] frame].size.height -
+        bounds.size.height - global_origin.y;

[Nico, 2011/02/16 18:20:03]

global_origin and the screen's frame are in screen coordinates (pixels), while
bounds is in view coordinates. All coordinates have to be in the same coordinate
system when you do calculations on them.

(Also, use NSHeight().)

[honten.org, 2011/02/16 18:45:37]

Here, my intention is |rect| is actually in view coordinates. So calling
covertRect:bounds converts to screen coordinates and they should be in the same
coordinate. I confirmed with Quartz Debug tools.

If I don't call convertRect:bounds, the render view height position becomes
strange in magnified view.

Or do you mean another problem? Or do I misunderstand?

On 2011/02/16 18:20:03, Nico wrote:

[Nico, 2011/02/16 21:16:01]

Chances are I misunderstand something.

…ah! So GetViewBounds() now returns screen coordinates, but SetSize() and
SetBounds() still get view coordinates? That's terrible  :-/ Can you file a bug
on me to fix this, and put a "// |rect.size()| is view coordinates,
|rect.origin| is screen coordinates, TODO(thakis): fix, http://crbug.com/XXX"
comment at the top of the function?

But with this assumption, this code here is correct. (please consider using
convertSize:toView: instead of convertRect:toView:, and use NSHeight()).

On 2011/02/16 18:45:37, honten wrote:

[honten.org, 2011/02/16 21:18:57]

Sure, I'll file it and fix the nits.

On 2011/02/16 21:16:01, Nico wrote:

[honten.org, 2011/02/18 09:11:26]

Done.

+  }
+
+  // Then |global_origin| is converted to client coordinate system.
+  NSPoint window_origin =
+      [[cocoa_view_ window] convertScreenToBase:global_origin];
+  NSRect frame = NSMakeRect(window_origin.x, window_origin.y,
+                            rect.width(), rect.height());
+  [cocoa_view_ setFrame:frame];
 }
 
 gfx::NativeView RenderWidgetHostViewMac::GetNativeView() {
   return native_view();
 }
 
 void RenderWidgetHostViewMac::MovePluginWindows(
     const std::vector<webkit::npapi::WebPluginGeometry>& moves) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   // Handle movement of accelerated plugins, which are the only "windowed"
@@ skipping 2124 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