De-jank window resizing in MacViews (part 2).

Part 1: https://chromium-review.googlesource.com/c/chromium/src/+/1066304

This change picks up from the above CL and adds hooks into the GPU process to
coordinate its CATransaction with the browser's on request (e.g. for resize).

New GPU IPCs let the browser process, via CATransactionObserver, begin a
transaction and then end it while in the post commit phase of its
CATransaction. This makes compositor changes display atomically with changes in
the browser process. It also fixes the window frame jank described in the above
CL when the GPU process assigns new IOSurfaces as CALayer contents.

I think this is because the two CATransactions happen as one from the window
server's point of view: in a test app, committing the two transactions like
this significantly reduced the time to commit.

Bug: 837660
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Ib5b2a3a3cae76640cc9872b7f265cb3bd13143a4
Reviewed-on: https://chromium-review.googlesource.com/1077047Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565687}

components/viz/host/server_gpu_memory_buffer_manager_unittest.cc

@@ -129,6 +129,12 @@ class TestGpuService : public mojom::GpuService {
 
   void OnForegrounded() override {}
 
+#if defined(OS_MACOSX)
+  void BeginCATransaction() override {}
+
+  void CommitCATransaction(CommitCATransactionCallback callback) override {}
+#endif
+
   void Crash() override {}
 
   void Hang() override {}

components/viz/service/gl/gpu_service_impl.cc

@@ -72,6 +72,10 @@
 #include "gpu/ipc/service/direct_composition_surface_win.h"
 #endif
 
+#if defined(OS_MACOSX)
+#include "ui/base/cocoa/quartz_util.h"
+#endif
+
 namespace viz {
 
 namespace {
@@ -91,14 +95,16 @@ bool GpuLogMessageHandler(int severity,
 
 // Returns a callback which does a PostTask to run |callback| on the |runner|
 // task runner.
-template <typename Param>
-base::OnceCallback<void(const Param&)> WrapCallback(
+template <typename... Params>
+base::OnceCallback<void(Params&&...)> WrapCallback(
     scoped_refptr<base::SingleThreadTaskRunner> runner,
-    base::OnceCallback<void(const Param&)> callback) {
+    base::OnceCallback<void(Params...)> callback) {
   return base::BindOnce(
       [](base::SingleThreadTaskRunner* runner,
-         base::OnceCallback<void(const Param&)> callback, const Param& param) {
-        runner->PostTask(FROM_HERE, base::BindOnce(std::move(callback), param));
+         base::OnceCallback<void(Params && ...)> callback, Params&&... params) {
+        runner->PostTask(FROM_HERE,
+                         base::BindOnce(std::move(callback),
+                                        std::forward<Params>(params)...));
       },
       base::RetainedRef(std::move(runner)), std::move(callback));
 }
@@ -765,6 +771,20 @@ void GpuServiceImpl::OnForegrounded() {
     watchdog_thread_->OnForegrounded();
 }
 
+#if defined(OS_MACOSX)
+void GpuServiceImpl::BeginCATransaction() {
+  DCHECK(io_runner_->BelongsToCurrentThread());
+  main_runner_->PostTask(FROM_HERE, base::BindOnce(&ui::BeginCATransaction));
+}
+
+void GpuServiceImpl::CommitCATransaction(CommitCATransactionCallback callback) {
+  DCHECK(io_runner_->BelongsToCurrentThread());
+  main_runner_->PostTaskAndReply(FROM_HERE,
+                                 base::BindOnce(&ui::CommitCATransaction),
+                                 WrapCallback(io_runner_, std::move(callback)));
+}
+#endif
+
 void GpuServiceImpl::Crash() {
   DCHECK(io_runner_->BelongsToCurrentThread());
   DVLOG(1) << "GPU: Simulating GPU crash";

components/viz/service/gl/gpu_service_impl.h

@@ -208,6 +208,10 @@ class VIZ_SERVICE_EXPORT GpuServiceImpl : public gpu::GpuChannelManagerDelegate,
   void OnBackgroundCleanup() override;
   void OnBackgrounded() override;
   void OnForegrounded() override;
+#if defined(OS_MACOSX)
+  void BeginCATransaction() override;
+  void CommitCATransaction(CommitCATransactionCallback callback) override;
+#endif
   void Crash() override;
   void Hang() override;
   void ThrowJavaException() override;

content/browser/BUILD.gn

@@ -1846,6 +1846,13 @@ jumbo_source_set("browser") {
     deps += [ "//printing" ]
   }
 
+  if (is_mac) {
+    sources += [
+      "gpu/ca_transaction_gpu_coordinator.cc",
+      "gpu/ca_transaction_gpu_coordinator.h",
+    ]
+  }
+
   if (!is_mac) {
     deps += [ "//sandbox" ]
   }

content/browser/gpu/ca_transaction_gpu_coordinator.cc

+// 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 "content/browser/gpu/ca_transaction_gpu_coordinator.h"
+
+#include "base/cancelable_callback.h"
+#include "content/browser/gpu/gpu_process_host.h"
+#include "content/public/browser/browser_thread.h"
+#include "services/viz/privileged/interfaces/gl/gpu_service.mojom.h"
+#include "ui/accelerated_widget_mac/ca_transaction_observer.h"
+
+namespace content {
+
+CATransactionGPUCoordinator::CATransactionGPUCoordinator(GpuProcessHost* host)
+    : host_(host) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::BindOnce(&ui::CATransactionCoordinator::AddPostCommitObserver,
+                     base::Unretained(&ui::CATransactionCoordinator::Get()),
+                     base::RetainedRef(this)));
+}
+
+CATransactionGPUCoordinator::~CATransactionGPUCoordinator() {
+  DCHECK(!host_);
+}
+
+void CATransactionGPUCoordinator::HostWillBeDestroyed() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::BindOnce(&ui::CATransactionCoordinator::RemovePostCommitObserver,
+                     base::Unretained(&ui::CATransactionCoordinator::Get()),
+                     base::RetainedRef(this)));
+  host_ = nullptr;
+}
+
+void CATransactionGPUCoordinator::OnActivateForTransaction() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::BindOnce(&CATransactionGPUCoordinator::OnActivateForTransactionOnIO,
+                     this));
+}
+
+void CATransactionGPUCoordinator::OnEnterPostCommit() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  // If HostWillBeDestroyed() is called during a commit, pending_commit_count_
+  // may be left non-zero. That's fine as long as this instance is destroyed
+  // (and removed from the list of post-commit observers) soon after.
+  pending_commit_count_++;
+
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::BindOnce(&CATransactionGPUCoordinator::OnEnterPostCommitOnIO,
+                     this));
+}
+
+bool CATransactionGPUCoordinator::ShouldWaitInPostCommit() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  return pending_commit_count_ > 0;
+}
+
+void CATransactionGPUCoordinator::OnActivateForTransactionOnIO() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (host_)
+    host_->gpu_service()->BeginCATransaction();
+}
+
+void CATransactionGPUCoordinator::OnEnterPostCommitOnIO() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (host_)
+    host_->gpu_service()->CommitCATransaction(base::BindOnce(
+        &CATransactionGPUCoordinator::OnCommitCompletedOnIO, this));
+}
+
+void CATransactionGPUCoordinator::OnCommitCompletedOnIO() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::BindOnce(&CATransactionGPUCoordinator::OnCommitCompletedOnUI,
+                     this));
+}
+
+void CATransactionGPUCoordinator::OnCommitCompletedOnUI() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  pending_commit_count_--;
+}
+
+}  // namespace content

content/browser/gpu/ca_transaction_gpu_coordinator.h

+// 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.
+
+#ifndef CONTENT_BROWSER_GPU_CA_TRANSACTION_GPU_COORDINATOR_H_
+#define CONTENT_BROWSER_GPU_CA_TRANSACTION_GPU_COORDINATOR_H_
+
+#include "base/memory/scoped_refptr.h"
+#include "base/memory/weak_ptr.h"
+#include "ui/accelerated_widget_mac/ca_transaction_observer.h"
+
+#include <memory>
+
+namespace content {
+
+class GpuProcessHost;
+
+// Synchronizes CATransaction commits between the browser and GPU processes.
+class CATransactionGPUCoordinator
+    : public base::RefCountedThreadSafe<CATransactionGPUCoordinator>,
+      public ui::CATransactionCoordinator::PostCommitObserver {
+ public:
+  CATransactionGPUCoordinator(GpuProcessHost* host);
+
+  void HostWillBeDestroyed();
+
+ private:
+  friend class base::RefCountedThreadSafe<CATransactionGPUCoordinator>;
+  virtual ~CATransactionGPUCoordinator();
+
+  // ui::CATransactionObserver implementation
+  void OnActivateForTransaction() override;
+  void OnEnterPostCommit() override;
+  bool ShouldWaitInPostCommit() override;
+
+  void OnActivateForTransactionOnIO();
+  void OnEnterPostCommitOnIO();
+  void OnCommitCompletedOnIO();
+  void OnCommitCompletedOnUI();
+
+  GpuProcessHost* host_;  // weak
+  int pending_commit_count_ = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(CATransactionGPUCoordinator);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_GPU_CA_TRANSACTION_GPU_COORDINATOR_H_

content/browser/gpu/gpu_process_host.cc

@@ -110,6 +110,10 @@
 #include "gpu/ipc/common/gpu_surface_tracker.h"
 #endif
 
+#if defined(OS_MACOSX)
+#include "content/browser/gpu/ca_transaction_gpu_coordinator.h"
+#endif
+
 namespace content {
 
 // UMA histogram names.
@@ -685,6 +689,10 @@ GpuProcessHost::GpuProcessHost(int host_id, GpuProcessKind kind)
       kind_(kind),
       process_launched_(false),
       status_(UNKNOWN),
+#if defined(OS_MACOSX)
+      ca_transaction_gpu_coordinator_(
+          base::MakeRefCounted<CATransactionGPUCoordinator>(this)),
+#endif
       gpu_host_binding_(this),
       weak_ptr_factory_(this) {
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ -711,6 +719,11 @@ GpuProcessHost::~GpuProcessHost() {
 
   SendOutstandingReplies(EstablishChannelStatus::GPU_HOST_INVALID);
 
+#if defined(OS_MACOSX)
+  ca_transaction_gpu_coordinator_->HostWillBeDestroyed();
+  ca_transaction_gpu_coordinator_ = nullptr;
+#endif
+
   if (status_ == UNKNOWN) {
     RunRequestGPUInfoCallbacks(gpu::GPUInfo());
   } else {

content/browser/gpu/gpu_process_host.h

@@ -55,6 +55,10 @@ struct SyncToken;
 namespace content {
 class BrowserChildProcessHostImpl;
 
+#if defined(OS_MACOSX)
+class CATransactionGPUCoordinator;
+#endif
+
 class GpuProcessHost : public BrowserChildProcessHostDelegate,
                        public IPC::Sender,
                        public viz::mojom::GpuHost {
@@ -310,6 +314,10 @@ class GpuProcessHost : public BrowserChildProcessHostDelegate,
   std::unique_ptr<BrowserChildProcessHostImpl> process_;
   std::unique_ptr<base::Thread> in_process_gpu_thread_;
 
+#if defined(OS_MACOSX)
+  scoped_refptr<CATransactionGPUCoordinator> ca_transaction_gpu_coordinator_;
+#endif
+
   // Track the URLs of the pages which have live offscreen contexts,
   // assumed to be associated with untrusted content such as WebGL.
   // For best robustness, when any context lost notification is

services/viz/privileged/interfaces/gl/gpu_service.mojom

@@ -106,6 +106,15 @@ interface GpuService {
   // Called by the browser immediately after the application is foregrounded.
   OnForegrounded();
 
+  // Begin a batch of layer tree changes.
+  [EnableIf=is_mac]
+  BeginCATransaction();
+
+  // Commit a batch of layer tree changes atomically. Returns after the commit
+  // completes.
+  [EnableIf=is_mac]
+  CommitCATransaction() => ();
+
   Crash();
   Hang();
   ThrowJavaException();

ui/accelerated_widget_mac/ca_transaction_observer.h

@@ -60,10 +60,7 @@ class ACCELERATED_WIDGET_MAC_EXPORT CATransactionCoordinator {
 
   static CATransactionCoordinator& Get();
 
-  void Synchronize() {
-    if (@available(macos 10.11, *))
-      SynchronizeImpl();
-  }
+  void Synchronize();
 
   void AddPreCommitObserver(PreCommitObserver*);
   void RemovePreCommitObserver(PreCommitObserver*);

ui/accelerated_widget_mac/ca_transaction_observer.mm

@@ -8,6 +8,7 @@
 #include "base/trace_event/trace_event.h"
 
 #import <AppKit/AppKit.h>
+#import <CoreFoundation/CoreFoundation.h>
 #import <QuartzCore/QuartzCore.h>
 
 typedef enum {
@@ -25,6 +26,7 @@ API_AVAILABLE(macos(10.11))
 namespace ui {
 
 namespace {
+NSString* kRunLoopMode = @"Chrome CATransactionCoordinator commit handler";
 constexpr auto kPostCommitTimeout = base::TimeDelta::FromMilliseconds(50);
 }  // namespace
 
@@ -34,6 +36,12 @@ CATransactionCoordinator& CATransactionCoordinator::Get() {
 }
 
 void CATransactionCoordinator::SynchronizeImpl() {
+  static bool registeredRunLoopMode = false;
+  if (!registeredRunLoopMode) {
+    CFRunLoopAddCommonMode(CFRunLoopGetCurrent(),
+                           static_cast<CFStringRef>(kRunLoopMode));
+    registeredRunLoopMode = true;
+  }
   if (active_)
     return;
   active_ = true;
@@ -55,8 +63,7 @@ void CATransactionCoordinator::SynchronizeImpl() {
           [start_date dateByAddingTimeInterval:timeout.InSecondsF()];
       if ([deadline isLessThanOrEqualTo:[NSDate date]])
         break;
-      [NSRunLoop.currentRunLoop runMode:NSDefaultRunLoopMode
-                             beforeDate:deadline];
+      [NSRunLoop.currentRunLoop runMode:kRunLoopMode beforeDate:deadline];
     }
   }
                          forPhase:kCATransactionPhasePreCommit];
@@ -76,8 +83,7 @@ void CATransactionCoordinator::SynchronizeImpl() {
         break;
       if ([deadline isLessThanOrEqualTo:[NSDate date]])
         break;
-      [NSRunLoop.currentRunLoop runMode:NSDefaultRunLoopMode
-                             beforeDate:deadline];
+      [NSRunLoop.currentRunLoop runMode:kRunLoopMode beforeDate:deadline];
     }
     active_ = false;
   }
@@ -87,6 +93,11 @@ void CATransactionCoordinator::SynchronizeImpl() {
 CATransactionCoordinator::CATransactionCoordinator() = default;
 CATransactionCoordinator::~CATransactionCoordinator() = default;
 
+void CATransactionCoordinator::Synchronize() {
+  if (@available(macos 10.11, *))
+    SynchronizeImpl();
+}
+
 void CATransactionCoordinator::AddPreCommitObserver(
     PreCommitObserver* observer) {
   pre_commit_observers_.AddObserver(observer);

ui/base/BUILD.gn

@@ -137,6 +137,8 @@ component("base") {
     "cocoa/nsgraphics_context_additions.mm",
     "cocoa/nsview_additions.h",
     "cocoa/nsview_additions.mm",
+    "cocoa/quartz_util.h",
+    "cocoa/quartz_util.mm",
     "cocoa/remote_layer_api.h",
     "cocoa/remote_layer_api.mm",
     "cocoa/scoped_cg_context_smooth_fonts.h",
@@ -584,6 +586,7 @@ component("base") {
     libs += [
       "Accelerate.framework",
       "AppKit.framework",
+      "QuartzCore.framework",
       "AudioUnit.framework",
       "Carbon.framework",
       "CoreVideo.framework",

ui/base/cocoa/quartz_util.h

+// 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.
+
+#ifndef UI_BASE_COCOA_QUARTZ_UTIL_H_
+#define UI_BASE_COCOA_QUARTZ_UTIL_H_
+
+#include "ui/base/ui_base_export.h"
+
+namespace ui {
+
+// Calls +[CATransaction begin].
+UI_BASE_EXPORT void BeginCATransaction();
+
+// Calls +[CATransaction commit].
+UI_BASE_EXPORT void CommitCATransaction();
+
+}  // namespace ui
+
+#endif  // UI_BASE_COCOA_QUARTZ_UTIL_H_

ui/base/cocoa/quartz_util.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/base/cocoa/quartz_util.h"
+
+#include <QuartzCore/QuartzCore.h>
+
+namespace ui {
+
+void BeginCATransaction() {
+  [CATransaction begin];
+}
+
+void CommitCATransaction() {
+  [CATransaction commit];
+}
+
+}  // namespace ui