1986332 - css animated gif frozen on second load in iframe srcdoc

Status: NEW

(Core :: Graphics: ImageLib, defect, P3)

Description

[eyal gruss]

i am displaying an animated gif in an iframe. i use srcdoc to set the iframe content. the gif is loaded as css background image. if i reload the exact same srcdoc again, the animated gif freezes. maybe this is due to caching?

observations:

• it works when i add a random hash to the filename
• it works when i load the image via <img>
• it works in chromium

demo: https://codepen.io/eyaler/pen/dPYQMgQ

maybe related: https://bugzilla.mozilla.org/show_bug.cgi?id=129986

Comment 1

[Mayank Bansal]

Thanks for the testcase!

Bisection:
Bug 1956486 - Annotate some mixed content tests as failing after navigation. r=tschuster
Differential Revision: https://phabricator.services.mozilla.com/D243771

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1956486

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Attachment: Test-case from comment 0

Comment 4

[Emilio Cobos Álvarez [:emilio]]

So this particular case was regressed by bug 1956486 but afaict you could trigger it before by using a <link>.

I don't know off-hand how does <img> avoid it, tho, since that should also hit the image cache...

Comment 5

[Emilio Cobos Álvarez [:emilio]]

I took a look and GlobalImageObserver::Notify isn't getting called on the reloaded page, even though we're registering it to the refresh driver...

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Oh, so we register it, but then the second page un-registers it... Something like this "fixes" it, but it's quite hacky:

diff --git a/layout/style/ImageLoader.cpp b/layout/style/ImageLoader.cpp
index 4f79ae123eb8..b46d56f5ed79 100644
--- a/layout/style/ImageLoader.cpp
+++ b/layout/style/ImageLoader.cpp
@@ -310,6 +310,15 @@ void ImageLoader::DeregisterImageRequest(imgIRequest* aRequest,

   if (auto entry = sImages->Lookup(aRequest)) {
     entry.Data()->mImageLoaders.EnsureRemoved(this);
+    if (aPresContext) {
+      auto* rd = aPresContext->RefreshDriver();
+      for (auto* otherLoader : entry.Data()->mImageLoaders) {
+        if (auto* otherPc = otherLoader->GetPresContext();
+            otherPc && otherPc->RefreshDriver() == rd) {
+          return;
+        }
+      }
+    }
   }

   if (aPresContext) {

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Attachment: Stack of the de-registration that makes the image not animate anymore.

HTML images get a fresh proxy so they don't suffer from this.

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Tim, I can think of multiple ways of going around fixing this...

• Maybe nsRefreshDriver should keep track of how many times the same proxy has been registered? That seems like the most "obvious" fix, perhaps.
• Alternatively, maybe worth pushing the animated request tracking to nsPresContext or Document, and just walking the documents like we do for ~all other refresh steps?
• Alternatively, I can make the CSS ImageLoader be a bit smarter maybe, but that is a bit more annoying and feels a bit hacky.

Maybe there's a better fix I might be overlooking? Have I missed something?

Comment 9

[Timothy Nikkel (:tnikkel)]

So far I haven't come up with anything better but I want to think a bit more on it still.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1956486