Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

This moves the SharedMemoryLimits out to gpu::SharedMemoryLimits in
gpu/command_buffer/client/ and puts the kNoLimit constant in there,
used by GLES2Implementation and MappedMemoryManager.

While doing this we determined that the max_transfer_buffer_usage_bytes
capability on ContextProvider is now unused, so delete it and just
have ContextProvider::ContextCapabilities return gpu::Capabilities
instead.

There is still also a GLInProcessContextSharedMemoryLimits class which
we can merge after this.

R=piman, sievers@chromium.org
TBR=nyquist, sky, raymes
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/6f4e1e255293c50cad768678852d0cb3d5576fdb
Cr-Commit-Position: refs/heads/master@{#388418}

[danakj, 2016-04-19 01:40:31]

Description was changed from

==========
Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

This moves the SharedMemoryLimits out to gpu::SharedMemoryLimits in
gpu/command_buffer/client/ and puts the kNoLimit constant in there,
used by GLES2Implementation and MappedMemoryManager.

While doing this we determined that the max_transfer_buffer_usage_bytes
capability on ContextProvider is now unused, so delete it and just
have ContextProvider::ContextCapabilities return gpu::Capabilities
instead.

There is still also a GLInProcessContextSharedMemoryLimits class which
we can merge after this.

R=piman@chomium.org, sievers@chromium.org
BUG=584497
==========

to

==========
Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

This moves the SharedMemoryLimits out to gpu::SharedMemoryLimits in
gpu/command_buffer/client/ and puts the kNoLimit constant in there,
used by GLES2Implementation and MappedMemoryManager.

While doing this we determined that the max_transfer_buffer_usage_bytes
capability on ContextProvider is now unused, so delete it and just
have ContextProvider::ContextCapabilities return gpu::Capabilities
instead.

There is still also a GLInProcessContextSharedMemoryLimits class which
we can merge after this.

R=piman@chomium.org, sievers@chromium.org
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[danakj, 2016-04-19 01:40:32]

Description was changed from

==========
Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

This moves the SharedMemoryLimits out to gpu::SharedMemoryLimits in
gpu/command_buffer/client/ and puts the kNoLimit constant in there,
used by GLES2Implementation and MappedMemoryManager.

While doing this we determined that the max_transfer_buffer_usage_bytes
capability on ContextProvider is now unused, so delete it and just
have ContextProvider::ContextCapabilities return gpu::Capabilities
instead.

There is still also a GLInProcessContextSharedMemoryLimits class which
we can merge after this.

R=piman@chomium.org, sievers@chromium.org
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

This moves the SharedMemoryLimits out to gpu::SharedMemoryLimits in
gpu/command_buffer/client/ and puts the kNoLimit constant in there,
used by GLES2Implementation and MappedMemoryManager.

While doing this we determined that the max_transfer_buffer_usage_bytes
capability on ContextProvider is now unused, so delete it and just
have ContextProvider::ContextCapabilities return gpu::Capabilities
instead.

There is still also a GLInProcessContextSharedMemoryLimits class which
we can merge after this.

R=piman@chomium.org, sievers@chromium.org
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[danakj, 2016-04-19 01:43:09]

Description was changed from

==========
Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

This moves the SharedMemoryLimits out to gpu::SharedMemoryLimits in
gpu/command_buffer/client/ and puts the kNoLimit constant in there,
used by GLES2Implementation and MappedMemoryManager.

While doing this we determined that the max_transfer_buffer_usage_bytes
capability on ContextProvider is now unused, so delete it and just
have ContextProvider::ContextCapabilities return gpu::Capabilities
instead.

There is still also a GLInProcessContextSharedMemoryLimits class which
we can merge after this.

R=piman@chomium.org, sievers@chromium.org
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

This moves the SharedMemoryLimits out to gpu::SharedMemoryLimits in
gpu/command_buffer/client/ and puts the kNoLimit constant in there,
used by GLES2Implementation and MappedMemoryManager.

While doing this we determined that the max_transfer_buffer_usage_bytes
capability on ContextProvider is now unused, so delete it and just
have ContextProvider::ContextCapabilities return gpu::Capabilities
instead.

There is still also a GLInProcessContextSharedMemoryLimits class which
we can merge after this.

R=piman@chomium.org, sievers@chromium.org
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[danakj, 2016-04-19 01:43:10]

danakj@chromium.org changed reviewers:
+ boliu@chromium.org, fsamuel@google.com

[danakj, 2016-04-19 01:43:29]

danakj@chromium.org changed reviewers:
+ fsamuel@chromium.org
- fsamuel@google.com

[danakj, 2016-04-19 01:44:03]

Description was changed from

==========
Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

This moves the SharedMemoryLimits out to gpu::SharedMemoryLimits in
gpu/command_buffer/client/ and puts the kNoLimit constant in there,
used by GLES2Implementation and MappedMemoryManager.

While doing this we determined that the max_transfer_buffer_usage_bytes
capability on ContextProvider is now unused, so delete it and just
have ContextProvider::ContextCapabilities return gpu::Capabilities
instead.

There is still also a GLInProcessContextSharedMemoryLimits class which
we can merge after this.

R=piman@chomium.org, sievers@chromium.org
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

This moves the SharedMemoryLimits out to gpu::SharedMemoryLimits in
gpu/command_buffer/client/ and puts the kNoLimit constant in there,
used by GLES2Implementation and MappedMemoryManager.

While doing this we determined that the max_transfer_buffer_usage_bytes
capability on ContextProvider is now unused, so delete it and just
have ContextProvider::ContextCapabilities return gpu::Capabilities
instead.

There is still also a GLInProcessContextSharedMemoryLimits class which
we can merge after this.

R=piman, sievers@chromium.org
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[danakj, 2016-04-19 01:44:03]

danakj@chromium.org changed reviewers:
+ piman@chromium.org
- piman@chomium.org

[danakj, 2016-04-19 01:45:30]

piman: please review everything
sievers: please review android stuff
boliu: webview
fsamual: mus

[danakj, 2016-04-19 01:46:03]

There's a few TODOs in here where I found confusing or undocumented things and I
want to clarify.

[boliu, 2016-04-19 01:48:55]

This needs a rebase.
content/browser/android/in_process/context_provider_in_process.* doesn't exist
anymore :)

[danakj, 2016-04-19 01:57:16]

https://codereview.chromium.org/1900993002/diff/20001/content/common/gpu/clie...
File content/common/gpu/client/context_provider_command_buffer.h (right):

https://codereview.chromium.org/1900993002/diff/20001/content/common/gpu/clie...
content/common/gpu/client/context_provider_command_buffer.h:69:
gpu::SharedMemoryLimits memory_limits_;
We could in theory pass this to BindToCurrentThread instead of storing it as a
member from creation, but different cc instances should have different numbers,
so we'd need to add it to cc::LayerTreeSettings then or something.

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

https://codereview.chromium.org/1900993002/diff/90001/blimp/client/feature/co...
File blimp/client/feature/compositor/blimp_context_provider.cc (right):

https://codereview.chromium.org/1900993002/diff/90001/blimp/client/feature/co...
blimp/client/feature/compositor/blimp_context_provider.cc:70: // TODO(danakj):
Why? Is this even valid? This is the CHROMIUM_image extension
I can try investigate this

https://codereview.chromium.org/1900993002/diff/90001/components/mus/surfaces...
File components/mus/surfaces/surfaces_context_provider.cc (right):

https://codereview.chromium.org/1900993002/diff/90001/components/mus/surfaces...
components/mus/surfaces/surfaces_context_provider.cc:28: // TODO(danakj): Wat.
We override this in BindToCurrentThread.
I'm going to try just removing this on TOT and see what breaks.

https://codereview.chromium.org/1900993002/diff/90001/content/browser/rendere...
File content/browser/renderer_host/compositor_impl_android.cc (right):

https://codereview.chromium.org/1900993002/diff/90001/content/browser/rendere...
content/browser/renderer_host/compositor_impl_android.cc:530: // TODO(danakj):
Why are these limits lower here? This context is used by the
Would like feedback here from sievers/boliu

https://codereview.chromium.org/1900993002/diff/90001/content/browser/rendere...
content/browser/renderer_host/compositor_impl_android.cc:545: // TODO(danakj):
Why this limit on the compositor context?
And here

[danakj, 2016-04-19 02:08:41]

On Mon, Apr 18, 2016 at 6:48 PM, <boliu@chromium.org> wrote:

> This needs a rebase.
> content/browser/android/in_process/context_provider_in_process.* doesn't
> exist
> anymore :)
>

Oh cool, done.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

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

lgtm

https://codereview.chromium.org/1900993002/diff/90001/blimp/client/feature/co...
File blimp/client/feature/compositor/blimp_context_provider.cc (right):

https://codereview.chromium.org/1900993002/diff/90001/blimp/client/feature/co...
blimp/client/feature/compositor/blimp_context_provider.cc:70: // TODO(danakj):
Why? Is this even valid? This is the CHROMIUM_image extension
On 2016/04/19 02:02:11, danakj wrote:
> I can try investigate this

It should already be set to true if the GpuMemoryBufferManager is not null (see
InProcessCommandBuffer::Initialize)
If the GMBManger is null, you don't want to use CHROMIUM_image

So yeah, this is suspicious.

https://codereview.chromium.org/1900993002/diff/90001/content/browser/rendere...
File content/browser/renderer_host/compositor_impl_android.cc (right):

https://codereview.chromium.org/1900993002/diff/90001/content/browser/rendere...
content/browser/renderer_host/compositor_impl_android.cc:534: // commands?
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.

[Fady Samuel, 2016-04-19 03:28:31]

mus lgtm. There are parts of mus I don't own here. Pass the rest to sky@ please.

[danakj, 2016-04-19 19:54:50]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-19 19:55:12]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1900993002/90001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1900993002/90001

[danakj, 2016-04-19 19:55:57]

danakj@chromium.org changed reviewers:
+ sky@chromium.org

[danakj, 2016-04-19 19:55:58]

+sky for mojo/ and components/mus/

[commit-bot: I haz the power, 2016-04-19 20:09:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-19 20:09:42]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

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

lgtm

https://codereview.chromium.org/1900993002/diff/90001/blimp/client/feature/co...
File blimp/client/feature/compositor/blimp_context_provider.cc (right):

https://codereview.chromium.org/1900993002/diff/90001/blimp/client/feature/co...
blimp/client/feature/compositor/blimp_context_provider.cc:70: // TODO(danakj):
Why? Is this even valid? This is the CHROMIUM_image extension
On 2016/04/19 03:22:17, piman wrote:
> On 2016/04/19 02:02:11, danakj wrote:
> > I can try investigate this
> 
> It should already be set to true if the GpuMemoryBufferManager is not null
(see
> InProcessCommandBuffer::Initialize)
> If the GMBManger is null, you don't want to use CHROMIUM_image
> 
> So yeah, this is suspicious.

I'm guessing this is either a copy&paste from somewhere or made some DHCECK() go
away or so. And nobody expected this to be actually working yet, see
crbug.com/577265. I'd just try to remove it.

However at this point I'm not sure what Blimp is doing with GLInProcessContext
anyways.

https://codereview.chromium.org/1900993002/diff/90001/content/browser/rendere...
File content/browser/renderer_host/compositor_impl_android.cc (right):

https://codereview.chromium.org/1900993002/diff/90001/content/browser/rendere...
content/browser/renderer_host/compositor_impl_android.cc:545: // TODO(danakj):
Why this limit on the compositor context?
On 2016/04/19 02:02:11, danakj wrote:
> And here

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.

https://codereview.chromium.org/1900993002/diff/90001/gpu/command_buffer/clie...
File gpu/command_buffer/client/shared_memory_limits.h (right):

https://codereview.chromium.org/1900993002/diff/90001/gpu/command_buffer/clie...
gpu/command_buffer/client/shared_memory_limits.h:1: // Copyright (c) 2016 The
Chromium Authors. All rights reserved.
nit: drop '(c)'

[boliu, 2016-04-19 21:20:03]

a_w rs lgtm

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

PTAL at the comments

https://codereview.chromium.org/1900993002/diff/90001/blimp/client/feature/co...
File blimp/client/feature/compositor/blimp_context_provider.cc (right):

https://codereview.chromium.org/1900993002/diff/90001/blimp/client/feature/co...
blimp/client/feature/compositor/blimp_context_provider.cc:70: // TODO(danakj):
Why? Is this even valid? This is the CHROMIUM_image extension
On 2016/04/19 20:24:13, sievers wrote:
> On 2016/04/19 03:22:17, piman wrote:
> > On 2016/04/19 02:02:11, danakj wrote:
> > > I can try investigate this
> > 
> > It should already be set to true if the GpuMemoryBufferManager is not null
> (see
> > InProcessCommandBuffer::Initialize)
> > If the GMBManger is null, you don't want to use CHROMIUM_image
> > 
> > So yeah, this is suspicious.
> 
> I'm guessing this is either a copy&paste from somewhere or made some DHCECK()
go
> away or so. And nobody expected this to be actually working yet, see
> crbug.com/577265. I'd just try to remove it.
> 
> However at this point I'm not sure what Blimp is doing with GLInProcessContext
> anyways.

This is in https://codereview.chromium.org/1898983003/ to remove.

https://codereview.chromium.org/1900993002/diff/90001/components/mus/surfaces...
File components/mus/surfaces/surfaces_context_provider.cc (right):

https://codereview.chromium.org/1900993002/diff/90001/components/mus/surfaces...
components/mus/surfaces/surfaces_context_provider.cc:28: // TODO(danakj): Wat.
We override this in BindToCurrentThread.
On 2016/04/19 02:02:11, danakj wrote:
> I'm going to try just removing this on TOT and see what breaks.

This is gone now.

https://codereview.chromium.org/1900993002/diff/90001/content/browser/rendere...
File content/browser/renderer_host/compositor_impl_android.cc (right):

https://codereview.chromium.org/1900993002/diff/90001/content/browser/rendere...
content/browser/renderer_host/compositor_impl_android.cc:545: // TODO(danakj):
Why this limit on the compositor context?
On 2016/04/19 20:24:13, sievers wrote:
> On 2016/04/19 02:02:11, danakj wrote:
> > And here
> 
> 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.

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

https://codereview.chromium.org/1900993002/diff/90001/gpu/command_buffer/clie...
File gpu/command_buffer/client/shared_memory_limits.h (right):

https://codereview.chromium.org/1900993002/diff/90001/gpu/command_buffer/clie...
gpu/command_buffer/client/shared_memory_limits.h:1: // Copyright (c) 2016 The
Chromium Authors. All rights reserved.
On 2016/04/19 20:24:13, sievers wrote:
> nit: drop '(c)'

Done.

[danakj, 2016-04-20 00:03:03]

danakj@chromium.org changed reviewers:
+ raymes@chromium.org, wez@chromium.org

[danakj, 2016-04-20 00:03:05]

+wez for blimp
+raymes for ppapi

Will TBR for these, and for sky@.

[danakj, 2016-04-20 00:03:32]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-20 00:04:09]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1900993002/130001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1900993002/130001

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

https://codereview.chromium.org/1900993002/diff/90001/content/browser/rendere...
File content/browser/renderer_host/compositor_impl_android.cc (right):

https://codereview.chromium.org/1900993002/diff/90001/content/browser/rendere...
content/browser/renderer_host/compositor_impl_android.cc:545: // TODO(danakj):
Why this limit on the compositor context?
On 2016/04/20 00:01:53, danakj wrote:
> On 2016/04/19 20:24:13, sievers wrote:
> > On 2016/04/19 02:02:11, danakj wrote:
> > > And here
> > 
> > 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.
> 
> OK I've turned this one into a TODO, and the above one into a set of comments.
> PTAL

sounds/looks good. thanks!

[Wez, 2016-04-20 00:09:07]

wez@chromium.org changed reviewers:
+ nyquist@chromium.org

[Wez, 2016-04-20 00:09:08]

Replacing myself with the eminently better qualified Tommy.

[danakj, 2016-04-20 00:13:07]

Description was changed from

==========
Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

This moves the SharedMemoryLimits out to gpu::SharedMemoryLimits in
gpu/command_buffer/client/ and puts the kNoLimit constant in there,
used by GLES2Implementation and MappedMemoryManager.

While doing this we determined that the max_transfer_buffer_usage_bytes
capability on ContextProvider is now unused, so delete it and just
have ContextProvider::ContextCapabilities return gpu::Capabilities
instead.

There is still also a GLInProcessContextSharedMemoryLimits class which
we can merge after this.

R=piman, sievers@chromium.org
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

This moves the SharedMemoryLimits out to gpu::SharedMemoryLimits in
gpu/command_buffer/client/ and puts the kNoLimit constant in there,
used by GLES2Implementation and MappedMemoryManager.

While doing this we determined that the max_transfer_buffer_usage_bytes
capability on ContextProvider is now unused, so delete it and just
have ContextProvider::ContextCapabilities return gpu::Capabilities
instead.

There is still also a GLInProcessContextSharedMemoryLimits class which
we can merge after this.

R=piman, sievers@chromium.org
TBR=nyquist, sky, raymes
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[danakj, 2016-04-20 00:13:15]

The CQ bit was unchecked by danakj@chromium.org

[danakj, 2016-04-20 00:13:21]

The CQ bit was checked by danakj@chromium.org

[danakj, 2016-04-20 00:13:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
fsamuel@chromium.org, piman@chromium.org, sievers@chromium.org,
boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/1900993002/#ps130001
(title: "move-limits: reviews")

[commit-bot: I haz the power, 2016-04-20 00:13:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1900993002/130001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1900993002/130001

[commit-bot: I haz the power, 2016-04-20 00:23:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-20 00:23:47]

Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[danakj, 2016-04-20 00:39:51]

The CQ bit was checked by danakj@chromium.org

[danakj, 2016-04-20 00:39:52]

The patchset sent to the CQ was uploaded after l-g-t-m from boliu@chromium.org,
piman@chromium.org, sievers@chromium.org, fsamuel@chromium.org
Link to the patchset: https://codereview.chromium.org/1900993002/#ps40002
(title: "move-limits: another.gpu.")

[commit-bot: I haz the power, 2016-04-20 00:40:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1900993002/40002
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1900993002/40002

[commit-bot: I haz the power, 2016-04-20 02:02:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-20 02:02:50]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[danakj, 2016-04-20 02:11:20]

https://codereview.chromium.org/1900993002/diff/40002/gpu/command_buffer/clie...
File gpu/command_buffer/client/shared_memory_limits.h (right):

https://codereview.chromium.org/1900993002/diff/40002/gpu/command_buffer/clie...
gpu/command_buffer/client/shared_memory_limits.h:13: size_t command_buffer_size
= 1024 * 1024;
Turns out these sizes don't match what the code in gpu::gles2 is expecting, and
we get narrowing conversions now.

This one should be int32_t which is what CommandBufferHelper takes.

The other four are "unsigned int" in GLES2Implementation, but I made them
uint32_t here cuz unsigned is terrible.

[danakj, 2016-04-20 02:12:15]

The CQ bit was checked by danakj@chromium.org

[danakj, 2016-04-20 02:12:16]

The patchset sent to the CQ was uploaded after l-g-t-m from boliu@chromium.org,
piman@chromium.org, sievers@chromium.org, fsamuel@chromium.org
Link to the patchset: https://codereview.chromium.org/1900993002/#ps160001
(title: "move-limits: types")

[commit-bot: I haz the power, 2016-04-20 02:12:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1900993002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1900993002/160001

[commit-bot: I haz the power, 2016-04-20 03:27:43]

Description was changed from

==========
Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

This moves the SharedMemoryLimits out to gpu::SharedMemoryLimits in
gpu/command_buffer/client/ and puts the kNoLimit constant in there,
used by GLES2Implementation and MappedMemoryManager.

While doing this we determined that the max_transfer_buffer_usage_bytes
capability on ContextProvider is now unused, so delete it and just
have ContextProvider::ContextCapabilities return gpu::Capabilities
instead.

There is still also a GLInProcessContextSharedMemoryLimits class which
we can merge after this.

R=piman, sievers@chromium.org
TBR=nyquist, sky, raymes
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

This moves the SharedMemoryLimits out to gpu::SharedMemoryLimits in
gpu/command_buffer/client/ and puts the kNoLimit constant in there,
used by GLES2Implementation and MappedMemoryManager.

While doing this we determined that the max_transfer_buffer_usage_bytes
capability on ContextProvider is now unused, so delete it and just
have ContextProvider::ContextCapabilities return gpu::Capabilities
instead.

There is still also a GLInProcessContextSharedMemoryLimits class which
we can merge after this.

R=piman, sievers@chromium.org
TBR=nyquist, sky, raymes
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-04-20 03:27:46]

Committed patchset #10 (id:160001)

[sky, 2016-04-20 14:42:28]

LGTM

[commit-bot: I haz the power, 2016-04-22 19:20:33]

Description was changed from

==========
Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

This moves the SharedMemoryLimits out to gpu::SharedMemoryLimits in
gpu/command_buffer/client/ and puts the kNoLimit constant in there,
used by GLES2Implementation and MappedMemoryManager.

While doing this we determined that the max_transfer_buffer_usage_bytes
capability on ContextProvider is now unused, so delete it and just
have ContextProvider::ContextCapabilities return gpu::Capabilities
instead.

There is still also a GLInProcessContextSharedMemoryLimits class which
we can merge after this.

R=piman, sievers@chromium.org
TBR=nyquist, sky, raymes
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

This moves the SharedMemoryLimits out to gpu::SharedMemoryLimits in
gpu/command_buffer/client/ and puts the kNoLimit constant in there,
used by GLES2Implementation and MappedMemoryManager.

While doing this we determined that the max_transfer_buffer_usage_bytes
capability on ContextProvider is now unused, so delete it and just
have ContextProvider::ContextCapabilities return gpu::Capabilities
instead.

There is still also a GLInProcessContextSharedMemoryLimits class which
we can merge after this.

R=piman, sievers@chromium.org
TBR=nyquist, sky, raymes
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/6f4e1e255293c50cad768678852d0cb3d5576fdb
Cr-Commit-Position: refs/heads/master@{#388418}
==========

[commit-bot: I haz the power, 2016-04-22 19:20:36]

Patchset 10 (id:??) landed as
https://crrev.com/6f4e1e255293c50cad768678852d0cb3d5576fdb
Cr-Commit-Position: refs/heads/master@{#388418}