Closed
Bug 1333968
Opened 8 years ago
Closed 8 years ago
Label the PCompositorBridge::DidComposite message
Categories
(Core :: Graphics: Layers, defect)
Core
Graphics: Layers
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: billm, Assigned: billm)
References
Details
(Whiteboard: [gfx-noted][QDL][BACKLOG][GFX])
Attachments
(3 files)
(deleted),
image/jpeg
|
Details | |
(deleted),
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
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.
Updated•8 years ago
|
Priority: -- → P2
Updated•8 years ago
|
Component: DOM → Graphics: Layers
Priority: P2 → --
Updated•8 years ago
|
Whiteboard: [gfx-noted]
Comment 2•8 years ago
|
||
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•8 years 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)
Comment 4•8 years ago
|
||
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•8 years ago
|
Blocks: gfx-labeling
Updated•8 years ago
|
Flags: needinfo?(wmccloskey)
Updated•8 years ago
|
Whiteboard: [gfx-noted] → [gfx-noted][QDL][BACKLOG][GFX]
Comment 5•8 years ago
|
||
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 years 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 years ago
|
||
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 years ago
|
||
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 years 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 years 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 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4026d045096f
https://hg.mozilla.org/mozilla-central/rev/47dfef12051f
Status: NEW → RESOLVED
Closed: 8 years 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.
Description
•