Android: Don't destroy LayerTreeHost when Surface goes away

content/browser/renderer_host/compositor_impl_android.cc

 // Copyright (c) 2012 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/renderer_host/compositor_impl_android.h"
 
 #include <android/bitmap.h>
 #include <android/native_window_jni.h>
 
 #include "base/android/jni_android.h"
@@ skipping 209 matching lines @@
 // static
 scoped_ptr<cc::SurfaceIdAllocator> CompositorImpl::CreateSurfaceIdAllocator() {
   scoped_ptr<cc::SurfaceIdAllocator> allocator(
       new cc::SurfaceIdAllocator(++g_surface_id_namespace));
   cc::SurfaceManager* manager = GetSurfaceManager();
   DCHECK(manager);
   allocator->RegisterSurfaceIdNamespace(manager);
   return allocator.Pass();
 }
 
 CompositorImpl::CompositorImpl(CompositorClient* client,

[danakj, 2015/09/28 22:45:43]

Style-guidey comment: This class should probably have an Init method. This is a
very heavy constructor.

[no sievers, 2015/09/30 02:32:40]

ui/compositor/compositor.cc is even heavier though :)
(not that that's a good argument)

                                gfx::NativeWindow root_window)
     : root_layer_(cc::Layer::Create(Compositor::LayerSettings())),
-      resource_manager_(&ui_resource_provider_),
       surface_id_allocator_(GetSurfaceManager() ? CreateSurfaceIdAllocator()
                                                 : nullptr),
       has_transparent_background_(false),
       device_scale_factor_(1),
       window_(NULL),
       surface_id_(0),
       client_(client),
       root_window_(root_window),
       did_post_swapbuffers_(false),
       ignore_schedule_composite_(false),
       needs_composite_(false),
       needs_animate_(false),
       will_composite_immediately_(false),
       composite_on_vsync_trigger_(DO_NOT_COMPOSITE),
       pending_swapbuffers_(0U),
       num_successive_context_creation_failures_(0),
       output_surface_request_pending_(false),
       weak_factory_(this) {
   DCHECK(client);
   DCHECK(root_window);
   root_window->AttachCompositor(this);
+  CreateLayerTreeHost();
+  ui_resource_provider_.reset(new ui::UIResourceProvider(host_.get()));

[danakj, 2015/09/28 22:45:43]

I'm wondering if this class had an Init, and maybe the ui_resource_provider_ had
an init, if you wouldn't need to make these 2 things into scoped_ptrs and they
could be inline members still.

[no sievers, 2015/09/30 02:32:40]

Done.

+  resource_manager_.reset(
+      new ui::ResourceManagerImpl(ui_resource_provider_.get()));
 }
 
 CompositorImpl::~CompositorImpl() {
   root_window_->DetachCompositor();
   // Clean-up any surface references.
   SetSurface(NULL);
 }
 
 void CompositorImpl::PostComposite(CompositingTrigger trigger) {
   DCHECK(needs_composite_);
@@ skipping 97 matching lines @@
   did_post_swapbuffers_ = false;
   host_->Composite(frame_time);
   if (did_post_swapbuffers_)
     pending_swapbuffers_++;
 
   // Need to track vsync to avoid compositing more than once per frame.
   root_window_->RequestVSyncUpdate();
 }
 
 ui::UIResourceProvider& CompositorImpl::GetUIResourceProvider() {
-  return ui_resource_provider_;
+  return *ui_resource_provider_;
 }
 
 ui::ResourceManager& CompositorImpl::GetResourceManager() {
-  return resource_manager_;
+  return *resource_manager_;
 }
 
 void CompositorImpl::SetRootLayer(scoped_refptr<cc::Layer> root_layer) {
   if (subroot_layer_.get()) {
     subroot_layer_->RemoveFromParent();
     subroot_layer_ = NULL;
   }
   if (root_layer.get()) {
     subroot_layer_ = root_layer;
     root_layer_->AddChild(root_layer);
@@ skipping 46 matching lines @@
   if (window) {
     SetWindowSurface(window);
     ANativeWindow_release(window);
     RegisterViewSurface(surface_id_, j_surface.obj());
   }
 }
 
 void CompositorImpl::CreateLayerTreeHost() {
   DCHECK(!host_);
   DCHECK(!WillCompositeThisFrame());
-  needs_composite_ = false;
-  pending_swapbuffers_ = 0;
   cc::LayerTreeSettings settings;
   settings.renderer_settings.refresh_rate = 60.0;
   settings.renderer_settings.allow_antialiasing = false;
   settings.renderer_settings.highp_threshold_min = 2048;
   settings.use_zero_copy = true;
 
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   settings.initial_debug_state.SetRecordRenderingStats(
       command_line->HasSwitch(cc::switches::kEnableGpuBenchmarking));
   settings.initial_debug_state.show_fps_counter =
       command_line->HasSwitch(cc::switches::kUIShowFPSCounter);
   // TODO(enne): Update this this compositor to use the scheduler.
   settings.single_thread_proxy_scheduler = false;
 
   if (command_line->HasSwitch(
           switches::kEnableAndroidCompositorAnimationTimelines))
     settings.use_compositor_animation_timelines = true;
 
   cc::LayerTreeHost::InitParams params;
   params.client = this;
   params.shared_bitmap_manager = HostSharedBitmapManager::current();
   params.gpu_memory_buffer_manager = BrowserGpuMemoryBufferManager::current();
   params.task_graph_runner = g_task_graph_runner.Pointer();
   params.main_task_runner = base::ThreadTaskRunnerHandle::Get();
   params.settings = &settings;
   host_ = cc::LayerTreeHost::CreateSingleThreaded(this, &params);
   host_->SetRootLayer(root_layer_);
-
-  host_->SetVisible(true);
+  host_->SetVisible(false);
   host_->SetLayerTreeHostClientReady();
   host_->SetViewportSize(size_);
   host_->set_has_transparent_background(has_transparent_background_);
   host_->SetDeviceScaleFactor(device_scale_factor_);
 
   if (needs_animate_)
     host_->SetNeedsAnimate();
 }
 
 void CompositorImpl::SetVisible(bool visible) {
   TRACE_EVENT1("cc", "CompositorImpl::SetVisible", "visible", visible);
   if (!visible) {
     DCHECK(host_);
     // Look for any layers that were attached to the root for readback
     // and are waiting for Composite() to happen.
     bool readback_pending = false;
     for (size_t i = 0; i < root_layer_->children().size(); ++i) {
       if (root_layer_->children()[i]->HasCopyRequest()) {
         readback_pending = true;
         break;
       }
     }
     if (readback_pending) {
       ignore_schedule_composite_ = true;
       host_->Composite(base::TimeTicks::Now());
       ignore_schedule_composite_ = false;
     }
     if (WillComposite())
       CancelComposite();
-    ui_resource_provider_.SetLayerTreeHost(NULL);
-    host_.reset();
+    host_->SetVisible(false);
+    host_->ReleaseOutputSurface();
+    pending_swapbuffers_ = 0;

[danakj, 2015/09/28 22:45:43]

I don't really know what these are about, but these were in CreateLTH which set
visible true. Now they are in a set visible false path. Why is that right?

[no sievers, 2015/09/30 02:32:40]

We just have to make sure the accounting doesn't break. If the output surface
gets deleted we don't get anymore pending acks. Now one corner case is that
|pending_swapbuffers_ = kMaxSwapBuffers| and we destroy the OS (or previously
LTH). When we become visible again, we then might bail out of
CompositorImpl::Composite() forever below (line 339) before ever getting a
RequestNewOutputSurface() if we don't reset the pending acks (before
Composite()) somewhere.

I don't think it matters whether we reset it on becoming invisible or visible
again, since it shouldn't be able to change if there is no OutputSurface,
because DidPostSwapBuffers() wouldn't get called even if we tried to force a
composite. Thinking more about it, we probably also want to avoid any failed
attempt to create an OutputSurface while we are invisible and don't have an
underlying surface. (Also there is a DCHECK() for the surface id in
CreateOutputSurface()). Since we cancel |current_composite_task_| here, I can
actually put a DCHECK(!host_->visible()) inside Composite().

Does that add up? (Thanks for forcing me to think it through thoroughly.)

+    needs_composite_ = false;
+    ui_resource_provider_->OnUIResourcesEvicted();
     establish_gpu_channel_timeout_.Stop();
     output_surface_request_pending_ = false;
     display_client_.reset();
     if (current_composite_task_) {
       current_composite_task_->Cancel();
       current_composite_task_.reset();
     }
-  } else if (!host_) {
-    CreateLayerTreeHost();
-    ui_resource_provider_.SetLayerTreeHost(host_.get());
+  } else {
+    host_->SetVisible(true);
+    SetNeedsComposite();
   }
 }
 
 void CompositorImpl::setDeviceScaleFactor(float factor) {
   device_scale_factor_ = factor;
   if (host_)
     host_->SetDeviceScaleFactor(factor);
 }
 
 void CompositorImpl::SetWindowBounds(const gfx::Size& size) {
   if (size_ == size)
     return;
 
   size_ = size;
   if (host_)
     host_->SetViewportSize(size);
   if (display_client_)
     display_client_->display()->Resize(size);
   root_layer_->SetBounds(size);
 }
 
 void CompositorImpl::SetHasTransparentBackground(bool flag) {
   has_transparent_background_ = flag;
   if (host_)
     host_->set_has_transparent_background(flag);
 }
 
 void CompositorImpl::SetNeedsComposite() {
-  if (!host_.get())
+  if (!host_->visible())
     return;
   DCHECK(!needs_composite_ || WillComposite());
 
   needs_composite_ = true;
   PostComposite(COMPOSITE_IMMEDIATELY);
 }
 
 static scoped_ptr<WebGraphicsContext3DCommandBufferImpl>
 CreateGpuProcessViewContext(
     const scoped_refptr<GpuChannelHost>& gpu_channel_host,
@@ skipping 125 matching lines @@
     surface_output_surface->set_display_client(display_client_.get());
     display_client_->display()->Resize(size_);
     host_->SetOutputSurface(surface_output_surface.Pass());
   } else {
     host_->SetOutputSurface(real_output_surface.Pass());
   }
 }
 
 void CompositorImpl::PopulateGpuCapabilities(
     gpu::Capabilities gpu_capabilities) {
-  ui_resource_provider_.SetSupportsETC1NonPowerOfTwo(
+  ui_resource_provider_->SetSupportsETC1NonPowerOfTwo(
       gpu_capabilities.texture_format_etc1_npot);
 }
 
 void CompositorImpl::AddObserver(VSyncObserver* observer) {
   observer_list_.AddObserver(observer);
 }
 
 void CompositorImpl::RemoveObserver(VSyncObserver* observer) {
   observer_list_.RemoveObserver(observer);
 }
 
 void CompositorImpl::ScheduleComposite() {
-  DCHECK(!needs_composite_ || WillComposite());
-  if (ignore_schedule_composite_)
+  if (ignore_schedule_composite_ || !host_->visible())
     return;
 
+  DCHECK_IMPLIES(needs_composite_, WillComposite());
   needs_composite_ = true;
   // We currently expect layer tree invalidations at most once per frame
   // during normal operation and therefore try to composite immediately
   // to minimize latency.
   PostComposite(COMPOSITE_IMMEDIATELY);
 }
 
 void CompositorImpl::ScheduleAnimation() {
-  DCHECK(!needs_composite_ || WillComposite());
+  DCHECK_IMPLIES(needs_composite_, WillComposite());
   needs_animate_ = true;
 
   if (needs_composite_)
     return;
 
   TRACE_EVENT0("cc", "CompositorImpl::ScheduleAnimation");
   needs_composite_ = true;
   PostComposite(COMPOSITE_EVENTUALLY);
 }
 
@@ skipping 53 matching lines @@
     composite_on_vsync_trigger_ = DO_NOT_COMPOSITE;
     PostComposite(trigger);
   }
 
   FOR_EACH_OBSERVER(VSyncObserver, observer_list_,
                     OnUpdateVSyncParameters(frame_time, vsync_period));
 }
 
 void CompositorImpl::SetNeedsAnimate() {
   needs_animate_ = true;
-  if (!host_)
+  if (!host_->visible())
     return;
 
   host_->SetNeedsAnimate();
 }
 
 }  // namespace content