Closed Bug 1730834 Opened 3 years ago Closed 3 years ago

Image doesn't animate if loaded after location.replace() call.

Categories

(Core :: Performance, defect)

Product:
Core
Component:
Performance
Version:
Firefox 94
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
94 Branch
Project Flags:
Webcompat Priority P3
Performance Impact ?
Tracking Flags:
Tracking Status
firefox-esr78 --- wontfix
firefox-esr91 --- wontfix
firefox92 --- wontfix
firefox93 --- wontfix
firefox94 --- verified

People

(Reporter: karlcow, Assigned: emilio)

References

(Regressed 1 open bug, Regression,
URL
)

Details

(Keywords: regression)

Attachments

(1 file)

Bug 1730834 - Don't stop animated images from nsDocumentViewer::Stop. r=smaug
48 bytes, text/x-phabricator-request
Details | Review
  1. Go to https://www.vodafone.co.uk/webcenter/portal/myvodafone/accountsummary

Expected:
loading gif visible before the login widget

Actual:
White page

Using webpagetest, we can clearly see a difference in between the two.

With Chrome
https://www.webpagetest.org/result/210915_AiDcRF_eee4758e5884dd7b55039834b7a0f48d/
With Firefox
https://www.webpagetest.org/result/210915_AiDcSH_c215b02ac2d8ede89c2e118c11ce274c/

Interesting on Chrome, the FCP (First Contentfull Paint) starts around 1.1s
while in Firefox it starts around 5.5s.

The gif is set through a background-image

<body
  style="
    background-color: rgb(255, 255, 255);
    background-repeat: no-repeat;
    background-attachment: fixed;
    background-position: center center;
    background-image: url('/cs/groups/public/documents/webasset/myvodafone_loading.gif');
  "
  onload="
        adfDoRedirect();
        window.setTimeout(
            function() {
                AdfLoopbackUtils.appendBodyStyle(
                    &quot;background-image: url('/cs/groups/public/documents/webasset/myvodafone_loading.gif')&quot;
                    )
                },
            1)"
></body>

This isn't a performance issue, isn't it? It is more like a why the gif isn't presented on the screen issue.

I collected a profile https://share.firefox.dev/3nBejh2, and looks like the markers indicated that we were player the animation, however it wasn't there?

Emilio, does anything look obscure to you?

Flags: needinfo?(emilio)

I agree this looks like an image animation / painting bug, somehow.

yes initially I thought it was the gif which was too slow to be downloaded. but that's not the case.
Then another hunch was what how firefox draw the body content box, when adding a background image with JS. Like maybe the image is here and playing but not visible because the body box is too small.

data:text/html,<!doctype html><body></body>

has a 0px height in all browsers.

And if I do

document.body.style.backgroundImage = "url('https://www.vodafone.co.uk/cs/groups/public/documents/webasset/myvodafone_loading.gif')"

the body is still 0 height but the logo is visible. So that was not it.

Yeah, this is an image animation issue. Looking at it on RR, the image doesn't animate because its mAnimationMode is kDontAnimMode. Seeing where that comes from, it comes from here:

(rr) bt
#0  nsPresContext::SetImageAnimationMode(unsigned short) (this=0x7fcac4789000, aMode=1) at /home/emilio/src/moz/gecko/layout/base/nsPresContext.cpp:1095
#1  0x00007fcadb4925eb in nsDocumentViewer::Stop() (this=0x7fcac4776580) at /home/emilio/src/moz/gecko/layout/base/nsDocumentViewer.cpp:1801
#2  0x00007fcadc33c57f in nsDocShell::Stop(unsigned int) (this=this@entry=0x7fcac477d400, aStopFlags=3) at /home/emilio/src/moz/gecko/docshell/base/nsDocShell.cpp:4132
#3  0x00007fcadc32b55c in nsDocShell::InternalLoad(nsDocShellLoadState*, mozilla::Maybe<unsigned int>) (this=this@entry=0x7fcac477d400, aLoadState=aLoadState@entry=0x7fcae249e760, aCacheKey=...)
    at /home/emilio/src/moz/gecko/docshell/base/nsDocShell.cpp:9336
#4  0x00007fcadc34b9dc in nsDocShell::LoadURI(nsDocShellLoadState*, bool, bool) (this=0x7fcac477d400, aLoadState=0x7fcae249e760, aSetNavigating=<optimized out>, aContinueHandlingSubframeHistory=<optimized out>)
    at /home/emilio/src/moz/gecko/docshell/base/nsDocShell.cpp:853
#5  0x00007fcadc329af8 in mozilla::dom::BrowsingContext::LoadURI(nsDocShellLoadState*, bool) (this=<optimized out>, aLoadState=0x7fcae249e760, aSetNavigating=false) at /home/emilio/src/moz/gecko/docshell/base/BrowsingContext.cpp:1921
#6  0x00007fcad92e9b78 in mozilla::dom::LocationBase::SetURI(nsIURI*, nsIPrincipal&, mozilla::ErrorResult&, bool) (this=<optimized out>, 
    this@entry=0x7fcac31720a8, aURI=<optimized out>, aSubjectPrincipal=..., aRv=<optimized out>, aReplace=true) at /home/emilio/src/moz/gecko/dom/base/LocationBase.cpp:155
#7  0x00007fcad92ec15c in mozilla::dom::LocationBase::SetHrefWithBase(nsTSubstring<char16_t> const&, nsIURI*, nsIPrincipal&, bool, mozilla::ErrorResult&)
    (this=this@entry=0x7fcac31720a8, aHref=<optimized out>, aBase=<optimized out>, aSubjectPrincipal=..., aReplace=true, aRv=...) at /home/emilio/src/moz/gecko/dom/base/LocationBase.cpp:234
#8  0x00007fcad92eb83f in mozilla::dom::LocationBase::DoSetHref(nsTSubstring<char16_t> const&, nsIPrincipal&, bool, mozilla::ErrorResult&) (this=0x7fcac31720a8, aHref=..., aSubjectPrincipal=..., aReplace=false, aRv=...)
    at /home/emilio/src/moz/gecko/dom/base/LocationBase.cpp:189
#9  0x00007fcad96389e4 in mozilla::dom::Location_Binding::replace(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&)Traceback (most recent call last):
  File "/home/emilio/src/moz/gecko/js/src/gdb/mozilla/Root.py", line 55, in to_string
    ptr = ptr.dereference()
gdb.error: value has been optimized out
 (cx=0x7fcac5b22000, obj=
, void_self=<optimized out>, args=...) at LocationBinding.cpp:1095
#10 0x00007fcada194284 in mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::CrossOriginThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) (cx=0x7ffd1b856378, 
    cx@entry=0x7fcac5b22000, argc=<optimized out>, vp=<optimized out>) at /home/emilio/src/moz/gecko/dom/bindings/BindingUtils.cpp:3300
#11 0x00007fcadc97a181 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&)
    (cx=0x7fcac5b22000, native=native@entry=0x7fcada19406b <mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::CrossOriginThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*)>, reason=<optimized out>, reason@entry=js::CallReason::Call, args=...) at /home/emilio/src/moz/gecko/js/src/vm/Interpreter.cpp:401
#12 0x00007fcadc96c2bb in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) (cx=0x7ffd1b856378, 
    cx@entry=0x7fcac5b22000, args=..., construct=construct@entry=js::NO_CONSTRUCT, reason=js::CallReason::Getter, reason@entry=js::CallReason::Call) at /home/emilio/src/moz/gecko/js/src/vm/Interpreter.cpp:488
#13 0x00007fcadc96cf30 in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) (cx=0x7fcac5b22000, args=..., reason=reason@entry=js::CallReason::Call) at /home/emilio/src/moz/gecko/js/src/vm/Interpreter.cpp:548
#14 0x00007fcadc961563 in js::CallFromStack(JSContext*, JS::CallArgs const&) (cx=0x7ffd1b856378, cx@entry=0xffff800000000000, args=...) at /home/emilio/src/moz/gecko/js/src/vm/Interpreter.cpp:552
#15 Interpret(JSContext*, js::RunState&) (cx=0x7ffd1b856378, cx@entry=0x7fcac5b22000, state=...) at /home/emilio/src/moz/gecko/js/src/vm/Interpreter.cpp:3255
#16 0x00007fcadc958bb1 in js::RunScript(JSContext*, js::RunState&) (cx=0x7fcac5b22000, state=...) at /home/emilio/src/moz/gecko/js/src/vm/Interpreter.cpp:370
#17 0x00007fcadc96c209 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) (cx=0x7ffd1b856378, 
    cx@entry=0x7fcac5b22000, args=..., construct=construct@entry=js::NO_CONSTRUCT, reason=<optimized out>, reason@entry=js::CallReason::Call) at /home/emilio/src/moz/gecko/js/src/vm/Interpreter.cpp:520
#18 0x00007fcadc96cf30 in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) (cx=cx@entry=0x7fcac5b22000, args=..., reason=reason@entry=js::CallReason::Call) at /home/emilio/src/moz/gecko/js/src/vm/Interpreter.cpp:548
#19 0x00007fcadc96d120 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason)

That is, from a location.replace() call in JS:

(rr) js
0 AdfLoopbackUtils.runLoopback(a = ""f7l3ch6ti"", b = ""_afrLoop"", d = ""5296247531134152"", e = "https://www.vodafone.co.uk/webcenter/portal/myvodafone/accountsummary", f = "false", g = ""_afrPage"", h = "null", k = ""f7l3ch6ti"", l = """", m = ""!"", n = "false", p = """", q = """", r = """", s = """", u = """", t = "2", v = ""https://www.vodafone.co.uk/webcenter/portal/myvodafone/accountsummary"", w = ""/_noloopbackerror_"", z = ""_afrFS"", y = ""_afrMT"", A = ""_afrMFW"", B = ""_afrMFH"", C = ""_afrMFDW"", D = ""_afrMFDH"", G = ""_afrMFC"", E = ""_afrMFCI"", H = ""_afrMFM"", I = ""_afrMFR"", J = ""_afrMFG"", N = ""_afrMFS"", Q = ""_afrMFO"", O = ""https://www.vodafone.co.uk/webcenter/portal/myvodafone/accountsummary?_afrLoop=5296247531134152&_afrWindowMode=2&Adf-Window-Id=f7l3ch6ti&_afrFS=16&_afrMT=screen&_afrMFW=1280&_afrMFH=887&_afrMFDW=1920&_afrMFDH=1080&_afrMFC=8&_afrMFCI=0&_afrMFM=0&_afrMFR=192&_afrMFG=0&_afrMFS=0&_afrMFO=0"", U = """", W = """", X = """", Y = """", Z = """", $ = """", aa = """", ba = """", ca = """", da = """", ea = """", fa = """", ga = """", R = "null") ["https://www.vodafone.co.uk/webcenter/portal/myvodafone/accountsummary":12:315]
    this = function AdfLoopbackUtils(){}
1 adfDoRedirect() ["https://www.vodafone.co.uk/webcenter/portal/myvodafone/accountsummary":19:18]
    this = [object Window]
2 onload(event = "[object Event]") ["https://www.vodafone.co.uk/webcenter/portal/myvodafone/accountsummary":1:0]
    this = [object Window]

So that comes from this line of code, which comes from bug 70030.

Flags: needinfo?(emilio)
Regressed by: 70030
Has Regression Range: --- → yes
Assignee: nobody → emilio
Summary: FCP is 4s later in Firefox compared to Chrome → Image doesn't animate if loaded after location.replace() call.

We don't stop other animations and such, so seems weird to do it only
for animated images. This line comes from bug 70030, but we no longer
provide "Esc" as a shortcut for Stop(), so it's probably no longer
relevant for users.

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8e8744f33c56
Don't stop animated images from nsDocumentViewer::Stop. r=smaug

https://hg.mozilla.org/mozilla-central/rev/8e8744f33c56

Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox94: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 94 Branch
Flags: qe-verify+

Reproduced the issue using an affected Nightly build from 2021-09-15 on Win 10x64.

This issue is verified as fixed on latest Beta 94.0b6, across the following platforms: Win 10x64 macOS 10.15 and Ubuntu 20.04.

Status: RESOLVED → VERIFIED
status-firefox94: fixed → verified
Flags: qe-verify+
Regressions: 1739518
Performance Impact: --- → ?
Whiteboard: [qf]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: