Closed Bug 1333968 Opened 3 years ago Closed 3 years ago

Label the PCompositorBridge::DidComposite message

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: billm, Assigned: billm)

References

(Blocks 1 open bug)

Details

(Whiteboard: [gfx-noted][QDL][BACKLOG][GFX])

Attachments

(3 files)

The layers ID is part of the message, so we should be able to look this up when the message is received. We'll have to put a lock around sTabChildren though.

We will also need a mechanism to set the event target for individual messages. Right now we can only do that for constructors (via GetConstructedEventTarget). Alternatively, we could send the DidComposite message using some sub-protocol that's already labeled. Probably need some feedback from graphics folks about that.
Priority: -- → P2
Component: DOM → Graphics: Layers
Priority: P2 → --
Whiteboard: [gfx-noted]
Need info myself to follow up this bug.
Flags: needinfo?(howareyou322)
I'm not sure what we want to do here. We are trying to labeling the tasks which use DispatchToMainThread() function. The PCompositorBridge::DidComposite() doesn't use DispatchToMainThread(). Why we need to label this task?
Are we going to post all tasks in DidComposite() to the main thread using DispatchToMainThread() instead of directly calling? Then, we could label these tasks.
Flags: needinfo?(wmccloskey)
Any IPC message is dispatched to a thread automatically by the IPC code, which is what happens for DidComposite.

Please let me think about this some more though. It's not ready to be worked on.
Flags: needinfo?(wmccloskey)
Flags: needinfo?(howareyou322)
Attached image DidComposite flow
Hello Bill, I am working on DOM labeling for gfx folders.

I drew a picture to explain the problem here, please correct me if my understanding is wrong.

To schedule the tasks from ipc whose target is in content side, we set an EventTarget to the ipc actor and mark every tasks received by the actor as the same EvenTarget before we dispatch the message from IO thread to the TaskQuene.

However, in this CompositorBridgeParent/Child case, the actors do not belong to specific TabGroups or DocGroups and the messages might have different EventTarget.
The information about the EventTarget(Layer ID) is wrapped in the message. Currently we deal with this information after the task is execute [1] which is too late for scheduling.

Therefore, we need another mechanism which can set the EventTarget dynamically before the message is dispatched from IO thread to TaskQuene.

Do you have a prototype about this mechanism ? Thank you.

[1] https://dxr.mozilla.org/mozilla-central/rev/6d38ad302429c98115c354d643e81987ecec5d3c/gfx/layers/ipc/CompositorBridgeChild.cpp#586
Thank you.
Flags: needinfo?(wmccloskey)
Flags: needinfo?(wmccloskey)
Whiteboard: [gfx-noted] → [gfx-noted][QDL][BACKLOG][GFX]
Hello Bill, is my description in comment 4 correct ?
Do you have prototypes for bug numbers for this mechanism ?
Flags: needinfo?(wmccloskey)
Sorry I missed your needinfo Kevin. I think it got canceled by mistake. I have some patches for this now.
Flags: needinfo?(wmccloskey)
This patch allows us to customize the nsIEventTarget that each individual message is dispatched to. It's necessary for DidComposite messages, where the tab is encoded in the message parameters.
Assignee: nobody → wmccloskey
Attachment #8855048 - Flags: review?(dvander)
This patch labels DidComposite. I had to switch a few things around since the GetSpecialMessageEventTarget method runs on the I/O thread.
Attachment #8855049 - Flags: review?(dvander)
Attachment #8855049 - Flags: review?(dvander) → review+
Comment on attachment 8855048 [details] [diff] [review]
allow a per-message event target to be specified

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

::: ipc/glue/ProtocolUtils.h
@@ +376,5 @@
>      GetActorEventTarget();
>  
>  protected:
>      virtual already_AddRefed<nsIEventTarget>
> +    GetSpecialMessageEventTarget(const Message& aMsg) { return nullptr; }

I'm not sure I like the name "Special" here since IPDL already has specially intercepted messages. Is there something that works better?
Attachment #8855048 - Flags: review?(dvander) → review+
Pushed by wmccloskey@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4026d045096f
Add GetSpecificMessageEventTarget method to change the event target for specific IPC messages (r=dvander)
https://hg.mozilla.org/integration/mozilla-inbound/rev/47dfef12051f
Label the DidComposite message (r=dvander)
https://hg.mozilla.org/mozilla-central/rev/4026d045096f
https://hg.mozilla.org/mozilla-central/rev/47dfef12051f
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.