1314426 - Add a method to kill the GPU process for testing

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Comment 1

[George Wright (:gw280) (needinfo me!)]

Attachment: 0001-Bug-1314426-Add-a-method-to-nsIDOMWindowUtils-to-ter.patch

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 8806486 [details] [diff] [review]
0001-Bug-1314426-Add-a-method-to-nsIDOMWindowUtils-to-ter.patch

Review of attachment 8806486 [details] [diff] [review]:
-----------------------------------------------------------------

We probably want some property that says whether or not the GPU process is enabled, too. Otherwise we can't tell when to display the button.

::: dom/base/nsDOMWindowUtils.cpp
@@ +4089,5 @@
>  
> +NS_IMETHODIMP
> +nsDOMWindowUtils::TerminateGPUProcess()
> +{
> +  GPUProcessManager* pm = GPUProcessManager::Get();

This is null in content processes, so you probably want a null check here.

Comment 3

[George Wright (:gw280) (needinfo me!)]

OK, I was under the impression that getFeatureStatus was sufficient, but it looks like it won't be.

I will also add a property to get the GPU process PID, as I forgot that bit.

Comment 4

[George Wright (:gw280) (needinfo me!)]

Attachment: 0001-Bug-1314426-Add-a-method-to-nsIDOMWindowUtils-to-ter.patch

Updated to:

- Change uuid in nsIDOMWindowUtils.idl
- Add a get pid method

Comment 5

[George Wright (:gw280) (needinfo me!)]

We can use the GPU process pid method to determine if the GPU process is active; I've followed the convention for ContentParent where it returns -1 if there is no process: http://searchfox.org/mozilla-central/source/dom/ipc/ContentParent.cpp#2362

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8806797 [details] [diff] [review]
0001-Bug-1314426-Add-a-method-to-nsIDOMWindowUtils-to-ter.patch

>+nsDOMWindowUtils::TerminateGPUProcess()
>+{
>+  GPUProcessManager* pm = GPUProcessManager::Get();
::Get() may return null


>+  pm->KillProcess();
so please null check before calling KillProcess()

>-[scriptable, uuid(46b44e33-13c2-4eb3-bf80-76a4e0857ccc)]
>+[scriptable, uuid(c471d440-004b-4c50-a6f2-747db5f443b6)]
> interface nsIDOMWindowUtils : nsISupports {
no need to update uuids anymore. but doesn't harm either.

Comment 7

[George Wright (:gw280) (needinfo me!)]

Attachment: 0001-Bug-1314426-Add-a-method-to-nsIDOMWindowUtils-to-ter.patch

ARGH, I uploaded the wrong patch. That one was basically a slight rehash of the original one. This is the correct patch. Sorry!

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8806805 [details] [diff] [review]
0001-Bug-1314426-Add-a-method-to-nsIDOMWindowUtils-to-ter.patch

>+nsDOMWindowUtils::GetGpuProcessPid(int32_t *aPid)
Nit, int32_t* aPid

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/131019a5ba42
Add a method to nsIDOMWindowUtils to terminate the GPU process and get the GPU process pid r=smaug,dvander

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/131019a5ba42

Comment 11

[Jim Jeffery not reading bug-mail 1/2/11]

oops, soory wrong bug... put it back to 'Fixed'