Remove WebGraphicsContext3D reason from the CauseForGpuLaunch enum

Remove WebGraphicsContext3D reason from the CauseForGpuLaunch enum

Replace "WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE" with a bunch
of more specific and real reasons.

R=piman
BUG=584497
Committed: https://crrev.com/3873e857d41721b7497c993f042c704f1243b9f3
Cr-Commit-Position: refs/heads/master@{#391721}

[danakj, 2016-05-03 22:24:58]

This is used for raster worker context too..

[danakj, 2016-05-03 22:25:19]

On 2016/05/03 22:24:58, danakj wrote:
> This is used for raster worker context too..

And for SharedMainThreadContextProvider..

[danakj, 2016-05-03 22:25:22]

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

[danakj, 2016-05-03 22:25:45]

On 2016/05/03 22:25:19, danakj wrote:
> On 2016/05/03 22:24:58, danakj wrote:
> > This is used for raster worker context too..
> 
> And for SharedMainThreadContextProvider..

I guess what I'm saying is idk what to call this.

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

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

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

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

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

Dry run: Try jobs failed on following builders:
  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...)

[danakj, 2016-05-03 22:34:13]

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

[commit-bot: I haz the power, 2016-05-03 22:34:37]

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

[danakj, 2016-05-03 22:44:48]

Oh, this also is in histograms.xml

  <int value="4" label="WebGraphicsContext3DCommandBufferImpl Initialize">      
                                                                          
    CAUSE_FOR_GPU_LAUNCH_WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE       
                                                                          
  </int>


So.. maybe we should just add some new ones and stop using this one? WDYT?

[piman, 2016-05-03 23:05:32]

lgtm

[Ken Russell (switch to Gerrit), 2016-05-03 23:12:41]

Description was changed from

==========
Rename the CauseForGpuLaunch from WebGraphicsContext3D.

Rename "WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE" to
"WEBGL_OR_COMPOSITOR_CONTEXT".

R=piman
BUG=584497
==========

to

==========
Rename the CauseForGpuLaunch from WebGraphicsContext3D.

Rename "WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE" to
"WEBGL_OR_COMPOSITOR_CONTEXT".

R=piman
BUG=584497
==========

[Ken Russell (switch to Gerrit), 2016-05-03 23:13:26]

On 2016/05/03 22:44:48, danakj wrote:
> Oh, this also is in histograms.xml
> 
>   <int value="4" label="WebGraphicsContext3DCommandBufferImpl Initialize">    
 
>                                                                           
>     CAUSE_FOR_GPU_LAUNCH_WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE     
 
>                                                                           
>   </int>
> 
> 
> So.. maybe we should just add some new ones and stop using this one? WDYT?

Yes, I think that would be best. Right now there is only one entry point which
creates the contexts for WebGL, isn't that correct?
RendererBlinkPlatformImpl::createOffscreenGraphicsContext3DProvider? The rest
can definitively be known to be compositor contexts.

xidachen@ is working on instantiating WebGL contexts in web workers, so that new
entry point would be another that could mark the context as being for WebGL.

How does that sound?

[danakj, 2016-05-03 23:31:43]

On Tue, May 3, 2016 at 4:13 PM, <kbr@chromium.org> wrote:

> On 2016/05/03 22:44:48, danakj wrote:
> > Oh, this also is in histograms.xml
> >
> > <int value="4" label="WebGraphicsContext3DCommandBufferImpl Initialize">
>
> >
> > CAUSE_FOR_GPU_LAUNCH_WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE
>
> >
> > </int>
> >
> >
> > So.. maybe we should just add some new ones and stop using this one?
> WDYT?
>
> Yes, I think that would be best. Right now there is only one entry point
> which
> creates the contexts for WebGL, isn't that correct?
> RendererBlinkPlatformImpl::createOffscreenGraphicsContext3DProvider? The
> rest
> can definitively be known to be compositor contexts.
>

Yes that's right. And in that place the contexts are always for WebGL.


>
> xidachen@ is working on instantiating WebGL contexts in web workers, so
> that new
> entry point would be another that could mark the context as being for
> WebGL.
>
> How does that sound?
>

Yep, okay I'll add new entries for:
- WebGL context
- Shared renderer main thread context
- Raster worker context

- And maybe for Compositor context... These contexts always share with
raster worker so we have to create the raster worker first and already have
a channel by then, though the code may not make that part clear.


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

-- 
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 23:34:43]

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

[commit-bot: I haz the power, 2016-05-03 23:34:44]

Dry run: This issue passed the CQ dry run.

[danakj, 2016-05-04 00:47:52]

On 2016/05/03 23:31:43, danakj wrote:
> Yep, okay I'll add new entries for:
> - WebGL context
> - Shared renderer main thread context
> - Raster worker context
> 
> - And maybe for Compositor context... These contexts always share with
> raster worker so we have to create the raster worker first and already have
> a channel by then, though the code may not make that part clear.

OK so there's a lot of various reasons we launch the GPU process, so I figured
we probably want something for each:

CAUSE_FOR_GPU_LAUNCH_BROWSER_SHARED_MAIN_THREAD_CONTEXT,
- The browser process shared context, used for tab capture and stuff.

CAUSE_FOR_GPU_LAUNCH_RENDERER_SHARED_MAIN_THREAD_CONTEXT,
- The renderer shared context, used for canvas and stuff.

CAUSE_FOR_GPU_LAUNCH_WEBGL_CONTEXT,
- A WebGL context

CAUSE_FOR_GPU_LAUNCH_SHARED_WORKER_THREAD_CONTEXT,
- A worker context in either process. This is what you'd see when starting the
ui or display compositors.

CAUSE_FOR_GPU_LAUNCH_RENDERER_VERIFY_GPU_COMPOSITING,
- In RenderWidget we establish a channel to check if we want to use GPU context.
So you'd see this instead if starting a renderer compositor since we do this
before trying to create a worker context.

CAUSE_FOR_GPU_LAUNCH_DISPLAY_COMPOSITOR_CONTEXT,
- On Android there is no worker context in the ui compositor, and the context is
shared between ui and display compositors, so you won't see SHARED_WORKER_THREAD
there, so I added this one.

Do these sound good to you Ken/Antoine? They're in the latest patch set.

I also reordered the logic in RenderWidget::CreateOutputSurface a bunch to make
things more clear, and pass explicit nullptrs in places instead of having
variables that are never set on some branches.

[danakj, 2016-05-04 00:50:50]

https://codereview.chromium.org/1950723002/diff/40001/content/browser/composi...
File content/browser/compositor/gpu_process_transport_factory.cc (right):

https://codereview.chromium.org/1950723002/diff/40001/content/browser/composi...
content/browser/compositor/gpu_process_transport_factory.cc:412:
CAUSE_FOR_GPU_LAUNCH_SHARED_WORKER_THREAD_CONTEXT,
This could really go either way.. it could be DISPLAY_COMPOSITOR_CONTEXT too..

[danakj, 2016-05-04 00:51:47]

Description was changed from

==========
Rename the CauseForGpuLaunch from WebGraphicsContext3D.

Rename "WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE" to
"WEBGL_OR_COMPOSITOR_CONTEXT".

R=piman
BUG=584497
==========

to

==========
Remove WebGraphicsContext3D reason from the CauseForGpuLaunch enum

Replace "WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE" with a bunch
of more specific and real reasons.

R=piman
BUG=584497
==========

[danakj, 2016-05-04 01:16:02]

https://codereview.chromium.org/1950723002/diff/60001/content/renderer/render...
File content/renderer/render_widget.cc (left):

https://codereview.chromium.org/1950723002/diff/60001/content/renderer/render...
content/renderer/render_widget.cc:811: if (!context_provider) {
Turns out this branch is only for layout tests in software compositing, which
doesn't exist anymore, so I dropped it. That means nothing makes a
"CompositorOutputSurface" anymore.

https://codereview.chromium.org/1950723002/diff/60001/content/renderer/render...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/1950723002/diff/60001/content/renderer/render...
content/renderer/render_widget.cc:746: if (use_software) {
This branch didn't exist before, we'd skip down to the !layouttests branch way
at the bottom and do this.

Since layout tests cant be software, we can do this branch and early-out at the
top now.

[danakj, 2016-05-04 01:16:22]

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

[danakj, 2016-05-04 01:16:43]

On 2016/05/04 00:47:52, danakj wrote:
> On 2016/05/03 23:31:43, danakj wrote:
> > Yep, okay I'll add new entries for:
> > - WebGL context
> > - Shared renderer main thread context
> > - Raster worker context
> > 
> > - And maybe for Compositor context... These contexts always share with
> > raster worker so we have to create the raster worker first and already have
> > a channel by then, though the code may not make that part clear.
> 
> OK so there's a lot of various reasons we launch the GPU process, so I figured
> we probably want something for each:
> 
> CAUSE_FOR_GPU_LAUNCH_BROWSER_SHARED_MAIN_THREAD_CONTEXT,
> - The browser process shared context, used for tab capture and stuff.
> 
> CAUSE_FOR_GPU_LAUNCH_RENDERER_SHARED_MAIN_THREAD_CONTEXT,
> - The renderer shared context, used for canvas and stuff.
> 
> CAUSE_FOR_GPU_LAUNCH_WEBGL_CONTEXT,
> - A WebGL context
> 
> CAUSE_FOR_GPU_LAUNCH_SHARED_WORKER_THREAD_CONTEXT,
> - A worker context in either process. This is what you'd see when starting the
> ui or display compositors.
> 
> CAUSE_FOR_GPU_LAUNCH_RENDERER_VERIFY_GPU_COMPOSITING,
> - In RenderWidget we establish a channel to check if we want to use GPU
context.
> So you'd see this instead if starting a renderer compositor since we do this
> before trying to create a worker context.
> 
> CAUSE_FOR_GPU_LAUNCH_DISPLAY_COMPOSITOR_CONTEXT,
> - On Android there is no worker context in the ui compositor, and the context
is
> shared between ui and display compositors, so you won't see
SHARED_WORKER_THREAD
> there, so I added this one.
> 
> Do these sound good to you Ken/Antoine? They're in the latest patch set.
> 
> I also reordered the logic in RenderWidget::CreateOutputSurface a bunch to
make
> things more clear, and pass explicit nullptrs in places instead of having
> variables that are never set on some branches.

+kbr who got removed somehow I think.

[Ken Russell (switch to Gerrit), 2016-05-04 01:47:05]

Thanks for re-adding me. The new categorization looks very very good. LGTM

[danakj, 2016-05-04 19:46:31]

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

[danakj, 2016-05-04 19:46:31]

+holte@chromium.org for histograms

[danakj, 2016-05-04 19:46:40]

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

[commit-bot: I haz the power, 2016-05-04 19:46:59]

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

[commit-bot: I haz the power, 2016-05-04 20:28:21]

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

[commit-bot: I haz the power, 2016-05-04 20:28:22]

Dry run: This issue passed the CQ dry run.

[piman, 2016-05-04 21:34:08]

lgtm

[danakj, 2016-05-05 00:10:47]

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

[danakj, 2016-05-05 00:10:47]

+rkaplow@chromium.org for histograms?

[danakj, 2016-05-05 00:10:58]

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

[commit-bot: I haz the power, 2016-05-05 00:11:52]

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

[Steven Holte, 2016-05-05 00:16:22]

lgtm

[commit-bot: I haz the power, 2016-05-05 00:17:04]

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

[commit-bot: I haz the power, 2016-05-05 00:17:05]

Dry run: This issue passed the CQ dry run.

[danakj, 2016-05-05 00:41:22]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2016-05-05 00:41:38]

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

[commit-bot: I haz the power, 2016-05-05 00:46:18]

Description was changed from

==========
Remove WebGraphicsContext3D reason from the CauseForGpuLaunch enum

Replace "WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE" with a bunch
of more specific and real reasons.

R=piman
BUG=584497
==========

to

==========
Remove WebGraphicsContext3D reason from the CauseForGpuLaunch enum

Replace "WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE" with a bunch
of more specific and real reasons.

R=piman
BUG=584497
==========

[commit-bot: I haz the power, 2016-05-05 00:46:20]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-05 00:47:56]

Description was changed from

==========
Remove WebGraphicsContext3D reason from the CauseForGpuLaunch enum

Replace "WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE" with a bunch
of more specific and real reasons.

R=piman
BUG=584497
==========

to

==========
Remove WebGraphicsContext3D reason from the CauseForGpuLaunch enum

Replace "WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE" with a bunch
of more specific and real reasons.

R=piman
BUG=584497

Committed: https://crrev.com/3873e857d41721b7497c993f042c704f1243b9f3
Cr-Commit-Position: refs/heads/master@{#391721}
==========

[commit-bot: I haz the power, 2016-05-05 00:47:58]

Patchset 4 (id:??) landed as
https://crrev.com/3873e857d41721b7497c993f042c704f1243b9f3
Cr-Commit-Position: refs/heads/master@{#391721}