Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

content/browser/renderer_host/compositor_impl_android.cc

Index: content/browser/renderer_host/compositor_impl_android.cc
diff --git a/content/browser/renderer_host/compositor_impl_android.cc b/content/browser/renderer_host/compositor_impl_android.cc
index a1da9c5c17b256f106f71e5637fa32a6d9e03b50..f5ed672b8600ee65ab8d59ce9b24e8c03cc5673c 100644
--- a/content/browser/renderer_host/compositor_impl_android.cc
+++ b/content/browser/renderer_host/compositor_impl_android.cc
@@ -147,7 +147,7 @@ class OutputSurfaceWithoutParent : public cc::OutputSurface,
         swap_buffers_completion_callback_.callback());
 
     populate_gpu_capabilities_callback_.Run(
-        context_provider_->ContextCapabilities().gpu);
+        context_provider_->ContextCapabilities());
     compositor_->AddObserver(this);
 
     client->SetBeginFrameSource(begin_frame_source_.get());
@@ -418,35 +418,6 @@ void CompositorImpl::SetNeedsComposite() {
   host_->SetNeedsAnimate();
 }
 
-static std::unique_ptr<WebGraphicsContext3DCommandBufferImpl>
-CreateGpuProcessViewContext(
-    const scoped_refptr<gpu::GpuChannelHost>& gpu_channel_host,
-    const gpu::gles2::ContextCreationAttribHelper& attributes,
-    int surface_id) {
-  GURL url("chrome://gpu/Compositor::createContext3D");
-  static const size_t kBytesPerPixel = 4;
-  gfx::DeviceDisplayInfo display_info;
-  size_t full_screen_texture_size_in_bytes =
-      display_info.GetDisplayHeight() *
-      display_info.GetDisplayWidth() *
-      kBytesPerPixel;
-  WebGraphicsContext3DCommandBufferImpl::SharedMemoryLimits limits;
-  limits.command_buffer_size = 64 * 1024;
-  limits.start_transfer_buffer_size = 64 * 1024;
-  limits.min_transfer_buffer_size = 64 * 1024;
-  limits.max_transfer_buffer_size = std::min(
-      3 * full_screen_texture_size_in_bytes, kDefaultMaxTransferBufferSize);
-  limits.mapped_memory_reclaim_limit = 2 * 1024 * 1024;
-  GpuSurfaceTracker* tracker = GpuSurfaceTracker::Get();
-  gpu::SurfaceHandle surface_handle = tracker->GetSurfaceHandle(surface_id);
-  bool share_resources = true;
-  bool automatic_flushes = false;
-  return base::WrapUnique(new WebGraphicsContext3DCommandBufferImpl(
-      surface_handle, url, gpu_channel_host.get(), attributes,
-      gfx::PreferIntegratedGpu, share_resources, automatic_flushes, limits,
-      nullptr));
-}
-
 void CompositorImpl::UpdateLayerTreeHost() {
   client_->UpdateLayerTreeHost();
   if (needs_animate_) {
@@ -549,11 +520,38 @@ void CompositorImpl::CreateOutputSurface() {
   // But from here on just try and always lead to either
   // DidInitializeOutputSurface() or DidFailToInitializeOutputSurface().
   scoped_refptr<gpu::GpuChannelHost> gpu_channel_host(factory->GetGpuChannel());
+
+  GURL url("chrome://gpu/CompositorImpl::CreateOutputSurface");
+  gpu::SurfaceHandle surface_handle =
+      GpuSurfaceTracker::Get()->GetSurfaceHandle(surface_id_);
+  constexpr bool share_resources = true;
+  constexpr bool automatic_flushes = false;
+
+  // TODO(danakj): Why are these limits lower here? This context is used by the

[danakj, 2016/04/19 02:02:11]

Would like feedback here from sievers/boliu

+  // browser compositor (so limits like RenderWidget make sense), but also by
+  // the display compositor and GpuProcessTransportFactory doesn't set limits
+  // on the display compositor context, and we do issue a fair number of draw
+  // commands?

[piman, 2016/04/19 03:22:17]

We should look into it. In the best case, if I counted right, it takes 172 bytes
in the command buffer for a content quad, and contiguous texture quads (for UI
layers) are coalesced, so only take a bit of transfer buffer + about 150 bytes
of command buffer per batch. It takes more if there's a lot of changes between
quad types. But I think this should account for ~200-300 quads total, or 100-150
quads per frame (assuming max pending = 2). It's not huge, but it's not entirely
unreasonable for Android, I think - we should probably measure to check.

+  gpu::SharedMemoryLimits limits;
+  limits.command_buffer_size = 64 * 1024;
+  limits.start_transfer_buffer_size = 64 * 1024;
+  limits.min_transfer_buffer_size = 64 * 1024;
+  constexpr size_t kBytesPerPixel = 4;
+  const size_t full_screen_texture_size_in_bytes =
+      gfx::DeviceDisplayInfo().GetDisplayHeight() *
+      gfx::DeviceDisplayInfo().GetDisplayWidth() * kBytesPerPixel;
+  limits.max_transfer_buffer_size = std::min(
+      3 * full_screen_texture_size_in_bytes, kDefaultMaxTransferBufferSize);
+  // TODO(danakj): Why this limit on the compositor context?

[danakj, 2016/04/19 02:02:11]

And here

[no sievers, 2016/04/19 20:24:13]

We definitely want to limit all shared memory aggressively on Android in
particular on anything with 1GB or less of RAM. And really it comes down to
having hard limits and avoiding shared memory spikes there that cause us to get
OOM-killed.

But some of the reasoning might be outdated. Today:

for command buffer: like piman said, and I think ericrk@ hooked this up to the
memory tracing so we could see if this is too low.

for transfer buffer (texImage2D/texSubImage2D): I think this would affect
UIResource uploading. which is mostly the omnibox and tab placeholders (the rest
should be more static).

(Note that a mapped memory fast path was also added to TexImage2D so it might
actually use that. It's disabled though if you have 1GB or less of RAM, see
gles2_implementation.cc:max_extra_transfer_buffer_size_.)


for mapped memory (mapTexImage2D, async readPixels): This was put as a hard
limit to avoid spikes. And IIRC would only matter if you started for example a
second readback while one is still in flight. But then again: we only use
texture CopyOutputRequests so the place to limit this is actually the GLHelper
context we allocated in RWHVAndroid and which does the async readback.

[danakj, 2016/04/20 00:01:53]

OK I've turned this one into a TODO, and the above one into a set of comments.
PTAL

[no sievers, 2016/04/20 00:06:29]

sounds/looks good. thanks!

+  limits.mapped_memory_reclaim_limit = 2 * 1024 * 1024;
+
   scoped_refptr<ContextProviderCommandBuffer> context_provider(
       new ContextProviderCommandBuffer(
-          CreateGpuProcessViewContext(gpu_channel_host, attributes,
-                                      surface_id_),
-          DISPLAY_COMPOSITOR_ONSCREEN_CONTEXT));
+          base::WrapUnique(new WebGraphicsContext3DCommandBufferImpl(
+              surface_handle, url, gpu_channel_host.get(), attributes,
+              gfx::PreferIntegratedGpu, share_resources, automatic_flushes,
+              nullptr)),
+          limits, DISPLAY_COMPOSITOR_ONSCREEN_CONTEXT));
   DCHECK(context_provider.get());
 
   std::unique_ptr<cc::OutputSurface> real_output_surface(