RemoteRWHVMac: Update first responder, key, and tooltip methods

Make RWHVMac call through RWHVNSViewBridge, and make RWHVCocoa call
through RWHVNSViewClient.

Some of these methods depended on knowing if RHWVMac::host() still
existed, so refactor tear-down to add a RWHNSViewBridge::Destroy method
which informs the RHWVCocoa whether or not its |client_| is still
callable. Once RWHVCocoa no longer calls into the RWHVMac directly, all
of these lifetimes will be clean-up-able.

Bug: 821651
Change-Id: I30f6060ce661504f1eadf933384a6f1cbe4a3c96
Reviewed-on: https://chromium-review.googlesource.com/979595
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546066}

content/browser/renderer_host/render_widget_host_ns_view_bridge.h

@@ -10,6 +10,7 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "base/strings/string16.h"
 #include "third_party/skia/include/core/SkColor.h"
 
 namespace content {
@@ -32,12 +33,22 @@ class RenderWidgetHostNSViewBridge {
   // method is expected to go away).
   virtual RenderWidgetHostViewCocoa* GetRenderWidgetHostViewCocoa() = 0;
 
+  // Remove the NSView from the view heirarchy and destroy it. After this is
+  // called, no calls back into the RenderWidgetHostNSViewClient may be made.
+  virtual void Destroy() = 0;
+
+  // Make the NSView be the first responder of its NSWindow.
+  virtual void MakeFirstResponder() = 0;
+
   // Set the background color of the hosted CALayer.
   virtual void SetBackgroundColor(SkColor color) = 0;
 
   // Call the -[NSView setHidden:] method.
   virtual void SetVisible(bool visible) = 0;
 
+  // Call the -[NSView setToolTipAtMousePoint] method.
+  virtual void SetTooltipText(const base::string16& display_text) = 0;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostNSViewBridge);
 };

content/browser/renderer_host/render_widget_host_ns_view_bridge.mm

@@ -7,6 +7,7 @@
 #import <Cocoa/Cocoa.h>
 
 #import "base/mac/scoped_nsobject.h"
+#include "base/strings/sys_string_conversions.h"
 #import "content/browser/renderer_host/render_widget_host_view_cocoa.h"
 #include "content/browser/renderer_host/render_widget_host_view_mac.h"
 #import "skia/ext/skia_utils_mac.h"
@@ -30,8 +31,11 @@ class RenderWidgetHostViewNSViewBridgeLocal
   ~RenderWidgetHostViewNSViewBridgeLocal() override;
   RenderWidgetHostViewCocoa* GetRenderWidgetHostViewCocoa() override;
 
+  void Destroy() override;
+  void MakeFirstResponder() override;
   void SetBackgroundColor(SkColor color) override;
   void SetVisible(bool visible) override;
+  void SetTooltipText(const base::string16& display_text) override;
 
  private:
   // display::DisplayObserver implementation.
@@ -45,6 +49,9 @@ class RenderWidgetHostViewNSViewBridgeLocal
   // The background CoreAnimation layer which is hosted by |cocoa_view_|.
   base::scoped_nsobject<CALayer> background_layer_;
 
+  // Cached copy of the tooltip text, to avoid redundant calls.
+  base::string16 tooltip_text_;
+
   DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostViewNSViewBridgeLocal);
 };
 
@@ -72,6 +79,18 @@ RenderWidgetHostViewNSViewBridgeLocal::GetRenderWidgetHostViewCocoa() {
   return cocoa_view_;
 }
 
+void RenderWidgetHostViewNSViewBridgeLocal::Destroy() {
+  [cocoa_view_ setClientWasDestroyed];
+  [cocoa_view_ retain];
+  [cocoa_view_ removeFromSuperview];
+  [cocoa_view_ autorelease];
+  cocoa_view_ = nil;
+}
+
+void RenderWidgetHostViewNSViewBridgeLocal::MakeFirstResponder() {
+  [[cocoa_view_ window] makeFirstResponder:cocoa_view_];
+}
+
 void RenderWidgetHostViewNSViewBridgeLocal::SetBackgroundColor(SkColor color) {
   ScopedCAActionDisabler disabler;
   base::ScopedCFTypeRef<CGColorRef> cg_color(
@@ -84,6 +103,30 @@ void RenderWidgetHostViewNSViewBridgeLocal::SetVisible(bool visible) {
   [cocoa_view_ setHidden:!visible];
 }
 
+void RenderWidgetHostViewNSViewBridgeLocal::SetTooltipText(
+    const base::string16& tooltip_text) {
+  // Called from the renderer to tell us what the tooltip text should be. It
+  // calls us frequently so we need to cache the value to prevent doing a lot
+  // of repeat work.
+  if (tooltip_text == tooltip_text_ || ![[cocoa_view_ window] isKeyWindow])
+    return;
+  tooltip_text_ = tooltip_text;
+
+  // Maximum number of characters we allow in a tooltip.
+  const size_t kMaxTooltipLength = 1024;
+
+  // Clamp the tooltip length to kMaxTooltipLength. It's a DOS issue on
+  // Windows; we're just trying to be polite. Don't persist the trimmed
+  // string, as then the comparison above will always fail and we'll try to
+  // set it again every single time the mouse moves.
+  base::string16 display_text = tooltip_text_;
+  if (tooltip_text_.length() > kMaxTooltipLength)
+    display_text = tooltip_text_.substr(0, kMaxTooltipLength);
+
+  NSString* tooltip_nsstring = base::SysUTF16ToNSString(display_text);
+  [cocoa_view_ setToolTipAtMousePoint:tooltip_nsstring];
+}
+
 void RenderWidgetHostViewNSViewBridgeLocal::OnDisplayMetricsChanged(
     const display::Display& display,
     uint32_t changed_metrics) {

content/browser/renderer_host/render_widget_host_ns_view_client.h

@@ -23,6 +23,15 @@ class RenderWidgetHostNSViewClient {
   // RenderWidgetHostNSViewBridge, this method is to be removed.
   virtual RenderWidgetHostViewMac* GetRenderWidgetHostViewMac() = 0;
 
+  // Indicates that the RenderWidgetHost is to shut down.
+  virtual void OnNSViewRequestShutdown() = 0;
+
+  // Indicates whether or not the NSView is its NSWindow's first responder.
+  virtual void OnNSViewIsFirstResponderChanged(bool is_first_responder) = 0;
+
+  // Indicates whether or not the NSView's NSWindow is key.
+  virtual void OnNSViewWindowIsKeyChanged(bool is_key) = 0;
+
   // Indicates the NSView's bounds in its NSWindow's DIP coordinate system (with
   // the origin at the upper-left corner), and indicate if the the NSView is
   // attached to an NSWindow (if it is not, then |view_bounds_in_window_dip|'s

content/browser/renderer_host/render_widget_host_view_cocoa.h

@@ -49,8 +49,14 @@ struct DidOverscrollParams;
                       NSTextInputClient> {
  @private
   // The communications channel to the RenderWidgetHostViewMac.
+  // TODO(ccameron): When RenderWidgetHostViewCocoa no longer directly accesses
+  // RenderWidgetHostViewMac, then |client_| can be made to be a weak pointer,
+  // and |clientWasDestroyed_| can be replaced with a null-check on |client_|.
   std::unique_ptr<content::RenderWidgetHostNSViewClient> client_;
 
+  // Whether or not it is safe to call back into the |client_| (see above TODO).
+  BOOL clientWasDestroyed_;
+
   // TODO(ccameron): Make all communication with the RenderWidgetHostView go
   // through |client_| and delete this member variable.
   content::RenderWidgetHostViewMac* renderWidgetHostView_;
@@ -196,6 +202,8 @@ struct DidOverscrollParams;
 
 - (void)setCanBeKeyView:(BOOL)can;
 - (void)setCloseOnDeactivate:(BOOL)b;
+// Inidicate that the client was destroyed and can't be called back into.
+- (void)setClientWasDestroyed;
 // True for always-on-top special windows (e.g. Balloons and Panels).
 - (BOOL)acceptsMouseEventsWhenInactive;
 // Cancel ongoing composition (abandon the marked text).

content/browser/renderer_host/render_widget_host_view_cocoa.mm

@@ -296,6 +296,10 @@ void ExtractUnderlines(NSAttributedString* string,
   closeOnDeactivate_ = b;
 }
 
+- (void)setClientWasDestroyed {
+  clientWasDestroyed_ = YES;
+}
+
 - (BOOL)shouldIgnoreMouseEvent:(NSEvent*)theEvent {
   NSWindow* window = [self window];
   // If this is a background window, don't handle mouse movement events. This
@@ -1277,7 +1281,7 @@ void ExtractUnderlines(NSAttributedString* string,
 }
 
 - (BOOL)acceptsFirstResponder {
-  if (!renderWidgetHostView_->host())
+  if (clientWasDestroyed_)
     return NO;
 
   return canBeKeyView_;
@@ -1289,7 +1293,7 @@ void ExtractUnderlines(NSAttributedString* string,
   if ([responderDelegate_ respondsToSelector:@selector(windowDidBecomeKey)])
     [responderDelegate_ windowDidBecomeKey];
   if ([self window].isKeyWindow && [[self window] firstResponder] == self)
-    renderWidgetHostView_->SetActive(true);
+    client_->OnNSViewWindowIsKeyChanged(true);
 }
 
 - (void)windowDidResignKey:(NSNotification*)notification {
@@ -1304,17 +1308,16 @@ void ExtractUnderlines(NSAttributedString* string,
     return;
 
   if ([[self window] firstResponder] == self)
-    renderWidgetHostView_->SetActive(false);
+    client_->OnNSViewWindowIsKeyChanged(false);
 }
 
 - (BOOL)becomeFirstResponder {
-  if (!renderWidgetHostView_->host())
+  if (clientWasDestroyed_)
     return NO;
   if ([responderDelegate_ respondsToSelector:@selector(becomeFirstResponder)])
     [responderDelegate_ becomeFirstResponder];
 
-  renderWidgetHostView_->host()->GotFocus();
-  renderWidgetHostView_->SetTextInputActive(true);
+  client_->OnNSViewIsFirstResponderChanged(true);
 
   // Cancel any onging composition text which was left before we lost focus.
   // TODO(suzhe): We should do it in -resignFirstResponder: method, but
@@ -1337,14 +1340,15 @@ void ExtractUnderlines(NSAttributedString* string,
 - (BOOL)resignFirstResponder {
   if ([responderDelegate_ respondsToSelector:@selector(resignFirstResponder)])
     [responderDelegate_ resignFirstResponder];
-  renderWidgetHostView_->SetTextInputActive(false);
-  if (!renderWidgetHostView_->host())
-    return YES;
 
-  if (closeOnDeactivate_)
-    renderWidgetHostView_->KillSelf();
+  if (clientWasDestroyed_)
+    return YES;
 
-  renderWidgetHostView_->host()->LostFocus();
+  client_->OnNSViewIsFirstResponderChanged(false);
+  if (closeOnDeactivate_) {
+    [self setHidden:YES];
+    client_->OnNSViewRequestShutdown();
+  }
 
   // We should cancel any onging composition whenever RWH's Blur() method gets
   // called, because in this case, webkit will confirm the ongoing composition
@@ -1869,11 +1873,15 @@ extern NSString* NSTextInputReplacementRangeAttributeName;
   if (!renderWidgetHostView_->browser_compositor_)
     return;
 
-  // Update the window's frame, the view's bounds, and the display info, as they
-  // have not been updated while unattached to a window.
+  // Update the window's frame, the view's bounds, focus, and the display info,
+  // as they have not been updated while unattached to a window.
   [self sendWindowFrameInScreenToClient];
   [self sendViewBoundsInWindowToClient];
   [self updateScreenProperties];
+  if (!clientWasDestroyed_) {
+    client_->OnNSViewIsFirstResponderChanged([[self window] firstResponder] ==
+                                             self);
+  }
 
   // If we switch windows (or are removed from the view hierarchy), cancel any
   // open mouse-downs.
@@ -2019,7 +2027,8 @@ extern NSString* NSTextInputReplacementRangeAttributeName;
 }
 
 - (void)popupWindowWillClose:(NSNotification*)notification {
-  renderWidgetHostView_->KillSelf();
+  [self setHidden:YES];
+  client_->OnNSViewRequestShutdown();
 }
 
 @end

content/browser/renderer_host/render_widget_host_view_mac.h

@@ -208,8 +208,6 @@ class CONTENT_EXPORT RenderWidgetHostViewMac
   // Forwards the mouse event to the renderer.
   void ForwardMouseEvent(const blink::WebMouseEvent& event);
 
-  void KillSelf();
-
   void SetTextInputActive(bool active);
 
   // Returns true and stores first rectangle for character range if the
@@ -283,6 +281,9 @@ class CONTENT_EXPORT RenderWidgetHostViewMac
 
   // RenderWidgetHostNSViewClient implementation.
   RenderWidgetHostViewMac* GetRenderWidgetHostViewMac() override;
+  void OnNSViewRequestShutdown() override;
+  void OnNSViewIsFirstResponderChanged(bool is_first_responder) override;
+  void OnNSViewWindowIsKeyChanged(bool is_key) override;
   void OnNSViewBoundsInWindowChanged(const gfx::Rect& view_bounds_in_window_dip,
                                      bool attached_to_window) override;
   void OnNSViewWindowFrameInScreenChanged(
@@ -386,6 +387,9 @@ class CONTENT_EXPORT RenderWidgetHostViewMac
   // Cached copy of the display information pushed to us from the NSView.
   display::Display display_;
 
+  // Whether or not the NSView is first responder.
+  bool is_first_responder_ = false;
+
   // Indicates if the page is loading.
   bool is_loading_;
 
@@ -395,9 +399,6 @@ class CONTENT_EXPORT RenderWidgetHostViewMac
   // The last scroll offset of the view.
   gfx::Vector2dF last_scroll_offset_;
 
-  // The text to be shown in the tooltip, supplied by the renderer.
-  base::string16 tooltip_text_;
-
   // True when this view acts as a platform view hack for a
   // RenderWidgetHostViewGuest.
   bool is_guest_view_hack_;

content/browser/renderer_host/render_widget_host_view_mac.mm

@@ -144,13 +144,6 @@ using blink::WebGestureEvent;
 
 @end
 
-namespace {
-
-// Maximum number of characters we allow in a tooltip.
-const size_t kMaxTooltipLength = 1024;
-
-}  // namespace
-
 namespace content {
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -623,11 +616,11 @@ gfx::NativeViewAccessible RenderWidgetHostViewMac::GetNativeViewAccessible() {
 }
 
 void RenderWidgetHostViewMac::Focus() {
-  [[cocoa_view() window] makeFirstResponder:cocoa_view()];
+  ns_view_bridge_->MakeFirstResponder();
 }
 
 bool RenderWidgetHostViewMac::HasFocus() const {
-  return [[cocoa_view() window] firstResponder] == cocoa_view();
+  return is_first_responder_;
 }
 
 bool RenderWidgetHostViewMac::IsSurfaceAvailableForCopy() const {
@@ -792,10 +785,9 @@ void RenderWidgetHostViewMac::Destroy() {
               object:popup_window_];
 
   // We've been told to destroy.
-  [cocoa_view() retain];
-  [cocoa_view() removeFromSuperview];
-  [cocoa_view() autorelease];
-
+  if (ns_view_bridge_)
+    ns_view_bridge_->Destroy();
+  ns_view_bridge_.reset();
   [popup_window_ close];
   popup_window_.autorelease();
 
@@ -829,25 +821,9 @@ void RenderWidgetHostViewMac::Destroy() {
   RenderWidgetHostViewBase::Destroy();
 }
 
-// Called from the renderer to tell us what the tooltip text should be. It
-// calls us frequently so we need to cache the value to prevent doing a lot
-// of repeat work.
 void RenderWidgetHostViewMac::SetTooltipText(
     const base::string16& tooltip_text) {
-  if (tooltip_text != tooltip_text_ && [[cocoa_view() window] isKeyWindow]) {
-    tooltip_text_ = tooltip_text;
-
-    // Clamp the tooltip length to kMaxTooltipLength. It's a DOS issue on
-    // Windows; we're just trying to be polite. Don't persist the trimmed
-    // string, as then the comparison above will always fail and we'll try to
-    // set it again every single time the mouse moves.
-    base::string16 display_text = tooltip_text_;
-    if (tooltip_text_.length() > kMaxTooltipLength)
-      display_text = tooltip_text_.substr(0, kMaxTooltipLength);
-
-    NSString* tooltip_nsstring = base::SysUTF16ToNSString(display_text);
-    [cocoa_view() setToolTipAtMousePoint:tooltip_nsstring];
-  }
+  ns_view_bridge_->SetTooltipText(tooltip_text);
 }
 
 viz::ScopedSurfaceIdAllocator RenderWidgetHostViewMac::ResizeDueToAutoResize(
@@ -987,10 +963,8 @@ void RenderWidgetHostViewMac::ForwardMouseEvent(const WebMouseEvent& event) {
   if (host())
     host()->ForwardMouseEvent(event);
 
-  if (event.GetType() == WebInputEvent::kMouseLeave) {
-    [cocoa_view() setToolTipAtMousePoint:nil];
-    tooltip_text_.clear();
-  }
+  if (event.GetType() == WebInputEvent::kMouseLeave)
+    ns_view_bridge_->SetTooltipText(base::string16());
 }
 
 void RenderWidgetHostViewMac::SetNeedsBeginFrames(bool needs_begin_frames) {
@@ -1013,15 +987,6 @@ void RenderWidgetHostViewMac::SetWantsAnimateOnlyBeginFrames() {
   browser_compositor_->SetWantsAnimateOnlyBeginFrames();
 }
 
-void RenderWidgetHostViewMac::KillSelf() {
-  if (!weak_factory_.HasWeakPtrs()) {
-    [cocoa_view() setHidden:YES];
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(&RenderWidgetHostViewMac::ShutdownHost,
-                              weak_factory_.GetWeakPtr()));
-  }
-}
-
 bool RenderWidgetHostViewMac::GetLineBreakIndex(
     const std::vector<gfx::Rect>& bounds,
     const gfx::Range& range,
@@ -1261,7 +1226,7 @@ bool RenderWidgetHostViewMac::LockMouse() {
   [NSCursor hide];
 
   // Clear the tooltip window.
-  SetTooltipText(base::string16());
+  ns_view_bridge_->SetTooltipText(base::string16());
 
   return true;
 }
@@ -1510,6 +1475,32 @@ RenderWidgetHostViewMac* RenderWidgetHostViewMac::GetRenderWidgetHostViewMac() {
   return this;
 }
 
+void RenderWidgetHostViewMac::OnNSViewRequestShutdown() {
+  if (!weak_factory_.HasWeakPtrs()) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(&RenderWidgetHostViewMac::ShutdownHost,
+                                  weak_factory_.GetWeakPtr()));
+  }
+}
+
+void RenderWidgetHostViewMac::OnNSViewIsFirstResponderChanged(
+    bool is_first_responder) {
+  if (is_first_responder_ == is_first_responder)
+    return;
+  is_first_responder_ = is_first_responder;
+  if (is_first_responder_) {
+    host()->GotFocus();
+    SetTextInputActive(true);
+  } else {
+    SetTextInputActive(false);
+    host()->LostFocus();
+  }
+}
+
+void RenderWidgetHostViewMac::OnNSViewWindowIsKeyChanged(bool is_key) {
+  SetActive(is_key);
+}
+
 void RenderWidgetHostViewMac::OnNSViewBoundsInWindowChanged(
     const gfx::Rect& view_bounds_in_window_dip,
     bool attached_to_window) {