MacViews: Implement text input.

ui/views/cocoa/bridged_native_widget.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/cocoa/bridged_native_widget.h"
 
 #include "base/logging.h"
-#import "ui/views/cocoa/bridged_content_view.h"
+#include "ui/views/cocoa/bridged_content_view.h"

[tapted, 2014/06/17 13:23:56]

include -> import. Typically import is used if a header happens to be one that
can't be compiled into a .cc file

[Andre, 2014/06/18 21:48:35]

Done.

+#include "ui/views/view.h"
 
 namespace views {
 
 BridgedNativeWidget::BridgedNativeWidget() {
 }
 
 BridgedNativeWidget::~BridgedNativeWidget() {
   SetRootView(NULL);
 }
 
 void BridgedNativeWidget::Init(base::scoped_nsobject<NSWindow> window) {
   DCHECK(!window_);
   window_.swap(window);
 }
 
 void BridgedNativeWidget::SetRootView(views::View* view) {
   if (view == [bridged_view_ view])
     return;
 
+  if ([bridged_view_ view])
+    [bridged_view_ view]->GetFocusManager()->RemoveFocusChangeListener(this);

[tapted, 2014/06/17 13:23:56]

I think this belongs in [BridgedContentView clearView] (which can get an early
return if already cleared).

Of course, [BridgedContentView initWithView] should be symmetric with that, so
the initializer will need to take a `BridgedNativeWidget* parent` argument. That
was always the plan - it just wouldn't have been used yet the first time it
landed.

[Andre, 2014/06/18 21:48:35]

Done.

+
   [bridged_view_ clearView];
   bridged_view_.reset();
   // Note that there can still be references to the old |bridged_view_|
   // floating around in Cocoa libraries at this point. However, references to
   // the old views::View will be gone, so any method calls will become no-ops.
 
-  if (view)
+  if (view) {
     bridged_view_.reset([[BridgedContentView alloc] initWithView:view]);
+    view->GetFocusManager()->AddFocusChangeListener(this);
+  }
   [window_ setContentView:bridged_view_];
   [window_ makeFirstResponder:bridged_view_];
 }
 
+// FocusChangeListener implementation.

[tapted, 2014/06/17 13:23:56]

I usually skip these comments for C++ implementations, since it's redundant from
the declaration. Just need to keep everything in the same order.

However, really big classes like views::View and NativeWidgetFoo tend to get a
different style of comment.

[Andre, 2014/06/18 21:48:35]

Done.

+
+void BridgedNativeWidget::OnWillChangeFocus(View* focused_before,
+                                            View* focused_now) {
+}
+
+void BridgedNativeWidget::OnDidChangeFocus(View* focused_before,
+                                           View* focused_now) {
+  if (focused_now)

[tapted, 2014/06/17 13:23:56]

nit: I'd probably write this

  ui::TextInputClient* input_client =
      focused_now ? focused_now->GetTextInputClient() : NULL;
  [bridged_view_ setTextInputClient:input_client];

I think that pattern is more common. And, since there's an NSTestInputClient and
a ui::TextInputClient it helps clear things up a bit.

[Andre, 2014/06/18 21:48:35]

Done.

+    [bridged_view_ setTextInputClient:focused_now->GetTextInputClient()];
+  else
+    [bridged_view_ setTextInputClient:NULL];
+}
+
 }  // namespace views