Hide WebGraphicsContext3DCommandBufferImpl.

Hide WebGraphicsContext3DCommandBufferImpl.

This makes the WebGraphicsContext3DCommandBufferImpl inside the
ContextProviderCommandBuffer an implementation detail of the class,
removing it from the constructor. Now callers pass all the interesting
data directly to the ContextProviderCommandBuffer constructor and it
stores them as members, passing them to InitializeOnCurrentThread in
WebGraphicsContext3DCommandBufferImpl when setting up the context.

R=piman@chromium.org, sievers
BUG=584497
Committed: https://crrev.com/ba37f8125d3253958faa590140c7a64c87cd5b8e
Cr-Commit-Position: refs/heads/master@{#391166}

[danakj, 2016-05-03 01:21:28]

Some IWYU fixes in here as well that were needed.

[piman, 2016-05-03 01:44:18]

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

https://codereview.chromium.org/1937313003/diff/20001/content/common/gpu/clie...
content/common/gpu/client/context_provider_command_buffer.cc:115:
surface_handle_, active_url_, channel_.get(), gpu_preference_,
We don't need channel_ after this point, and CommandBufferProxyImpl will keep a
reference. So we could std::move(channel_) ?

https://codereview.chromium.org/1937313003/diff/20001/content/common/gpu/clie...
content/common/gpu/client/context_provider_command_buffer.cc:117:
std::move(share_group), attributes_, context_type_)) {
mmh. This was true before, but we use share_group below. Maybe should stop
std::move'ing it?

[danakj, 2016-05-03 01:56:18]

PTAL

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

https://codereview.chromium.org/1937313003/diff/20001/content/common/gpu/clie...
content/common/gpu/client/context_provider_command_buffer.cc:115:
surface_handle_, active_url_, channel_.get(), gpu_preference_,
On 2016/05/03 01:44:18, piman wrote:
> We don't need channel_ after this point, and CommandBufferProxyImpl will keep
a
> reference. So we could std::move(channel_) ?

Well, all WGC3DCBImpl does with it is channel_->CreateCommandBuffer() so it's a
bit odd to pass a ref here to do that. I'm about to move the whole thing out
here, so how about we defer this until that point. Then we can drop the ref here
if you want.

https://codereview.chromium.org/1937313003/diff/20001/content/common/gpu/clie...
content/common/gpu/client/context_provider_command_buffer.cc:117:
std::move(share_group), attributes_, context_type_)) {
On 2016/05/03 01:44:18, piman wrote:
> mmh. This was true before, but we use share_group below. Maybe should stop
> std::move'ing it?

Gah, yes thanks. We're not checking lost because of this.

[piman, 2016-05-03 02:25:38]

lgtm

[danakj, 2016-05-03 02:26:07]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2016-05-03 02:26:18]

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

[piman, 2016-05-03 02:27:48]

On Mon, May 2, 2016 at 6:56 PM, <danakj@chromium.org> wrote:

> PTAL
>
>
>
>
https://codereview.chromium.org/1937313003/diff/20001/content/common/gpu/clie...
> File content/common/gpu/client/context_provider_command_buffer.cc
> (right):
>
>
>
https://codereview.chromium.org/1937313003/diff/20001/content/common/gpu/clie...
> content/common/gpu/client/context_provider_command_buffer.cc:115:
> surface_handle_, active_url_, channel_.get(), gpu_preference_,
> On 2016/05/03 01:44:18, piman wrote:
> > We don't need channel_ after this point, and CommandBufferProxyImpl
> will keep a
> > reference. So we could std::move(channel_) ?
>
> Well, all WGC3DCBImpl does with it is channel_->CreateCommandBuffer() so
> it's a bit odd to pass a ref here to do that. I'm about to move the
> whole thing out here, so how about we defer this until that point. Then
> we can drop the ref here if you want.
>

Eh, I'm just thinking ahead of myself, I'm working on changing this to
CBPI::Create(scoped_refptr<GpuChannelHost>, ...) at which point it'll make
more sense.


>
>
>
https://codereview.chromium.org/1937313003/diff/20001/content/common/gpu/clie...
> content/common/gpu/client/context_provider_command_buffer.cc:117:
> std::move(share_group), attributes_, context_type_)) {
> On 2016/05/03 01:44:18, piman wrote:
> > mmh. This was true before, but we use share_group below. Maybe should
> stop
> > std::move'ing it?
>
> Gah, yes thanks. We're not checking lost because of this.
>
> https://codereview.chromium.org/1937313003/
>

-- 
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.

[commit-bot: I haz the power, 2016-05-03 03:58:13]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-03 03:59:50]

Description was changed from

==========
Hide WebGraphicsContext3DCommandBufferImpl.

This makes the WebGraphicsContext3DCommandBufferImpl inside the
ContextProviderCommandBuffer an implementation detail of the class,
removing it from the constructor. Now callers pass all the interesting
data directly to the ContextProviderCommandBuffer constructor and it
stores them as members, passing them to InitializeOnCurrentThread in
WebGraphicsContext3DCommandBufferImpl when setting up the context.

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

to

==========
Hide WebGraphicsContext3DCommandBufferImpl.

This makes the WebGraphicsContext3DCommandBufferImpl inside the
ContextProviderCommandBuffer an implementation detail of the class,
removing it from the constructor. Now callers pass all the interesting
data directly to the ContextProviderCommandBuffer constructor and it
stores them as members, passing them to InitializeOnCurrentThread in
WebGraphicsContext3DCommandBufferImpl when setting up the context.

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

Committed: https://crrev.com/ba37f8125d3253958faa590140c7a64c87cd5b8e
Cr-Commit-Position: refs/heads/master@{#391166}
==========

[commit-bot: I haz the power, 2016-05-03 03:59:52]

Patchset 3 (id:??) landed as
https://crrev.com/ba37f8125d3253958faa590140c7a64c87cd5b8e
Cr-Commit-Position: refs/heads/master@{#391166}