Seth Fowler [:seth] [:s2h] Assignee
	Description • 10 years ago

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!

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 1 • 10 years ago

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.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 2 • 10 years ago

Thanks for the review! Pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/0b1b75d692bf

	Ryan VanderMeulen [:RyanVM]
	Comment 3 • 10 years ago

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.

	Ryan VanderMeulen [:RyanVM]
	Comment 4 • 10 years ago

And near permafail on B2G reftests.
https://treeherder.mozilla.org/ui/logviewer.html#?job_id=5352821&repo=mozilla-inbound

	Daniel Stenberg [:bagder]
	Comment 5 • 10 years ago

(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! =)

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 6 • 10 years ago

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.

	Daniel Stenberg [:bagder]
	Comment 7 • 10 years ago

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.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 8 • 10 years ago

(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.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 9 • 10 years ago

Here's a new try job for this patch:

https://tbpl.mozilla.org/?tree=Try&rev=199550778a99

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 10 • 10 years ago

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

	Phil Ringnalda (:philor)
	Comment 11 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/ee6ddbf938c7

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 12 • 10 years ago

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.

	Seth Fowler [:seth] [:s2h] Assignee
	Comment 13 • 10 years ago

Pushed to Aurora:

https://hg.mozilla.org/releases/mozilla-aurora/rev/9c590c364f26

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 14 • 10 years ago

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.