On IRC joe suggested doing this by changing RasterImage::RequestDecode from only doing a decode synchronously if the image is under some size to always decoding synchronously for $CONSTANT ms, and then posting the rest of the task to event loop if the decode did not finish in under $CONSTANT ms. Making this change should be easy for the case where we're not already in a size decode, and I think we can probably get away with ignoring that case.
The only part of this that seems hard to me is continuing to ensure that we satisfy consumers expectations (if there are any?) about async notifications. Does SyncDecode notify asynchronously? If not, this should be pretty straightforward, I think.
Fwiw, this sounds like a great solution to me in terms of UX.
Created attachment 560643 [details] [diff] [review] Patch
Comment on attachment 560643 [details] [diff] [review] Patch Review of attachment 560643 [details] [diff] [review]: ----------------------------------------------------------------- Remove the pref from all.js too, and make sure it's not used in mobile's default prefs. ::: modules/libpr0n/src/RasterImage.cpp @@ +2388,5 @@ > // If we've read all the data we have, we're done > if (mBytesDecoded == mSourceData.Length()) > return NS_OK; > > + // If we can do decoding now, do so. Small images will decode completely, "If we're able to decode now, do so."
Comment on attachment 560643 [details] [diff] [review] Patch > + if (!mDecoded && !mInDecoder && mHasSourceData) > + return mWorker->Run(); You should be able to call mWorker->Run() unconditionally here. We return early if we're fully decoded, called from inside the decoder, or don't have any data to decode. If we make it this far, our preconditions for decoding should already be true.
(In reply to Robert Lickenbrock [:rclickenbrock] from comment #5) > Comment on attachment 560643 [details] [diff] [review] [diff] [details] [review] > Patch > > > + if (!mDecoded && !mInDecoder && mHasSourceData) > > + return mWorker->Run(); > > You should be able to call mWorker->Run() unconditionally here. We return > early if we're fully decoded, called from inside the decoder, or don't have > any data to decode. If we make it this far, our preconditions for decoding > should already be true. Lets do that in another bug please.
Backed out for Android reftest failures: https://hg.mozilla.org/mozilla-central/rev/29c1738d7e27
Per IRC conversation with joe, the root problem here is that drawWindow doesn't seem to cause sync decoding of CSS images.
I've spent the last two days trying to get to a point where I can run reftests on android under a debugger without success, so I'm giving up here.
Ok, I finally managed to get this under a debugger. The reftest harness is just broken for the multiprocess world. Still tracking down exactly what's wrong.
6 years ago
6 years ago
6 years ago
Presumably we can now land this, as we don't care about multiprocess images.
B2G will care, and I'd rather not land stuff that's known to be broken.
Also, after thinking about this a bit more, if we land this as is we'll be landing a nice random orange timebomb.
Marking as Snappy P1: Right now, if we have a bunch of near-150KB images on a page, when those images come in from cache or are discarded and re-decoded, we'll decode each image synchronously. It's bad. (Perhaps this should be P2; I'm not sure how often we hit this case. But 150KB *compressed* is pretty big.)
Created attachment 644319 [details] [diff] [review] Disable failing Android XUL tests After several days of attempting to debug and reproduce, we've decided to disable the offending tests. We couldn't reproduce this on desktop with reftest-ipc, or even on Android XUL on multiple phones/tegras.
Comment on attachment 644319 [details] [diff] [review] Disable failing Android XUL tests I'd like this to be random-if(somethingBetterThanAndroidetc) so that we can revert all of these later if we stop testing this configuration.
Disabled background-size-body-percent-percent-overflow.html; rs=khuey https://hg.mozilla.org/mozilla-central/rev/9c67b2ff95b7
https://hg.mozilla.org/mozilla-central/rev/3bb125057939 And test disablings: https://hg.mozilla.org/mozilla-central/rev/d3829aea9cd1 https://hg.mozilla.org/mozilla-central/rev/b11ae0f50e1e https://hg.mozilla.org/mozilla-central/rev/b9b3b3373c08 https://hg.mozilla.org/mozilla-central/rev/9c67b2ff95b7 https://hg.mozilla.org/mozilla-central/rev/131799806373 https://hg.mozilla.org/mozilla-central/rev/996a68d8420e
Some of these disabled tests might have caught bug 783355 ...
Mark more as random: https://hg.mozilla.org/mozilla-central/rev/f5f29adc6d30 (Bug 769975 & bug 783182)
Mark another as random: https://hg.mozilla.org/integration/mozilla-inbound/rev/bb2bd7862bbd (Bug 783466)