Closed Bug 1314426 Opened 3 years ago Closed 3 years ago

Add a method to kill the GPU process for testing

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: gw280, Assigned: gw280)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file, 2 obsolete files)

No description provided.
Assignee: nobody → gwright
Blocks: 1297792, 1297260
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.
Attachment #8806486 - Flags: review?(dvander) → review+
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.
Attachment #8806486 - Flags: review?(bugs)
Updated to:

- Change uuid in nsIDOMWindowUtils.idl
- Add a get pid method
Attachment #8806486 - Attachment is obsolete: true
Attachment #8806797 - Flags: review?(dvander)
Attachment #8806797 - Flags: review?(bugs)
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 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.
Attachment #8806797 - Flags: review?(bugs) → review+
ARGH, I uploaded the wrong patch. That one was basically a slight rehash of the original one. This is the correct patch. Sorry!
Attachment #8806797 - Attachment is obsolete: true
Attachment #8806797 - Flags: review?(dvander)
Attachment #8806805 - Flags: review?(dvander)
Attachment #8806805 - Flags: review?(bugs)
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
Attachment #8806805 - Flags: review?(bugs) → review+
Attachment #8806805 - Flags: review?(dvander) → review+
Whiteboard: [gfx-noted]
Pushed by gwright@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/131019a5ba42
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Resolution: FIXED → WORKSFORME
oops, soory wrong bug... put it back to 'Fixed'
Resolution: WORKSFORME → FIXED
You need to log in before you can comment on or make changes to this bug.