mac: Eliminate remaining flashes during navigation

During navigation, in RenderFrameHostManager::CommitPending, when the
navigation results in creation of a new RenderWidgetHostView, the
sequence of events in that function is that
- the old RenderWidgetHostView is hidden
- the new RenderWidgetHostView takes content from the old one
- the new RenderWidgetHostView is shown

On macOS, this can result in a one-frame flicker where there is no
content present (between the hide and the show). Fix this by adding
a ScopedCocoaDisableScreenUpdates for the duration of CommitPending.
This will make all Cocoa changes in the function be atomic.

In order to include ScopedCocoaDisableScreenUpdates in a non-objective-C
file, move its ScopedCocoaDisableScreenUpdates implementation out of
the header file.

Bug: 829523
Change-Id: If541ef8985cf7990d63a79435b179f84339a9372
Reviewed-on: https://chromium-review.googlesource.com/1024968Reviewed-by: ccameron <ccameron@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553628}

content/browser/frame_host/render_frame_host_manager.cc

@@ -18,6 +18,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/stl_util.h"
 #include "base/trace_event/trace_event.h"
+#include "build/build_config.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/devtools/render_frame_devtools_agent_host.h"
 #include "content/browser/frame_host/debug_urls.h"
@@ -51,6 +52,10 @@
 #include "content/public/common/url_constants.h"
 #include "content/public/common/url_utils.h"
 
+#if defined(OS_MACOSX)
+#include "ui/gfx/mac/scoped_cocoa_disable_screen_updates.h"
+#endif  // defined(OS_MACOSX)
+
 namespace content {
 
 RenderFrameHostManager::RenderFrameHostManager(
@@ -2078,6 +2083,17 @@ void RenderFrameHostManager::CommitPending() {
                "FrameTreeNode id", frame_tree_node_->frame_tree_node_id());
   DCHECK(speculative_render_frame_host_);
 
+#if defined(OS_MACOSX)
+  // The old RenderWidgetHostView will be hidden before the new
+  // RenderWidgetHostView takes its contents. Ensure that Cocoa sees this as
+  // a single transaction.
+  // https://crbug.com/829523
+  // TODO(ccameron): This can be removed when the RenderWidgetHostViewMac uses
+  // the same ui::Compositor as MacViews.
+  // https://crbug.com/331669
+  gfx::ScopedCocoaDisableScreenUpdates disabler;
+#endif  // defined(OS_MACOSX)
+
   bool is_main_frame = frame_tree_node_->IsMainFrame();
 
   // First check whether we're going to want to focus the location bar after

ui/gfx/BUILD.gn

@@ -125,6 +125,7 @@ jumbo_component("gfx") {
     "mac/nswindow_frame_controls.h",
     "mac/nswindow_frame_controls.mm",
     "mac/scoped_cocoa_disable_screen_updates.h",
+    "mac/scoped_cocoa_disable_screen_updates.mm",
     "nine_image_painter.cc",
     "nine_image_painter.h",
     "path.cc",

ui/gfx/mac/scoped_cocoa_disable_screen_updates.h

@@ -5,10 +5,8 @@
 #ifndef UI_GFX_MAC_SCOPED_COCOA_DISABLE_SCREEN_UPDATES_H_
 #define UI_GFX_MAC_SCOPED_COCOA_DISABLE_SCREEN_UPDATES_H_
 
-#import <Cocoa/Cocoa.h>
-
-#include "base/mac/mac_util.h"
 #include "base/macros.h"
+#include "ui/gfx/gfx_export.h"
 
 namespace gfx {
 
@@ -17,25 +15,10 @@ namespace gfx {
 // can be nested, and there is a time-maximum (about 1 second) after which
 // Cocoa will automatically re-enable updating. This class doesn't attempt to
 // overrule that.
-class ScopedCocoaDisableScreenUpdates {
+class GFX_EXPORT ScopedCocoaDisableScreenUpdates {
  public:
-  ScopedCocoaDisableScreenUpdates() {
-    if (base::mac::IsAtLeastOS10_11()) {
-      // Beginning with OS X 10.11, [NSAnimationContext beginGrouping] is the
-      // preferred way of disabling screen updates. Use of
-      // NSDisableScreenUpdates() is discouraged.
-      [NSAnimationContext beginGrouping];
-    } else {
-      NSDisableScreenUpdates();
-    }
-  }
-  ~ScopedCocoaDisableScreenUpdates() {
-    if (base::mac::IsAtLeastOS10_11()) {
-      [NSAnimationContext endGrouping];
-    } else {
-      NSEnableScreenUpdates();
-    }
-  }
+  ScopedCocoaDisableScreenUpdates();
+  ~ScopedCocoaDisableScreenUpdates();
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ScopedCocoaDisableScreenUpdates);

ui/gfx/mac/scoped_cocoa_disable_screen_updates.mm

+// Copyright 2018 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 "ui/gfx/mac/scoped_cocoa_disable_screen_updates.h"
+
+#import <Cocoa/Cocoa.h>
+
+#include "base/mac/mac_util.h"
+
+namespace gfx {
+
+ScopedCocoaDisableScreenUpdates::ScopedCocoaDisableScreenUpdates() {
+  if (base::mac::IsAtLeastOS10_11()) {
+    // Beginning with OS X 10.11, [NSAnimationContext beginGrouping] is the
+    // preferred way of disabling screen updates. Use of
+    // NSDisableScreenUpdates() is discouraged.
+    [NSAnimationContext beginGrouping];
+  } else {
+    NSDisableScreenUpdates();
+  }
+}
+
+ScopedCocoaDisableScreenUpdates::~ScopedCocoaDisableScreenUpdates() {
+  if (base::mac::IsAtLeastOS10_11()) {
+    [NSAnimationContext endGrouping];
+  } else {
+    NSEnableScreenUpdates();
+  }
+}
+
+}  // namespace gfx