Remove create/delete methods from WebGraphicsContext3D.

Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org, pfeldman@chromium.org
TBR=pfeldman
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/d8fb03aaed3416b77589c154ae1e9c2c832bfa4b
Cr-Commit-Position: refs/heads/master@{#382506}

Committed: https://crrev.com/8e6d7a72951c1f62377f6f69fab17375c92118e8
Cr-Commit-Position: refs/heads/master@{#382690}

[danakj, 2016-03-19 01:20:37]

Description was changed from

==========
Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org,
pfeldman@chromium.org
BUG=584497
==========

to

==========
Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org,
pfeldman@chromium.org
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[danakj, 2016-03-19 01:21:02]

kbr: please review everything
chrishtr: public/ OWNERS
pfeldman: TBR for test_runner

[chrishtr, 2016-03-19 01:29:08]

lgtm

[piman, 2016-03-19 01:31:26]

piman@chromium.org changed reviewers:
+ piman@chromium.org

[piman, 2016-03-19 01:31:27]

lgtm

[Ken Russell (switch to Gerrit), 2016-03-19 01:46:39]

Thanks for continuing to do this cleanup. LGTM with one slight concern.

https://codereview.chromium.org/1814263004/diff/1/components/test_runner/test...
File components/test_runner/test_plugin.h (right):

https://codereview.chromium.org/1814263004/diff/1/components/test_runner/test...
components/test_runner/test_plugin.h:160: uint32_t color_texture_;
Seems a little strange to use uint32_t for an OpenGL ID (though maybe no
stranger than "unsigned"). Is it not legal to use the GLuint typedef here and in
similar places?

https://codereview.chromium.org/1814263004/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webgl/CHROMIUMValuebuffer.cpp (right):

https://codereview.chromium.org/1814263004/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webgl/CHROMIUMValuebuffer.cpp:27: uint32_t
buffer;
Can we use GLuint here and throughout the Blink code for variables that hold
OpenGL identifiers?

https://codereview.chromium.org/1814263004/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webgl/WebGLFramebuffer.h (right):

https://codereview.chromium.org/1814263004/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webgl/WebGLFramebuffer.h:67: uint32_t object()
const { return m_object; }
Here and for m_object it would be semantically helpful to use GLuint.

[danakj, 2016-03-19 02:51:41]

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

[commit-bot: I haz the power, 2016-03-19 02:51:52]

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

[commit-bot: I haz the power, 2016-03-19 03:37:29]

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

[commit-bot: I haz the power, 2016-03-19 03:37:30]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[danakj, 2016-03-21 22:27:02]

https://codereview.chromium.org/1814263004/diff/1/components/test_runner/test...
File components/test_runner/test_plugin.h (right):

https://codereview.chromium.org/1814263004/diff/1/components/test_runner/test...
components/test_runner/test_plugin.h:160: uint32_t color_texture_;
On 2016/03/19 01:46:39, Ken Russell wrote:
> Seems a little strange to use uint32_t for an OpenGL ID (though maybe no
> stranger than "unsigned"). Is it not legal to use the GLuint typedef here and
in
> similar places?

Done.

https://codereview.chromium.org/1814263004/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webgl/CHROMIUMValuebuffer.cpp (right):

https://codereview.chromium.org/1814263004/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webgl/CHROMIUMValuebuffer.cpp:27: uint32_t
buffer;
On 2016/03/19 01:46:39, Ken Russell wrote:
> Can we use GLuint here and throughout the Blink code for variables that hold
> OpenGL identifiers?

Done.

https://codereview.chromium.org/1814263004/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webgl/WebGLFramebuffer.h (right):

https://codereview.chromium.org/1814263004/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webgl/WebGLFramebuffer.h:67: uint32_t object()
const { return m_object; }
On 2016/03/19 01:46:39, Ken Russell wrote:
> Here and for m_object it would be semantically helpful to use GLuint.

Done.

[danakj, 2016-03-21 22:27:08]

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

[commit-bot: I haz the power, 2016-03-21 22:27:36]

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

[commit-bot: I haz the power, 2016-03-21 22:29:03]

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

[commit-bot: I haz the power, 2016-03-21 22:29:05]

Dry run: Try jobs failed on following builders:
  ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Ken Russell (switch to Gerrit), 2016-03-21 22:29:56]

Thanks for the change to use the GL typedefs. I think that makes the code easier
to understand. LGTM again.

[danakj, 2016-03-21 22:49:27]

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

[commit-bot: I haz the power, 2016-03-21 22:50:00]

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

[danakj, 2016-03-21 23:11:59]

Description was changed from

==========
Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org,
pfeldman@chromium.org
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org,
pfeldman@chromium.org
TBR=pfeldman
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[danakj, 2016-03-21 23:12:55]

Description was changed from

==========
Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org,
pfeldman@chromium.org
TBR=pfeldman
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org,
pfeldman@chromium.org
TBR=pfeldman
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2016-03-21 23:42:26]

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

[commit-bot: I haz the power, 2016-03-21 23:42:28]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[danakj, 2016-03-22 00:05:59]

I can't repro these failures locally, whyyy. I tried enable_oilpan=false. Hmmmm

[danakj, 2016-03-22 00:09:27]

On Mon, Mar 21, 2016 at 5:05 PM, <danakj@chromium.org> wrote:

> I can't repro these failures locally, whyyy. I tried enable_oilpan=false.
> Hmmmm
>

Aha.. oilpan = false + release build. They work in debug ~_~.


>
> https://codereview.chromium.org/1814263004/
>

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

[Ken Russell (switch to Gerrit), 2016-03-22 00:10:59]

On 2016/03/22 00:09:27, danakj wrote:
> On Mon, Mar 21, 2016 at 5:05 PM, <mailto:danakj@chromium.org> wrote:
> 
> > I can't repro these failures locally, whyyy. I tried enable_oilpan=false.
> > Hmmmm
> >
> 
> Aha.. oilpan = false + release build. They work in debug ~_~.

LMK if you need help tracking them down...

[danakj, 2016-03-22 00:13:22]

On Mon, Mar 21, 2016 at 5:10 PM, <kbr@chromium.org> wrote:

> On 2016/03/22 00:09:27, danakj wrote:
> > On Mon, Mar 21, 2016 at 5:05 PM, <mailto:danakj@chromium.org> wrote:
> >
> > > I can't repro these failures locally, whyyy. I tried
> enable_oilpan=false.
> > > Hmmmm
> > >
> >
> > Aha.. oilpan = false + release build. They work in debug ~_~.
>
> LMK if you need help tracking them down...
>

Ah..

    frontColorBufferMailbox->mailbox.allowOverlay =
frontColorBufferMailbox->textureInfo.imageId != 0;

I bet debug builds aren't default initing the variable to 0, and the stub
methods don't do anything at all, but the mock ones used to.

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

[danakj, 2016-03-22 00:16:22]

On Mon, Mar 21, 2016 at 5:05 PM, <danakj@chromium.org> wrote:

> I can't repro these failures locally, whyyy. I tried enable_oilpan=false.
> Hmmmm
>

Aha.. oilpan = false + release build. They work in debug ~_~.


>
> https://codereview.chromium.org/1814263004/
>

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

[danakj, 2016-03-22 00:20:12]

On Mon, Mar 21, 2016 at 5:10 PM, <kbr@chromium.org> wrote:

> On 2016/03/22 00:09:27, danakj wrote:
> > On Mon, Mar 21, 2016 at 5:05 PM, <mailto:danakj@chromium.org> wrote:
> >
> > > I can't repro these failures locally, whyyy. I tried
> enable_oilpan=false.
> > > Hmmmm
> > >
> >
> > Aha.. oilpan = false + release build. They work in debug ~_~.
>
> LMK if you need help tracking them down...
>

Ah..

    frontColorBufferMailbox->mailbox.allowOverlay =
frontColorBufferMailbox->textureInfo.imageId != 0;

I bet debug builds aren't default initing the variable to 0, and the stub
methods don't do anything at all, but the mock ones used to.

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

[danakj, 2016-03-22 00:21:26]

https://codereview.chromium.org/1814263004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp (right):

https://codereview.chromium.org/1814263004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp:281: if
(!newTexture.textureId)
ditto.

This fixes the rest of the tests.

https://codereview.chromium.org/1814263004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp:637: if
(!offscreenColorTexture)
This is one problem. Texture id can't be 0 from GenTextures anyways so it's not
appropriate. But since the stub does nothing, it can be 0 in tests. But it's not
worth branching on. So removing the branch.

This fixes some of the tests.

https://codereview.chromium.org/1814263004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp:986: if
(!textureId) {
ditto

[Ken Russell (switch to Gerrit), 2016-03-22 00:30:30]

https://codereview.chromium.org/1814263004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp (right):

https://codereview.chromium.org/1814263004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp:637: if
(!offscreenColorTexture)
On 2016/03/22 00:21:26, danakj wrote:
> This is one problem. Texture id can't be 0 from GenTextures anyways so it's
not
> appropriate. But since the stub does nothing, it can be 0 in tests. But it's
not
> worth branching on. So removing the branch.
> 
> This fixes some of the tests.

What about the situation where the context's been lost? I'd be concerned about
just deleting the code here and below.

Can the mock that's being used be changed to return a non-zero value for the
Gen* calls?

[danakj, 2016-03-22 00:37:14]

https://codereview.chromium.org/1814263004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp (right):

https://codereview.chromium.org/1814263004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp:637: if
(!offscreenColorTexture)
On 2016/03/22 00:30:30, Ken Russell wrote:
> On 2016/03/22 00:21:26, danakj wrote:
> > This is one problem. Texture id can't be 0 from GenTextures anyways so it's
> not
> > appropriate. But since the stub does nothing, it can be 0 in tests. But it's
> not
> > worth branching on. So removing the branch.
> > 
> > This fixes some of the tests.
> 
> What about the situation where the context's been lost? I'd be concerned about
> just deleting the code here and below.

I don't think that has any impact on our ability to gen ids on the client side.
I am looking and failing to find any "if context is lost" checks in GenTextures
implementations.

There are 3 impls of MakeIds in
https://code.google.com/p/chromium/codesearch#chromium/src/gpu/command_buffer...
which appear to be there the numbers always come from. Am I missing something?

> Can the mock that's being used be changed to return a non-zero value for the
> Gen* calls?

There's one more test that's flaky without doing that change, cuz the same ID
can end up occuring more than once, so I've done that too.

[danakj, 2016-03-22 00:37:28]

PTAL

[danakj, 2016-03-22 00:37:35]

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

[commit-bot: I haz the power, 2016-03-22 00:37:57]

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

[commit-bot: I haz the power, 2016-03-22 00:39:44]

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

[commit-bot: I haz the power, 2016-03-22 00:39:46]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Ken Russell (switch to Gerrit), 2016-03-22 00:43:46]

Still LGTM, but:

https://codereview.chromium.org/1814263004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp
(right):

https://codereview.chromium.org/1814263004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp:173:
static GLuint id = 1;
I think it'd be more reliable to add the most recently allocated texture ID as a
member variable rather than a static local in this function.

[danakj, 2016-03-22 00:47:47]

Thanks!

https://codereview.chromium.org/1814263004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp
(right):

https://codereview.chromium.org/1814263004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp:173:
static GLuint id = 1;
On 2016/03/22 00:43:45, Ken Russell wrote:
> I think it'd be more reliable to add the most recently allocated texture ID as
a
> member variable rather than a static local in this function.

Well, the result should be roughly the same, the difference being that as a
static if multiple contexts are being used, a single context may not give out
sequential ids (it may skip some numbers). And it would let us query the
variable in tests though if we needed to.

All things being equal I prefer it in the method if we don't need to query it,
as it keeps the class definition simpler and limits the scope of the variable
maximally.

[danakj, 2016-03-22 00:48:49]

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

[commit-bot: I haz the power, 2016-03-22 00:49:07]

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

[Ken Russell (switch to Gerrit), 2016-03-22 00:50:37]

https://codereview.chromium.org/1814263004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp
(right):

https://codereview.chromium.org/1814263004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp:173:
static GLuint id = 1;
On 2016/03/22 00:47:47, danakj wrote:
> On 2016/03/22 00:43:45, Ken Russell wrote:
> > I think it'd be more reliable to add the most recently allocated texture ID
as
> a
> > member variable rather than a static local in this function.
> 
> Well, the result should be roughly the same, the difference being that as a
> static if multiple contexts are being used, a single context may not give out
> sequential ids (it may skip some numbers). And it would let us query the
> variable in tests though if we needed to.
> 
> All things being equal I prefer it in the method if we don't need to query it,
> as it keeps the class definition simpler and limits the scope of the variable
> maximally.

OK. I defer to your judgment.

[danakj, 2016-03-22 01:20:00]

The CQ bit was unchecked by danakj@chromium.org

[danakj, 2016-03-22 01:20:08]

The CQ bit was checked by danakj@chromium.org

[danakj, 2016-03-22 01:20:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1814263004/#ps100001
(title: "complex-create: rebase")

[commit-bot: I haz the power, 2016-03-22 01:20:31]

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

[commit-bot: I haz the power, 2016-03-22 04:12:02]

Description was changed from

==========
Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org,
pfeldman@chromium.org
TBR=pfeldman
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org,
pfeldman@chromium.org
TBR=pfeldman
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2016-03-22 04:12:03]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-03-22 04:13:35]

Description was changed from

==========
Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org,
pfeldman@chromium.org
TBR=pfeldman
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org,
pfeldman@chromium.org
TBR=pfeldman
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

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

[commit-bot: I haz the power, 2016-03-22 04:13:36]

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

[Dan Beam, 2016-03-22 04:31:04]

https://codereview.chromium.org/1814263004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp
(right):

https://codereview.chromium.org/1814263004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp:255:
WebGraphicsContext3D* webContext = context();
now unused

https://codereview.chromium.org/1814263004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp:276:
WebGraphicsContext3D* webContext = context();
now unused

[Dan Beam, 2016-03-22 04:37:14]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/1817323003/ by dbeam@chromium.org.

The reason for reverting is: FAILED: [snip]
../../third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp:255:27:
error: unused variable 'webContext' [-Werror,-Wunused-variable]
    WebGraphicsContext3D* webContext = context();
                          ^
../../third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp:276:27:
error: unused variable 'webContext' [-Werror,-Wunused-variable]
    WebGraphicsContext3D* webContext = context();
                          ^
2 errors generated..

[danakj, 2016-03-22 18:10:56]

Description was changed from

==========
Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org,
pfeldman@chromium.org
TBR=pfeldman
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

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

to

==========
Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org,
pfeldman@chromium.org
TBR=pfeldman
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

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

[danakj, 2016-03-22 18:14:17]

The CQ bit was checked by danakj@chromium.org

[danakj, 2016-03-22 18:14:18]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org,
chrishtr@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1814263004/#ps120001
(title: "complex-create: rebase")

[commit-bot: I haz the power, 2016-03-22 18:14:38]

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

[danakj, 2016-03-22 18:16:17]

On 2016/03/22 04:37:14, Dan Beam wrote:
> A revert of this CL (patchset #6 id:100001) has been created in
> https://codereview.chromium.org/1817323003/ by mailto:dbeam@chromium.org.
> 
> The reason for reverting is: FAILED: [snip]
>
../../third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp:255:27:
> error: unused variable 'webContext' [-Werror,-Wunused-variable]
>     WebGraphicsContext3D* webContext = context();
>                           ^
>
../../third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp:276:27:
> error: unused variable 'webContext' [-Werror,-Wunused-variable]
>     WebGraphicsContext3D* webContext = context();
>                           ^
> 2 errors generated..

Filed https://bugs.chromium.org/p/chromium/issues/detail?id=596964 CQ failing to
build the same as waterfall.

[danakj, 2016-03-22 18:18:51]

On 2016/03/22 18:16:17, danakj wrote:
> On 2016/03/22 04:37:14, Dan Beam wrote:
> > A revert of this CL (patchset #6 id:100001) has been created in
> > https://codereview.chromium.org/1817323003/ by mailto:dbeam@chromium.org.
> > 
> > The reason for reverting is: FAILED: [snip]
> >
>
../../third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp:255:27:
> > error: unused variable 'webContext' [-Werror,-Wunused-variable]
> >     WebGraphicsContext3D* webContext = context();
> >                           ^
> >
>
../../third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp:276:27:
> > error: unused variable 'webContext' [-Werror,-Wunused-variable]
> >     WebGraphicsContext3D* webContext = context();
> >                           ^
> > 2 errors generated..
> 
> Filed https://bugs.chromium.org/p/chromium/issues/detail?id=596964 CQ failing
to
> build the same as waterfall.

Oh, this probably just collided with https://codereview.chromium.org/1816623002
which added this code into the compile.

[commit-bot: I haz the power, 2016-03-22 20:06:59]

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

[commit-bot: I haz the power, 2016-03-22 20:07:01]

Failed to apply patch for
third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp:
While running git apply --index -3 -p1;
  error: patch failed:
third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp:618
  Falling back to three-way merge...
  Applied patch to
'third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp' with
conflicts.
  U third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp

Patch:      
third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp
Index: third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp
diff --git
a/third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp
b/third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp
index
44fa3e381414d3e757a6a84ffe8486ac40a13164..2909155514630ea517ca6e191c17146809c6f108
100644
--- a/third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp
+++ b/third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp
@@ -168,26 +168,33 @@ public:
         memcpy(syncToken, &fenceSync, sizeof(fenceSync));
     }
 
-    uint32_t boundTexture() const { return m_boundTexture; }
-    uint32_t boundTextureTarget() const { return m_boundTextureTarget; }
-    uint32_t mostRecentlyWaitedSyncToken() const { return
m_mostRecentlyWaitedSyncToken; }
-    uint32_t currentImageId() const { return m_currentImageId; }
+    void GenTextures(GLsizei n, GLuint* textures) override
+    {
+        static GLuint id = 1;
+        for (GLsizei i = 0; i < n; ++i)
+            textures[i] = id++;
+    }
+
+    GLuint boundTexture() const { return m_boundTexture; }
+    GLuint boundTextureTarget() const { return m_boundTextureTarget; }
+    GLuint mostRecentlyWaitedSyncToken() const { return
m_mostRecentlyWaitedSyncToken; }
+    GLuint currentImageId() const { return m_currentImageId; }
     IntSize mostRecentlyProducedSize() const { return
m_mostRecentlyProducedSize; }
     bool allowImageChromium() const { return m_allowImageChromium; }
 
     void setAllowImageChromium(bool allow) { m_allowImageChromium = allow; }
 
 private:
-    uint32_t m_boundTexture = 0;
-    uint32_t m_boundTextureTarget = 0;
-    uint32_t m_mostRecentlyWaitedSyncToken = 0;
+    GLuint m_boundTexture = 0;
+    GLuint m_boundTextureTarget = 0;
+    GLuint m_mostRecentlyWaitedSyncToken = 0;
     WGC3Dbyte m_currentMailboxByte = 0;
     IntSize m_mostRecentlyProducedSize;
     bool m_allowImageChromium = true;
-    uint32_t m_currentImageId = 1;
-    HashMap<uint32_t, IntSize> m_textureSizes;
-    HashMap<uint32_t, IntSize> m_imageSizes;
-    HashMap<uint32_t, uint32_t> m_imageToTextureMap;
+    GLuint m_currentImageId = 1;
+    HashMap<GLuint, IntSize> m_textureSizes;
+    HashMap<GLuint, IntSize> m_imageSizes;
+    HashMap<GLuint, GLuint> m_imageToTextureMap;
 };
 
 class WebGraphicsContext3DForTests : public MockWebGraphicsContext3D {
@@ -618,22 +625,29 @@ public:
         }
     }
 
-    uint32_t stencilAttachment() const { return m_stencilAttachment; }
-    uint32_t depthAttachment() const { return m_depthAttachment; }
-    uint32_t depthStencilAttachment() const { return m_depthStencilAttachment;
}
+    void GenRenderbuffers(GLsizei n, GLuint* renderbuffers) override
+    {
+        for (GLsizei i = 0; i < n; ++i)
+            renderbuffers[i] = m_nextGenRenderbufferId++;
+    }
+
+    GLuint stencilAttachment() const { return m_stencilAttachment; }
+    GLuint depthAttachment() const { return m_depthAttachment; }
+    GLuint depthStencilAttachment() const { return m_depthStencilAttachment; }
+    size_t numAllocatedRenderBuffer() const { return m_nextGenRenderbufferId -
1; }
 
 private:
-    uint32_t m_depthAttachment = 0;
-    uint32_t m_stencilAttachment = 0;
-    uint32_t m_depthStencilAttachment = 0;
+    GLuint m_nextGenRenderbufferId = 1;
+    GLuint m_depthAttachment = 0;
+    GLuint m_stencilAttachment = 0;
+    GLuint m_depthStencilAttachment = 0;
 };
 
 class DepthStencilTrackingContext : public MockWebGraphicsContext3D {
 public:
-    DepthStencilTrackingContext() : m_nextRenderBufferId(1) {}
     ~DepthStencilTrackingContext() override {}
 
-    int numAllocatedRenderBuffer() const { return m_nextRenderBufferId - 1; }
+    size_t numAllocatedRenderBuffer() const { return
m_contextGL.numAllocatedRenderBuffer(); }
     WebGLId stencilAttachment() const { return m_contextGL.stencilAttachment();
}
     WebGLId depthAttachment() const { return m_contextGL.depthAttachment(); }
     WebGLId depthStencilAttachment() const { return
m_contextGL.depthStencilAttachment(); }
@@ -646,15 +660,9 @@ public:
         return WebString();
     }
 
-    WebGLId createRenderbuffer() override
-    {
-        return ++m_nextRenderBufferId;
-    }
-
     gpu::gles2::GLES2Interface* getGLES2Interface() override { return
&m_contextGL; }
 
 private:
-    WebGLId m_nextRenderBufferId;
     DepthStencilTrackingGLES2Interface m_contextGL;
 };
 
@@ -667,7 +675,7 @@ struct DepthStencilTestCase {
 
     bool requestStencil;
     bool requestDepth;
-    int expectedRenderBuffers;
+    size_t expectedRenderBuffers;
     const char* const testCaseName;
 };

[danakj, 2016-03-22 20:26:22]

The CQ bit was checked by danakj@chromium.org

[danakj, 2016-03-22 20:26:23]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org,
chrishtr@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1814263004/#ps140001
(title: "complex-create: rebase")

[commit-bot: I haz the power, 2016-03-22 20:26:51]

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

[commit-bot: I haz the power, 2016-03-22 21:46:26]

Description was changed from

==========
Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org,
pfeldman@chromium.org
TBR=pfeldman
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

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

to

==========
Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org,
pfeldman@chromium.org
TBR=pfeldman
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

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

[commit-bot: I haz the power, 2016-03-22 21:46:28]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-03-22 21:47:48]

Description was changed from

==========
Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org,
pfeldman@chromium.org
TBR=pfeldman
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

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

to

==========
Remove create/delete methods from WebGraphicsContext3D.

Have callers use the new Gen/Delete methods on GLES2Interface instead.

R=bajones@chromium.org, chrishtr@chromium.org, kbr@chromium.org,
pfeldman@chromium.org
TBR=pfeldman
BUG=584497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/d8fb03aaed3416b77589c154ae1e9c2c832bfa4b
Cr-Commit-Position: refs/heads/master@{#382506}

Committed: https://crrev.com/8e6d7a72951c1f62377f6f69fab17375c92118e8
Cr-Commit-Position: refs/heads/master@{#382690}
==========

[commit-bot: I haz the power, 2016-03-22 21:47:51]

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