[Autofill] Add Mac implementation for the in-browser process popup view.

chrome/browser/ui/cocoa/autofill/autofill_popup_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.
+
+#import <Cocoa/Cocoa.h>
+
+#include "chrome/browser/ui/cocoa/autofill/autofill_popup_view_mac.h"
+
+#include "base/logging.h"
+#include "chrome/browser/ui/autofill/autofill_popup_controller.h"
+#import "chrome/browser/ui/cocoa/autofill/autofill_popup_view_cocoa.h"
+#include "ui/base/cocoa/window_size_constants.h"
+#include "ui/gfx/rect.h"
+
+AutofillPopupViewMac::AutofillPopupViewMac(
+    AutofillPopupController* controller)
+    : controller_(controller) {
+  window_ =
+      [[NSWindow alloc] initWithContentRect:ui::kWindowSizeDeterminedLater
+                                  styleMask:NSBorderlessWindowMask
+                                    backing:NSBackingStoreBuffered
+                                      defer:YES];
+  // TODO(isherman): Should this be NSPopupMenuWindowLevel or perhaps
+  // NSFloatingWindowLevel or NSModalPanelWindowLevel instead?

[Ilya Sherman, 2013/01/04 01:24:29]

Friendly Cocoa reviewers: Thoughts?

[Scott Hess - ex-Googler, 2013/01/05 01:06:49]

Omnibox is at NSNormalWindowLevel.  Levels are a bit weird - so long as you
order above your window, and are operating WRT that window (not as something
independent), you maybe don't need to worry about this at all.  Actually, I see
that below you're making it a child window - in that case I don't think you need
to worry about levels at all.

[Ilya Sherman, 2013/01/05 03:14:10]

Done.

+  [window_ setLevel:NSNormalWindowLevel];
+  // TODO(isherman): Are these explicit calls needed / recommended?

[Ilya Sherman, 2013/01/04 01:24:29]

Friendly Cocoa reviewers: Thoughts?

[Scott Hess - ex-Googler, 2013/01/05 01:06:49]

For borderless, you might need to disable moving by background.  I'm not sure
how the background color and alpha interact.  The opaque setting is a signal
others use to optimize things (setting to YES signals that you're arranging it
to be opaque).

[Ilya Sherman, 2013/01/05 03:14:10]

It looks like everything works if I remove all of these, including the window
level, so I've removed them (other than opaque), on the assumption that less
code is better.  Let me know if there are some you think I should definitely
keep.

+  [window_ setMovableByWindowBackground:NO];
+  [window_ setBackgroundColor:[NSColor clearColor]];
+  [window_ setAlphaValue:1.0];
+  [window_ setOpaque:YES];
+  [window_ setHasShadow:NO];
+  [window_ accessibilitySetOverrideValue:NSAccessibilityUnknownRole
+                            forAttribute:NSAccessibilityRoleAttribute];
+
+  view_ = [[[AutofillPopupViewCocoa alloc]
+             initWithController:controller_
+                          frame:[window_ frame]] autorelease];
+  [window_ setContentView:view_];
+}
+
+AutofillPopupViewMac::~AutofillPopupViewMac() {
+  [view_ controllerDestroyed];

[Scott Hess - ex-Googler, 2013/01/05 01:06:49]

Since this is called from the destructor, you might want to document on the view
side that the controller_ should not be called at this point.

[Ilya Sherman, 2013/01/05 03:14:10]

Sorry, I'm confused -- document where?

[Scott Hess - ex-Googler, 2013/01/09 21:36:17]

I mean that -controllerDestroyed cannot send any messages to |this|, so it might
be worth documenting that at -controllerDestroyed's implementation.

[Ilya Sherman, 2013/01/10 02:21:21]

Done.

+  controller_->ViewDestroyed();
+
+  [window_ close];
+}
+
+void AutofillPopupViewMac::Hide() {
+  delete this;
+}
+
+void AutofillPopupViewMac::Show() {
+  SetInitialBounds();
+  UpdateBoundsAndRedrawPopup();
+  [[controller_->container_view() window] addChildWindow:window_
+                                                 ordered:NSWindowAbove];
+}
+
+void AutofillPopupViewMac::InvalidateRow(size_t row) {
+  gfx::Rect dirty_rect = controller_->GetRowBounds(row);
+  [view_ setNeedsDisplayInRect:[view_ flipRectToNSRect:dirty_rect]];
+}
+
+void AutofillPopupViewMac::UpdateBoundsAndRedrawPopup() {
+  [window_ setFrame:NSRectFromCGRect(controller_->popup_bounds().ToCGRect())
+            display:YES];
+}
+
+void AutofillPopupViewMac::SetInitialBounds() {
+  // The bounds rect in Chrome's screen coordinates.
+  // TODO(isherman): Does this handle Hi-DPI mode correctly?  According to the
+  // docs, the origin should be in screen coordinates, whereas the size should
+  // be in view coordinates.

[Ilya Sherman, 2013/01/04 01:24:29]

Friendly Cocoa reviewers: Thoughts?

[Scott Hess - ex-Googler, 2013/01/05 01:06:49]

I think I'm going to need to patch this in and look at it before I can
speculate.  What kind of thing do I need to be doing in a webpage to see this at
work?

[Ilya Sherman, 2013/01/05 03:14:10]

As preconditions, you'll need to enable the flag for this UI, and you'll need to
have some Autofill profiles or credit cards saved at chrome://settings/autofill.
 (Credit cards work best, since they have extra icons.)  Then, navigate to a
page like [1] with a fillable form, and double click one of the form fields to
bring up the UI.  For best results, compare to the same action in an official
build, since this CL is about migrating that UI to be a Chrome native one.

[1] https://www.corp.google.com/~isherman/no_crawl/autofill/cc.html

[Scott Hess - ex-Googler, 2013/01/09 21:36:17]

Weird.  When I double-click a field, then click (either in the box or elsewhere
on the page), the Chromium window dissappears.  I can get it back by switching
to another application and then back, but the popup stays up and I can't get it
to go away, even when switching tabs.  I'm also seeing lots of string truncation
("Chromium Autofi").  This is on a retina mac running 10.8.

Anyhow, looking at things, if I were to make suggestions about the "best" way to
break things down, I'd use a window with a custom content view to draw the place
you're putting things, then a subview which was an NSMatrix for the elements,
which would be custom cells.  But the use of a separator may mess that up.  The
rationale for using this kind of setup is that various framework things will
kick in automatically, like handling of accessibility, and possibly the RTL
stuff.

Also - what is your scrolling story if the popup gets too big?

[Ilya Sherman, 2013/01/10 02:21:21]

Huh, I don't see that behavior.  I guess this means I'm doing something dumb
that causes unspecified behavior, but I'm not sure what that something dumb is
-- any ideas?
Yeah, the strings are being intentionally truncated in the cross-platform
controller because we haven't figured out the story for eliding the strings when
they get to long.  This is our slap-you-in-the-face reminder not to ship the UI
prior to figuring that out.
Hmm, I'll investigate that.
Not sure... I should probably figure that out.  Added a TODO for now.

[Ilya Sherman, 2013/01/10 04:17:15]

According to the docs, the separators do indeed mess that up: "The cell objects
that an NSMatrix contains are usually of a single subclass of NSCell, but they
can be of multiple subclasses of NSCell. The only restriction is that all cell
objects must be the same size."

I suppose I could theoretically draw the separator as a border on the cell above
it, though it would mean that the controller and the view would disagree on the
view's size by a pixel per separator.  This approach sounds generally kind of
icky, and I'd prefer to find some moar better solution.  WDYT?

+  gfx::Rect bounds(controller_->element_bounds().x(),
+                   controller_->element_bounds().bottom(),
+                   controller_->GetPopupRequiredWidth(),
+                   controller_->GetPopupRequiredHeight());
+
+  // Flip coordinates back into Cocoa-land.
+  // TODO(isherman): Does this handle multi-monitor setups correctly?
+  NSScreen* screen = [[NSScreen screens] objectAtIndex:0];
+  bounds.set_y([screen frame].size.height - bounds.y() - bounds.height());
+
+  // TODO(isherman): Position the popup correctly if it can't fit below the text
+  // field: http://crbug.com/164603
+
+  controller_->SetPopupBounds(bounds);
+}
+
+AutofillPopupView* AutofillPopupView::Create(
+    AutofillPopupController* controller) {
+  return new AutofillPopupViewMac(controller);
+}