Remove redundant code from RWHVCocoa, move some code to RWHVNSViewBridge

Remove suppressNextEscapeKeyUp_ because its functionality is made
redundant by keyDownCodes_. The escape-key-specific version was
introduced in crrev.com/165615 while the generic version was introduced
in crrev.com/304728.

While in the -[NSView keyEvent:] method, replace repeated method calls
with local variables in preparation for moving most of the funciton
across an IPC boundary, and fold shouldAutohideCursorForEvent into
the function (since it's called nowhere else).

Move SetBackgroundColor and Show/Hide/IsHidden functionality into
the RWHVNSViewBridge.

Previously committed as crrev.com/545160

TBR=tapted (reviewed previous version, identical except for test fix)

Bug: 821651
Change-Id: Ic7cab38ea46c25de7f9be1e5127e2a7b4bb2b52a
Reviewed-on: https://chromium-review.googlesource.com/977227
Reviewed-by: ccameron <ccameron@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545353}

content/browser/renderer_host/render_widget_host_ns_view_bridge.h

@@ -10,6 +10,7 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "third_party/skia/include/core/SkColor.h"
 
 namespace content {
 
@@ -31,6 +32,12 @@ class RenderWidgetHostNSViewBridge {
   // method is expected to go away).
   virtual RenderWidgetHostViewCocoa* GetRenderWidgetHostViewCocoa() = 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;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostNSViewBridge);
 };

content/browser/renderer_host/render_widget_host_ns_view_bridge.mm

@@ -4,8 +4,13 @@
 
 #import "content/browser/renderer_host/render_widget_host_ns_view_bridge.h"
 
+#import <Cocoa/Cocoa.h>
+
+#import "base/mac/scoped_nsobject.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"
+#import "ui/base/cocoa/animation_utils.h"
 
 namespace content {
 
@@ -18,26 +23,56 @@
     : public RenderWidgetHostNSViewBridge {
  public:
   explicit RenderWidgetHostViewNSViewBridgeLocal(
-      std::unique_ptr<RenderWidgetHostNSViewClient> client) {
-    // Since we autorelease |cocoa_view|, our caller must put |GetNativeView()|
-    // into the view hierarchy right after calling us.
-    cocoa_view_ = [[[RenderWidgetHostViewCocoa alloc]
-        initWithClient:std::move(client)] autorelease];
-  }
-  ~RenderWidgetHostViewNSViewBridgeLocal() override {}
-
-  RenderWidgetHostViewCocoa* GetRenderWidgetHostViewCocoa() override {
-    return cocoa_view_;
-  }
+      std::unique_ptr<RenderWidgetHostNSViewClient> client);
+  ~RenderWidgetHostViewNSViewBridgeLocal() override;
+  RenderWidgetHostViewCocoa* GetRenderWidgetHostViewCocoa() override;
+
+  void SetBackgroundColor(SkColor color) override;
+  void SetVisible(bool visible) override;
 
  private:
   // Weak, this is owned by |cocoa_view_|'s |client_|, and |cocoa_view_| owns
   // its |client_|.
   RenderWidgetHostViewCocoa* cocoa_view_ = nil;
 
+  // The background CoreAnimation layer which is hosted by |cocoa_view_|.
+  base::scoped_nsobject<CALayer> background_layer_;
+
   DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostViewNSViewBridgeLocal);
 };
 
+RenderWidgetHostViewNSViewBridgeLocal::RenderWidgetHostViewNSViewBridgeLocal(
+    std::unique_ptr<RenderWidgetHostNSViewClient> client) {
+  // Since we autorelease |cocoa_view|, our caller must put |GetNativeView()|
+  // into the view hierarchy right after calling us.
+  cocoa_view_ = [[[RenderWidgetHostViewCocoa alloc]
+      initWithClient:std::move(client)] autorelease];
+
+  background_layer_.reset([[CALayer alloc] init]);
+  [cocoa_view_ setLayer:background_layer_];
+  [cocoa_view_ setWantsLayer:YES];
+}
+
+RenderWidgetHostViewNSViewBridgeLocal::
+    ~RenderWidgetHostViewNSViewBridgeLocal() {}
+
+RenderWidgetHostViewCocoa*
+RenderWidgetHostViewNSViewBridgeLocal::GetRenderWidgetHostViewCocoa() {
+  return cocoa_view_;
+}
+
+void RenderWidgetHostViewNSViewBridgeLocal::SetBackgroundColor(SkColor color) {
+  ScopedCAActionDisabler disabler;
+  base::ScopedCFTypeRef<CGColorRef> cg_color(
+      skia::CGColorCreateFromSkColor(color));
+  [background_layer_ setBackgroundColor:cg_color];
+}
+
+void RenderWidgetHostViewNSViewBridgeLocal::SetVisible(bool visible) {
+  ScopedCAActionDisabler disabler;
+  [cocoa_view_ setHidden:!visible];
+}
+
 }  // namespace
 
 // static

content/browser/renderer_host/render_widget_host_view_cocoa.h

@@ -156,12 +156,6 @@ struct DidOverscrollParams;
   // gesture ends.
   BOOL gestureBeginPinchSent_;
 
-  // If true then escape key down events are suppressed until the first escape
-  // key up event. (The up event is suppressed as well). This is used by the
-  // flash fullscreen code to avoid sending a key up event without a matching
-  // key down event.
-  BOOL suppressNextEscapeKeyUp_;
-
   // This is used to indicate if a stylus is currently in the proximity of the
   // tablet.
   bool isStylusEnteringProximity_;
@@ -179,7 +173,6 @@ struct DidOverscrollParams;
 
 @property(nonatomic, assign) NSRange selectedRange;
 @property(nonatomic, assign) NSRange markedRange;
-@property(nonatomic, readonly) BOOL suppressNextEscapeKeyUp;
 
 // Common code path for handling begin gesture events. This helper method is
 // called via different codepaths based on OS version and SDK:
@@ -213,6 +206,7 @@ struct DidOverscrollParams;
 - (void)showLookUpDictionaryOverlayAtPoint:(NSPoint)point;
 - (void)showLookUpDictionaryOverlayFromRange:(NSRange)range
                                   targetView:(NSView*)targetView;
+- (BOOL)suppressNextKeyUpForTesting:(int)keyCode;
 
 // Methods previously marked as private.
 - (id)initWithClient:

content/browser/renderer_host/render_widget_host_view_cocoa.mm

@@ -136,7 +136,6 @@ - (BOOL)isSpeaking;
 
 // Private methods:
 @interface RenderWidgetHostViewCocoa ()
-- (BOOL)shouldAutohideCursorForEvent:(NSEvent*)event;
 - (void)processedWheelEvent:(const blink::WebMouseWheelEvent&)event
                    consumed:(BOOL)consumed;
 - (void)keyEvent:(NSEvent*)theEvent wasKeyEquivalent:(BOOL)equiv;
@@ -152,7 +151,6 @@ - (void)showLookUpDictionaryOverlayInternal:(NSAttributedString*)string
 
 @implementation RenderWidgetHostViewCocoa
 @synthesize selectedRange = selectedRange_;
-@synthesize suppressNextEscapeKeyUp = suppressNextEscapeKeyUp_;
 @synthesize markedRange = markedRange_;
 
 - (id)initWithClient:(std::unique_ptr<RenderWidgetHostNSViewClient>)client {
@@ -478,6 +476,9 @@ - (EventHandled)keyEvent:(NSEvent*)theEvent {
 
 - (void)keyEvent:(NSEvent*)theEvent wasKeyEquivalent:(BOOL)equiv {
   TRACE_EVENT0("browser", "RenderWidgetHostViewCocoa::keyEvent");
+  NSEventType eventType = [theEvent type];
+  NSEventModifierFlags modifierFlags = [theEvent modifierFlags];
+  int keyCode = [theEvent keyCode];
 
   // If the user changes the system hotkey mapping after Chrome has been
   // launched, then it is possible that a formerly reserved system hotkey is no
@@ -491,15 +492,14 @@ - (void)keyEvent:(NSEvent*)theEvent wasKeyEquivalent:(BOOL)equiv {
   if (EventIsReservedBySystem(theEvent))
     return;
 
-  DCHECK([theEvent type] != NSKeyDown ||
-         !equiv == !([theEvent modifierFlags] & NSCommandKeyMask));
+  DCHECK(eventType != NSKeyDown ||
+         !equiv == !(modifierFlags & NSCommandKeyMask));
 
-  if ([theEvent type] == NSFlagsChanged) {
+  if (eventType == NSFlagsChanged) {
     // Ignore NSFlagsChanged events from the NumLock and Fn keys as
     // Safari does in -[WebHTMLView flagsChanged:] (of "WebHTMLView.mm").
     // Also ignore unsupported |keyCode| (255) generated by Convert, NonConvert
     // and KanaMode from JIS PC keyboard.
-    int keyCode = [theEvent keyCode];
     if (!keyCode || keyCode == 10 || keyCode == 63 || keyCode == 255)
       return;
   }
@@ -524,10 +524,6 @@ - (void)keyEvent:(NSEvent*)theEvent wasKeyEquivalent:(BOOL)equiv {
   if (event.GetType() == NativeWebKeyboardEvent::kRawKeyDown &&
       event.windows_key_code == ui::VKEY_ESCAPE &&
       renderWidgetHostView_->pepper_fullscreen_window()) {
-    RenderWidgetHostViewMac* parent =
-        renderWidgetHostView_->fullscreen_parent_host_view();
-    if (parent)
-      parent->cocoa_view()->suppressNextEscapeKeyUp_ = YES;
     widgetHost->ShutdownAndDestroyWidget(true);
     return;
   }
@@ -539,36 +535,34 @@ - (void)keyEvent:(NSEvent*)theEvent wasKeyEquivalent:(BOOL)equiv {
   if (!widgetHost)
     return;
 
-  // Suppress the escape key up event if necessary.
-  if (event.windows_key_code == ui::VKEY_ESCAPE && suppressNextEscapeKeyUp_) {
-    if (event.GetType() == NativeWebKeyboardEvent::kKeyUp)
-      suppressNextEscapeKeyUp_ = NO;
-    return;
-  }
-
   // Do not forward key up events unless preceded by a matching key down,
   // otherwise we might get an event from releasing the return key in the
-  // omnibox (http://crbug.com/338736).
-  if ([theEvent type] == NSKeyUp) {
-    auto numErased = keyDownCodes_.erase([theEvent keyCode]);
+  // omnibox (https://crbug.com/338736) or from closing another window
+  // (https://crbug.com/155492).
+  if (eventType == NSKeyUp) {
+    auto numErased = keyDownCodes_.erase(keyCode);
     if (numErased < 1)
       return;
   }
 
+  bool shouldAutohideCursor =
+      renderWidgetHostView_->GetTextInputType() != ui::TEXT_INPUT_TYPE_NONE &&
+      eventType == NSKeyDown && !(modifierFlags & NSCommandKeyMask);
+
   // We only handle key down events and just simply forward other events.
-  if ([theEvent type] != NSKeyDown) {
+  if (eventType != NSKeyDown) {
     widgetHost->ForwardKeyboardEventWithLatencyInfo(event, latency_info);
 
     // Possibly autohide the cursor.
-    if ([self shouldAutohideCursorForEvent:theEvent]) {
+    if (shouldAutohideCursor) {
       [NSCursor setHiddenUntilMouseMoves:YES];
       cursorHidden_ = YES;
     }
 
     return;
   }
 
-  keyDownCodes_.insert([theEvent keyCode]);
+  keyDownCodes_.insert(keyCode);
 
   base::scoped_nsobject<RenderWidgetHostViewCocoa> keepSelfAlive([self retain]);
 
@@ -722,7 +716,7 @@ - (void)keyEvent:(NSEvent*)theEvent wasKeyEquivalent:(BOOL)equiv {
       widgetHost->ForwardKeyboardEventWithLatencyInfo(event, latency_info);
     } else if ((!textInserted || delayEventUntilAfterImeCompostion) &&
                event.text[0] != '\0' &&
-               (([theEvent modifierFlags] & kCtrlCmdKeyMask) ||
+               ((modifierFlags & kCtrlCmdKeyMask) ||
                 (hasEditCommands_ && editCommands_.empty()))) {
       // We don't get insertText: calls if ctrl or cmd is down, or the key event
       // generates an insert command. So synthesize a keypress event for these
@@ -734,12 +728,16 @@ - (void)keyEvent:(NSEvent*)theEvent wasKeyEquivalent:(BOOL)equiv {
   }
 
   // Possibly autohide the cursor.
-  if ([self shouldAutohideCursorForEvent:theEvent]) {
+  if (shouldAutohideCursor) {
     [NSCursor setHiddenUntilMouseMoves:YES];
     cursorHidden_ = YES;
   }
 }
 
+- (BOOL)suppressNextKeyUpForTesting:(int)keyCode {
+  return keyDownCodes_.count(keyCode) == 0;
+}
+
 - (void)forceTouchEvent:(NSEvent*)theEvent {
   if (ui::ForceClickInvokesQuickLook())
     [self quickLookWithEvent:theEvent];
@@ -1346,18 +1344,6 @@ - (RenderWidgetHostViewMac*)renderWidgetHostViewMac {
   return renderWidgetHostView_;
 }
 
-// Determine whether we should autohide the cursor (i.e., hide it until mouse
-// move) for the given event. Customize here to be more selective about which
-// key presses to autohide on.
-- (BOOL)shouldAutohideCursorForEvent:(NSEvent*)event {
-  return (renderWidgetHostView_->GetTextInputType() !=
-              ui::TEXT_INPUT_TYPE_NONE &&
-          [event type] == NSKeyDown &&
-          !([event modifierFlags] & NSCommandKeyMask))
-             ? YES
-             : NO;
-}
-
 - (NSArray*)accessibilityArrayAttributeValues:(NSString*)attribute
                                         index:(NSUInteger)index
                                      maxCount:(NSUInteger)maxCount {

content/browser/renderer_host/render_widget_host_view_mac.h

@@ -257,9 +257,6 @@ class CONTENT_EXPORT RenderWidgetHostViewMac
   // These member variables should be private, but the associated ObjC class
   // needs access to them and can't be made a friend.
 
-  // The background CoreAnimation layer which is hosted by |cocoa_view_|.
-  base::scoped_nsobject<CALayer> background_layer_;
-
   // Delegated frame management and compositor interface.
   std::unique_ptr<BrowserCompositorMac> browser_compositor_;
   BrowserCompositorMac* BrowserCompositorForTesting() const {
@@ -283,10 +280,6 @@ class CONTENT_EXPORT RenderWidgetHostViewMac
 
   CONTENT_EXPORT void release_pepper_fullscreen_window_for_testing();
 
-  RenderWidgetHostViewMac* fullscreen_parent_host_view() const {
-    return fullscreen_parent_host_view_;
-  }
-
   int window_number() const;
 
   // Update the size, scale factor, color profile, and any other properties
@@ -385,6 +378,9 @@ class CONTENT_EXPORT RenderWidgetHostViewMac
   // Interface through which the NSView is to be manipulated.
   std::unique_ptr<RenderWidgetHostNSViewBridge> ns_view_bridge_;
 
+  // State tracked by Show/Hide/IsShowing.
+  bool is_visible_ = false;
+
   // Indicates if the page is loading.
   bool is_loading_;
 

content/browser/renderer_host/render_widget_host_view_mac.mm

@@ -259,7 +259,6 @@ float FlipYFromRectToScreen(float y, float rect_height) {
       is_loading_(false),
       allow_pause_for_resize_or_repaint_(true),
       is_guest_view_hack_(is_guest_view_hack),
-      fullscreen_parent_host_view_(nullptr),
       weak_factory_(this) {
   // The NSView on the other side of |ns_view_bridge_| owns us. We will
   // be destroyed when it releases the unique_ptr that we pass to it here at
@@ -269,10 +268,6 @@ float FlipYFromRectToScreen(float y, float rect_height) {
   ns_view_bridge_ = RenderWidgetHostNSViewBridge::Create(
       std::unique_ptr<RenderWidgetHostNSViewClient>(this));
 
-  background_layer_.reset([[CALayer alloc] init]);
-  [cocoa_view() setLayer:background_layer_];
-  [cocoa_view() setWantsLayer:YES];
-
   viz::FrameSinkId frame_sink_id = is_guest_view_hack_
                                        ? AllocateFrameSinkIdForGuestViewHack()
                                        : host()->GetFrameSinkId();
@@ -429,11 +424,11 @@ float FlipYFromRectToScreen(float y, float rect_height) {
 // will enter fullscreen instead.
 void RenderWidgetHostViewMac::InitAsFullscreen(
     RenderWidgetHostView* reference_host_view) {
-  fullscreen_parent_host_view_ =
+  RenderWidgetHostViewMac* parent_view =
       static_cast<RenderWidgetHostViewMac*>(reference_host_view);
   NSWindow* parent_window = nil;
-  if (reference_host_view)
-    parent_window = [reference_host_view->GetNativeView() window];
+  if (parent_view)
+    parent_window = [parent_view->cocoa_view() window];
   NSScreen* screen = [parent_window screen];
   if (!screen)
     screen = [NSScreen mainScreen];
@@ -567,9 +562,8 @@ float FlipYFromRectToScreen(float y, float rect_height) {
 }
 
 void RenderWidgetHostViewMac::Show() {
-  ScopedCAActionDisabler disabler;
-  [cocoa_view() setHidden:NO];
-
+  is_visible_ = true;
+  ns_view_bridge_->SetVisible(is_visible_);
   browser_compositor_->SetRenderWidgetHostIsHidden(false);
 
   ui::LatencyInfo renderer_latency_info;
@@ -590,9 +584,8 @@ float FlipYFromRectToScreen(float y, float rect_height) {
   if (!browser_compositor_)
     return;
 
-  ScopedCAActionDisabler disabler;
-  [cocoa_view() setHidden:YES];
-
+  is_visible_ = false;
+  ns_view_bridge_->SetVisible(is_visible_);
   host()->WasHidden();
   browser_compositor_->SetRenderWidgetHostIsHidden(true);
 }
@@ -691,7 +684,7 @@ float FlipYFromRectToScreen(float y, float rect_height) {
 }
 
 bool RenderWidgetHostViewMac::IsShowing() {
-  return ![cocoa_view() isHidden];
+  return is_visible_;
 }
 
 gfx::Rect RenderWidgetHostViewMac::GetViewBounds() const {
@@ -1526,11 +1519,7 @@ void CombineTextNodesAndMakeCallback(SpeechCallback callback,
   if (color == background_layer_color_)
     return;
   background_layer_color_ = color;
-
-  ScopedCAActionDisabler disabler;
-  base::ScopedCFTypeRef<CGColorRef> cg_color(
-      skia::CGColorCreateFromSkColor(color));
-  [background_layer_ setBackgroundColor:cg_color];
+  ns_view_bridge_->SetBackgroundColor(color);
 }
 
 BrowserAccessibilityManager*