Closed Bug 1118694 Opened 6 years ago Closed 6 years ago

Always retarget OnDataAvailable for raster images

Categories

(Core :: ImageLib, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox37 --- fixed
firefox38 --- fixed

People

(Reporter: seth, Assigned: seth)

References

Details

Attachments

(1 file)

In bug 867755 it was determined that we sometimes couldn't retarget OnDataAvailable for raster image loads, because we couldn't synchronously decode off-main-thread at that time. This was because we couldn't allocate image frames off-main-thread.

Bug 1117607 eliminated the final restrictions on allocating image frames off-main-thread, so that means we can always retarget OnDataAvailable for raster images now!
Here's the patch; it basically just removes the code related to this restriction
from bug 867755. I also took the opportunity to directly retrieve the event
target from DecodePool instead of going through RasterImage, which lets us
remove RasterImage::GetEventTarget as well.
Attachment #8545174 - Flags: review?(sworkman)
Attachment #8545174 - Flags: review?(sworkman) → review+
Blocks: 1119158
Backed out for the same B2G Nuwa test failures that were visible in your recent Try pushes for these patches.
https://hg.mozilla.org/integration/mozilla-inbound/rev/7967b585efe9

https://treeherder.mozilla.org/ui/logviewer.html#?job_id=4155884&repo=try
https://treeherder.mozilla.org/ui/logviewer.html#?job_id=4156168&repo=try

FWIW, bagder has been hitting these same failures on a patch of his he's been trying to get landed. Not sure what underlying issue is being tickled here, but you two might want to coordinate.
Flags: needinfo?(daniel)
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #3)

> https://treeherder.mozilla.org/ui/logviewer.html#?job_id=4155884&repo=try
> https://treeherder.mozilla.org/ui/logviewer.html#?job_id=4156168&repo=try
> 
> FWIW, bagder has been hitting these same failures on a patch of his he's
> been trying to get landed. Not sure what underlying issue is being tickled
> here, but you two might want to coordinate.

I got the exact same problems yes indeed, test_NuwaProcessDeadlock.html and test_NuwaProcessCreation.html both. However, during the day I've played with variations of my patch and even running completely without it. Even with a clean up-to-date mozilla-central repo I get repeated test failures on those two mochitests on my local dev machine. I'll admit they look completely strange to me and I haven't yet dug much deeper as to why they occur or what they actually mean. I intend to dig further tomorrow. Advice, suggestions or just hoorays from bystanders will not be shot down! =)
Flags: needinfo?(daniel)
Daniel, please see my updated patch on bug 1119158, which was the real cause of my issue. It turns out you have to register new threads with the Nuwa process on B2G.

(Note that I haven't tested that code yet because I'm still fixing bugs in earlier patches in my patch queue, but it should be more or less correct. =)

Just needinfo'ing you to be sure you see this.
Flags: needinfo?(daniel)
Excellent, thank you! Are you or anyone else aware of this being documented anywhere? 

https://developer.mozilla.org/en-US/Firefox_OS/Security/B2G_IPC_internals seems very sparse on implementation requirement/details. https://wiki.mozilla.org/NuwaTemplateProcess also documents the Nuwa concept but not a lot is said about what implementers need to consider.
Flags: needinfo?(daniel)
(In reply to Daniel Stenberg [:bagder] from comment #7)
> Excellent, thank you! Are you or anyone else aware of this being documented
> anywhere? 

I'm not sure; I figured the issue out by asking someone else. Someone on #b2g might know more.
Here's a new try job for this patch:

https://tbpl.mozilla.org/?tree=Try&rev=199550778a99
It looks like all the test failures there are either unrelated or things that have been fixed by other patches, so I went ahead and pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/ee6ddbf938c7
https://hg.mozilla.org/mozilla-central/rev/ee6ddbf938c7
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Comment on attachment 8545174 [details] [diff] [review]
Always retarget OnDataAvailable for RasterImage

Approval Request Comment
[Feature/regressing bug #]: Part of fix for bug 1125272 and bug 1119938.
[User impact if declined]: Already approved.
[Describe test coverage new/current, TreeHerder]: In 38 for a long time.
[Risks and why]: Low risk; in 38 for a long time.
[String/UUID change made/needed]: None.
Attachment #8545174 - Flags: approval-mozilla-aurora?
Comment on attachment 8545174 [details] [diff] [review]
Always retarget OnDataAvailable for RasterImage

Aurora approval previously granted for a collection of ImageLib related bugs that Seth worked on and QE verified before uplift.
Attachment #8545174 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.