Label the PCompositorBridge::DidComposite message

RESOLVED FIXED in Firefox 55

Status

()

Core
Graphics: Layers
RESOLVED FIXED
11 months ago
8 months ago

People

(Reporter: billm, Assigned: billm)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

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

Attachments

(3 attachments)

(Assignee)

Description

11 months ago
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

Updated

11 months ago
Component: DOM → Graphics: Layers
Priority: P2 → --

Updated

11 months ago
Whiteboard: [gfx-noted]

Comment 1

10 months ago
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)
(Assignee)

Comment 3

10 months ago
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)
Created attachment 8847531 [details]
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)

Updated

9 months ago
Blocks: 1341537

Updated

9 months ago
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)
(Assignee)

Comment 6

8 months ago
Sorry I missed your needinfo Kevin. I think it got canceled by mistake. I have some patches for this now.
Flags: needinfo?(wmccloskey)
(Assignee)

Comment 7

8 months ago
Created attachment 8855048 [details] [diff] [review]
allow a per-message event target to be specified

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)
(Assignee)

Comment 8

8 months ago
Created attachment 8855049 [details] [diff] [review]
label DidComposite

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)
(Assignee)

Updated

8 months ago
Attachment #8855048 - Attachment is patch: true
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+

Comment 10

8 months ago
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)

Comment 11

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4026d045096f
https://hg.mozilla.org/mozilla-central/rev/47dfef12051f
Status: NEW → RESOLVED
Last Resolved: 8 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.