mac: Destroy all ui::Compositors at shutdown

It used to be a valid assumption that ui::Compositors would be destroyed
before BrowserCompositorMac::DisableRecyclingForShutdown was called, but
changes to NSWindows' lifetimes have made that assumption no longer
valid.

Destroy all BrowserCompositorMac instances at that call. Make relevant
methods on RenderWidgetHostViewCocoa and RenderWidgetHostViewMac
early-out when the BrowserCompositorMac is gone.

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

content/browser/renderer_host/browser_compositor_view_mac.h

@@ -30,6 +30,7 @@ class BrowserCompositorMacClient {
   virtual void BrowserCompositorMacOnBeginFrame() = 0;
   virtual void OnFrameTokenChanged(uint32_t frame_token) = 0;
   virtual void DidReceiveFirstFrameAfterNavigation() = 0;
+  virtual void DestroyCompositorForShutdown() = 0;
 };
 
 // This class owns a DelegatedFrameHost, and will dynamically attach and

content/browser/renderer_host/browser_compositor_view_mac.mm

@@ -28,13 +28,15 @@ namespace content {
 
 namespace {
 
-// Set when no browser compositors should remain alive.
-bool g_has_shut_down = false;
-
-// The number of placeholder objects allocated. If this reaches zero, then
-// the RecyclableCompositorMac being held on to for recycling,
-// |g_spare_recyclable_compositors|, will be freed.
-uint32_t g_browser_compositor_count = 0;
+// Weak pointers to all BrowserCompositorMac instances, used to
+// - Determine if a spare RecyclableCompositorMac should be kept around (one
+//   should be only if there exists at least one BrowserCompositorMac).
+// - Force all ui::Compositors to be destroyed at shut-down (because the NSView
+//   signals to shut down will come in very late, long after things that the
+//   ui::Compositor depend on have been destroyed).
+//   https://crbug.com/805726
+base::LazyInstance<std::set<BrowserCompositorMac*>>::Leaky
+    g_browser_compositors;
 
 // A spare RecyclableCompositorMac kept around for recycling.
 base::LazyInstance<base::circular_deque<
@@ -46,7 +48,7 @@ void ReleaseSpareCompositors() {
   while (g_spare_recyclable_compositors.Get().size() > 1)
     g_spare_recyclable_compositors.Get().pop_front();
 
-  if (!g_browser_compositor_count)
+  if (g_browser_compositors.Get().empty())
     g_spare_recyclable_compositors.Get().clear();
 }
 
@@ -69,10 +71,6 @@ class RecyclableCompositorMac : public ui::CompositorObserver {
   // Delete a compositor, or allow it to be recycled.
   static void Recycle(std::unique_ptr<RecyclableCompositorMac> compositor);
 
-  // Indicate that the recyclable compositor should be destroyed, and no future
-  // compositors should be recycled.
-  static void DisableRecyclingForShutdown();
-
   ui::Compositor* compositor() { return &compositor_; }
   ui::AcceleratedWidgetMac* accelerated_widget_mac() {
     return accelerated_widget_mac_.get();
@@ -153,11 +151,6 @@ std::unique_ptr<RecyclableCompositorMac> RecyclableCompositorMac::Create() {
 // static
 void RecyclableCompositorMac::Recycle(
     std::unique_ptr<RecyclableCompositorMac> compositor) {
-  // It is an error to have a browser compositor continue to exist after
-  // shutdown.
-  CHECK(!g_has_shut_down);
-  CHECK(compositor);
-  CHECK(content::ImageTransportFactory::GetInstance());
   content::ImageTransportFactory::GetInstance()
       ->SetCompositorSuspendedForRecycle(compositor->compositor(), true);
 
@@ -183,7 +176,7 @@ BrowserCompositorMac::BrowserCompositorMac(
     : client_(client),
       accelerated_widget_mac_ns_view_(accelerated_widget_mac_ns_view),
       weak_factory_(this) {
-  g_browser_compositor_count += 1;
+  g_browser_compositors.Get().insert(this);
 
   root_layer_.reset(new ui::Layer(ui::LAYER_SOLID_COLOR));
   delegated_frame_host_.reset(new DelegatedFrameHost(
@@ -206,12 +199,12 @@ BrowserCompositorMac::~BrowserCompositorMac() {
   delegated_frame_host_.reset();
   root_layer_.reset();
 
-  DCHECK_GT(g_browser_compositor_count, 0u);
-  g_browser_compositor_count -= 1;
+  size_t num_erased = g_browser_compositors.Get().erase(this);
+  DCHECK_EQ(1u, num_erased);
 
   // If there are no compositors allocated, destroy the recyclable
   // RecyclableCompositorMac.
-  if (!g_browser_compositor_count)
+  if (g_browser_compositors.Get().empty())
     g_spare_recyclable_compositors.Get().clear();
 }
 
@@ -411,7 +404,14 @@ void BrowserCompositorMac::TransitionToState(State new_state) {
 
 // static
 void BrowserCompositorMac::DisableRecyclingForShutdown() {
-  g_has_shut_down = true;
+  // Ensure that the client has destroyed its BrowserCompositorViewMac before
+  // it dependencies are destroyed.
+  // https://crbug.com/805726
+  while (!g_browser_compositors.Get().empty()) {
+    BrowserCompositorMac* browser_compositor =
+        *g_browser_compositors.Get().begin();
+    browser_compositor->client_->DestroyCompositorForShutdown();
+  }
   g_spare_recyclable_compositors.Get().clear();
 }
 

content/browser/renderer_host/render_widget_host_view_mac.h

@@ -493,6 +493,7 @@ class CONTENT_EXPORT RenderWidgetHostViewMac
   void BrowserCompositorMacOnBeginFrame() override;
   void OnFrameTokenChanged(uint32_t frame_token) override;
   void DidReceiveFirstFrameAfterNavigation() override;
+  void DestroyCompositorForShutdown() override;
 
   // AcceleratedWidgetMacNSView implementation.
   NSView* AcceleratedWidgetGetNSView() const override;

content/browser/renderer_host/render_widget_host_view_mac.mm

@@ -401,6 +401,10 @@ void RenderWidgetHostViewMac::DidReceiveFirstFrameAfterNavigation() {
   render_widget_host_->DidReceiveFirstFrameAfterNavigation();
 }
 
+void RenderWidgetHostViewMac::DestroyCompositorForShutdown() {
+  browser_compositor_.reset();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // AcceleratedWidgetMacNSView, public:
 
@@ -729,7 +733,7 @@ RenderWidgetHostViewMac::GetFocusedRenderWidgetHostDelegate() {
 }
 
 void RenderWidgetHostViewMac::UpdateNSViewAndDisplayProperties() {
-  if (!render_widget_host_)
+  if (!browser_compositor_)
     return;
 
   // During auto-resize it is the responsibility of the caller to ensure that
@@ -1729,8 +1733,10 @@ void RenderWidgetHostViewMac::OnGetRenderedTextCompleted(
 }
 
 void RenderWidgetHostViewMac::PauseForPendingResizeOrRepaintsAndDraw() {
-  if (!render_widget_host_ || render_widget_host_->is_hidden())
+  if (!render_widget_host_ || !browser_compositor_ ||
+      render_widget_host_->is_hidden()) {
     return;
+  }
 
   // Pausing for one view prevents others from receiving frames.
   // This may lead to large delays, causing overlaps. See crbug.com/352020.
@@ -3483,6 +3489,12 @@ extern NSString *NSTextInputReplacementRangeAttributeName;
 }
 
 - (void)viewDidMoveToWindow {
+  // This can be called very late during shutdown, so it needs to be guarded by
+  // a check that DestroyCompositorForShutdown has not yet been called.
+  // https://crbug.com/805726
+  if (!renderWidgetHostView_->browser_compositor_)
+    return;
+
   if ([self window])
     [self updateScreenProperties];
   renderWidgetHostView_->browser_compositor_->SetNSViewAttachedToWindow(