gpu: Remove WebGraphicsContext3D::makeContextCurrent() in Blink

gpu: Remove WebGraphicsContext3D::makeContextCurrent() in Blink

makeContextCurrent() has two roles;
1. Initialize.
2. Makes the OpenGL context current on the current thread.

#2 is the legacy of WebKit and chromium GPU architecture doesn't require it.
So all #2 usages are removed.

BUG=404121
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181032

[dshwang, 2014-08-15 14:47:27]

Blink side of https://codereview.chromium.org/470973002/

[piman, 2014-08-15 18:10:46]

LGTM if it works with skia.

[jamesr, 2014-08-15 23:38:28]

Where is the initial bind happening?

[Ken Russell (switch to Gerrit), 2014-08-15 23:42:09]

I second James's question.

Also, please send this to the try servers so its correctness can be evaluated.

[jamesr, 2014-08-15 23:57:06]

If you want to say that the initial bind to create happens on the chromium side
that could work, but you'll have to point out where that is.  It also means that
chromium side code is picking which thread the context is bound to, but that
should be OK as well now that chromium knows about all the threads blink might
create (i.e. web worker threads are created chromium side).

[dshwang, 2014-08-16 15:21:28]

I've made this CL depending on crrev.com/470973002, which makes 3d context for
webgl initialized when creating.
So I wasn't willing to kick trybot until crrev.com/470973002 is landed, but
piman@ suggested this CL should be landed first in crrev.com/470973002/#msg19

So I remain the initial bind for webgl as jamesr@ and kbr@ mentioned. Now only
one makeContextCurrent remains in WebGLRenderingContext.cpp

The plan is
1. land this CL
2. land chromium side; crrev.com/470973002
3. remove remaining makeContextCurrent on blink side.

https://codereview.chromium.org/481433002/diff/1/Source/core/html/canvas/WebG...
File Source/core/html/canvas/WebGLRenderingContext.cpp (right):

https://codereview.chromium.org/481433002/diff/1/Source/core/html/canvas/WebG...
Source/core/html/canvas/WebGLRenderingContext.cpp:88:
OwnPtr<blink::WebGraphicsContext3D> context =
adoptPtr(blink::Platform::current()->createOffscreenGraphicsContext3D(attributes,
0));
chromium doesn't do initial bind unlike
createSharedOffscreenGraphicsContext3D().

https://codereview.chromium.org/481433002/diff/20001/Source/platform/graphics...
File Source/platform/graphics/Canvas2DLayerBridge.cpp (right):

https://codereview.chromium.org/481433002/diff/20001/Source/platform/graphics...
Source/platform/graphics/Canvas2DLayerBridge.cpp:64:
OwnPtr<WebGraphicsContext3DProvider> contextProvider =
adoptPtr(Platform::current()->createSharedOffscreenGraphicsContext3DProvider());
chromium already did initial bind in
createSharedOffscreenGraphicsContext3DProvider(). It's why we don't call
makeContextCurrent() here.

[dshwang, 2014-08-26 18:18:36]

Patchset #3 (id:40001) has been deleted

[dshwang, 2014-08-26 18:21:47]

Hi, could you review again?

https://codereview.chromium.org/481433002/diff/60001/Source/core/html/canvas/...
File Source/core/html/canvas/WebGLRenderingContextBase.cpp (right):

https://codereview.chromium.org/481433002/diff/60001/Source/core/html/canvas/...
Source/core/html/canvas/WebGLRenderingContextBase.cpp:5508: if (context &&
context->makeContextCurrent()) {
Currently, this code skipped the first makeContextCurrent() after
Platform::createOffscreenGraphicsContext3D(..), but other conservative
makeContextCurrent() call saves it.
Now we need makeContextCurrent() here.

[Ken Russell (switch to Gerrit), 2014-08-26 21:52:11]

LGTM

The plan to land first this and later crrev.com/470973002 sounds fine. The fact
that RendererWebKitPlatformSupportImpl::createOffscreenGraphicsContext3D
performs the first BindToCurrentThread and returns NULL if it fails is important
to catch race conditions during GPU process re-launching.

Please land both this side and the Chromium side through the CQ and watch the
GPU bots for any sign of destabilization. Please revert if any shows up. Thanks.

[dshwang, 2014-08-27 09:53:42]

On 2014/08/26 21:52:11, Ken Russell wrote:
> LGTM
> 
> The plan to land first this and later crrev.com/470973002 sounds fine. The
fact
> that RendererWebKitPlatformSupportImpl::createOffscreenGraphicsContext3D
> performs the first BindToCurrentThread and returns NULL if it fails is
important
> to catch race conditions during GPU process re-launching.
> 
> Please land both this side and the Chromium side through the CQ and watch the
> GPU bots for any sign of destabilization. Please revert if any shows up.
Thanks.

Thank you for review. I'll keep eyes on bot carefully.

jamesr@, could you review public/platform/WebGraphicsContext3D.h?

[jamesr, 2014-08-27 18:20:57]

lgtm

[dshwang, 2014-08-27 22:04:35]

The CQ bit was checked by dongseong.hwang@intel.com

[commit-bot: I haz the power, 2014-08-27 22:04:53]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/481433002/60001

[commit-bot: I haz the power, 2014-08-27 22:09:55]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/1...)
  mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)
  mac_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)

[commit-bot: I haz the power, 2014-08-27 22:11:08]

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

[commit-bot: I haz the power, 2014-08-27 22:11:09]

Try jobs failed on following builders:
  mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)
  mac_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)

[Ken Russell (switch to Gerrit), 2014-08-27 22:27:12]

On 2014/08/27 22:11:09, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
>
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)
>   mac_gpu_triggered_tests on tryserver.chromium.gpu
>
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)

Sorry about the lack of stack traces on these bots -- I'm working on this right
now on http://crbug.com/405623 -- but I'm pretty sure those failures are both
real and caused by this patch. Can you reproduce them locally?

[dshwang, 2014-08-27 22:32:37]

On 2014/08/27 22:27:12, Ken Russell wrote:
> On 2014/08/27 22:11:09, I haz the power (commit-bot) wrote:
> > Try jobs failed on following builders:
> >   mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
> >
>
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)
> >   mac_gpu_triggered_tests on tryserver.chromium.gpu
> >
>
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)
> 
> Sorry about the lack of stack traces on these bots -- I'm working on this
right
> now on http://crbug.com/405623 -- but I'm pretty sure those failures are both
> real and caused by this patch. Can you reproduce them locally?

Yes, let me try and fix. Thanks.

[dshwang, 2014-08-28 12:34:57]

On 2014/08/27 22:32:37, dshwang wrote:
> On 2014/08/27 22:27:12, Ken Russell wrote:
> > On 2014/08/27 22:11:09, I haz the power (commit-bot) wrote:
> > > Try jobs failed on following builders:
> > >   mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
> > >
> >
>
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)
> > >   mac_gpu_triggered_tests on tryserver.chromium.gpu
> > >
> >
>
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)
> > 
> > Sorry about the lack of stack traces on these bots -- I'm working on this
> right
> > now on http://crbug.com/405623 -- but I'm pretty sure those failures are
both
> > real and caused by this patch. Can you reproduce them locally?
> 
> Yes, let me try and fix. Thanks.

Now, mac bot is fine although I don't change any code. IMO mac bot was in
trouble at that time for some reason.

[dshwang, 2014-08-28 12:35:02]

The CQ bit was checked by dongseong.hwang@intel.com

[commit-bot: I haz the power, 2014-08-28 12:36:05]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/481433002/80001

[commit-bot: I haz the power, 2014-08-28 12:52:04]

Committed patchset #4 (id:80001) as 181032