1606628 - Remove nsStyleImageRequest.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Comment 1

[Emilio Cobos Álvarez [:emilio]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e590dbc559d4f395cec522bda93cdad90e9b454

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1606628 - Remove nsStyleImageRequest. r=heycam,tnikkel!

This removes nsStyleImageRequest by moving the load state to LoadData instead
(where other lazy state like the resolved URL and load id lives).

That way we can use cbindgen for more stuff (there's no blocker for using it for
all images now), and we can undo the image tracking shenanigans that I had to do
in bug 1605803 in nsImageFrame.

This removes the mDocGroup member because well, there's no real upside of that
now that quantum DOM is not a thing.

It also removes the static clones of the image requests, and the need for each
computed value instance to have its own request. These were needed because we
needed the image loader for the particular document to observe the image
changes. But we were also tracking the request -> loader for other purposes.
Instead, Now all the images get loaded with GlobalImageObserver as a listener,
which looks in the image map and forwards the notification to all the interested
loaders instead dynamically.

The style value is only responsible to load the image, and no longer tracks /
locks it. Instead, the loader does so, via the image tracker.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c9f4dd3ed78b
Remove nsStyleImageRequest. r=tnikkel,heycam

Comment 4

[Alexandru Michis [:malexandru]]

Backed out changeset c9f4dd3ed78b for causing failures in ImageLoader.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/51250763258f4f25aeefbf32259981adebf50645

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=287809512&resultStatus=testfailed%2Cbusted%2Cexception&revision=c9f4dd3ed78b916078cbc1ce5028c12e737c2fbf&searchStr=%28bc

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=287809512&repo=autoland&lineNumber=1887

[task 2020-02-06T21:10:08.178Z] 21:10:08 INFO - TEST-START | browser/components/preferences/in-content/tests/browser_connection_dnsoverhttps.js
[task 2020-02-06T21:10:22.243Z] 21:10:22 INFO - GECKO(1431) | MEMORY STAT | vsize 20975543MB | residentFast 1756MB
[task 2020-02-06T21:10:22.243Z] 21:10:22 INFO - TEST-OK | browser/components/preferences/in-content/tests/browser_connection_dnsoverhttps.js | took 14060ms
[task 2020-02-06T21:10:22.284Z] 21:10:22 INFO - checking window state
[task 2020-02-06T21:10:22.324Z] 21:10:22 INFO - TEST-START | browser/components/preferences/in-content/tests/browser_containers_name_input.js
[task 2020-02-06T21:10:24.375Z] 21:10:24 INFO - GECKO(1431) | Assertion failure: lookup.Data()->mImageLoaders.IsEmpty() (Shouldn't be keeping references to any loader by now), at /builds/worker/workspace/build/src/layout/style/ImageLoader.cpp:433
[task 2020-02-06T21:10:24.375Z] 21:10:24 INFO - GECKO(1431) | AddressSanitizer:DEADLYSIGNAL
[task 2020-02-06T21:10:24.375Z] 21:10:24 INFO - GECKO(1431) | =================================================================
[task 2020-02-06T21:10:24.376Z] 21:10:24 ERROR - GECKO(1431) | ==1431==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x7ff0210f8bd0 bp 0x7ffec9cacff0 sp 0x7ffec9cacfe0 T0)
[task 2020-02-06T21:10:24.377Z] 21:10:24 INFO - GECKO(1431) | ==1431==The signal is caused by a WRITE memory access.
[task 2020-02-06T21:10:24.377Z] 21:10:24 INFO - GECKO(1431) | ==1431==Hint: address points to the zero page.
[task 2020-02-06T21:10:24.896Z] 21:10:24 INFO - GECKO(1431) | #0 0x7ff0210f8bcf in mozilla::css::ImageLoader::DeregisterImageFromAllLoaders(imgRequestProxy*) /builds/worker/workspace/build/src/layout/style/ImageLoader.cpp:431:5
[task 2020-02-06T21:10:24.896Z] 21:10:24 INFO - GECKO(1431) | #1 0x7ff021188a59 in Run /builds/worker/workspace/build/src/layout/style/nsStyleStruct.cpp:197:5
[task 2020-02-06T21:10:24.897Z] 21:10:24 INFO - GECKO(1431) | #2 0x7ff021188a59 in Gecko_LoadData_Drop /builds/worker/workspace/build/src/layout/style/nsStyleStruct.cpp:218:13
[task 2020-02-06T21:10:24.932Z] 21:10:24 INFO - GECKO(1431) | #3 0x7ff01ebe5604 in ~StyleOwned_Body /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ServoStyleConsts.h:4328:10
[task 2020-02-06T21:10:24.932Z] 21:10:24 INFO - GECKO(1431) | #4 0x7ff01ebe5604 in ~StyleLoadDataSource /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ServoStyleConsts.h:4367:30
[task 2020-02-06T21:10:24.932Z] 21:10:24 INFO - GECKO(1431) | #5 0x7ff01ebe5604 in mozilla::StyleCssUrlData::~StyleCssUrlData() /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ServoStyleConsts.h:4390:8
[task 2020-02-06T21:10:24.932Z] 21:10:24 INFO - GECKO(1431) | #6 0x7ff021198e9b in Release /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ServoStyleConstsInlines.h:256:12
[task 2020-02-06T21:10:24.932Z] 21:10:24 INFO - GECKO(1431) | #7 0x7ff021198e9b in ~StyleArc /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ServoStyleConstsInlines.h:278:3
[task 2020-02-06T21:10:24.932Z] 21:10:24 INFO - GECKO(1431) | #8 0x7ff021198e9b in ~StyleCssUrl /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ServoStyleConsts.h:4448:8
[task 2020-02-06T21:10:24.932Z] 21:10:24 INFO - GECKO(1431) | #9 0x7ff021198e9b in ~StyleComputedUrl /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ServoStyleConsts.h:8358:8
[task 2020-02-06T21:10:24.932Z] 21:10:24 INFO - GECKO(1431) | #10 0x7ff021198e9b in nsStyleImage::SetNull() /builds/worker/workspace/build/src/layout/style/nsStyleStruct.cpp:1681:12
[task 2020-02-06T21:10:24.932Z] 21:10:24 INFO - GECKO(1431) | #11 0x7ff021198c5b in nsStyleImage::~nsStyleImage() /builds/worker/workspace/build/src/layout/style/nsStyleStruct.cpp:1638:5
[task 2020-02-06T21:10:25.129Z] 21:10:25 INFO - GECKO(1431) | #12 0x7ff02615ae62 in style::gecko_properties::_$LT$impl$u20$core..ops..drop..Drop$u20$for$u20$style..gecko_bindings..structs..root..mozilla..GeckoBackground$GT$::drop::h9fe3cfe85c48d22e /builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/build/style-b1f5d9f93ce53e27/out/gecko_properties.rs:17738:12
[task 2020-02-06T21:10:25.130Z] 21:10:25 INFO - GECKO(1431) | #13 0x7ff02615ae62 in core::ptr::real_drop_in_place::hbf1303b25db14b77 /rustc/4560ea788cb760f0a34127156c78e2552949f734/src/libcore/ptr/mod.rs:175
[task 2020-02-06T21:10:25.131Z] 21:10:25 INFO - GECKO(1431) | #14 0x7ff02615ae62 in core::ptr::real_drop_in_place::h051fe934ecde7af0 /rustc/4560ea788cb760f0a34127156c78e2552949f734/src/libcore/ptr/mod.rs:175
[task 2020-02-06T21:10:25.131Z] 21:10:25 INFO - GECKO(1431) | #15 0x7ff02615ae62 in core::ptr::real_drop_in_place::h84dd6806887eef36 /rustc/4560ea788cb760f0a34127156c78e2552949f734/src/libcore/ptr/mod.rs:175
[task 2020-02-06T21:10:25.132Z] 21:10:25 INFO - GECKO(1431) | #16 0x7ff02615ae62 in servo_arc::Arc$LT$T$GT$::drop_slow::h9c5dbd7dca36e2d8 /builds/worker/workspace/build/src/servo/components/servo_arc/lib.rs:359:41
[task 2020-02-06T21:10:25.291Z] 21:10:25 INFO - GECKO(1431) | #17 0x7ff02648bc8a in _$LT$servo_arc..Arc$LT$T$GT$$u20$as$u20$core..ops..drop..Drop$GT$::drop::h6fff547f5374661d /builds/worker/workspace/build/src/servo/components/servo_arc/lib.rs:545:12
[task 2020-02-06T21:10:25.293Z] 21:10:25 INFO - GECKO(1431) | #18 0x7ff02648bc8a in core::ptr::real_drop_in_place::h5158de196a6717eb /rustc/4560ea788cb760f0a34127156c78e2552949f734/src/libcore/ptr/mod.rs:175
[task 2020-02-06T21:10:25.293Z] 21:10:25 INFO - GECKO(1431) | #19 0x7ff02648bc8a in _$LT$servo_arc..RawOffsetArc$LT$T$GT$$u20$as$u20$core..ops..drop..Drop$GT$::drop::haeccffcb3b9a862b /builds/worker/workspace/build/src/servo/components/servo_arc/lib.rs:1106:10
[task 2020-02-06T21:10:25.294Z] 21:10:25 INFO - GECKO(1431) | #20 0x7ff02648bc8a in core::ptr::real_drop_in_place::h409df5c1c0f7f9b6 /rustc/4560ea788cb760f0a34127156c78e2552949f734/src/libcore/ptr/mod.rs:175
[task 2020-02-06T21:10:25.295Z] 21:10:25 INFO - GECKO(1431) | #21 0x7ff02648bc8a in core::ptr::real_drop_in_place::hee0e8bd78828a117 /rustc/4560ea788cb760f0a34127156c78e2552949f734/src/libcore/ptr/mod.rs:175
[task 2020-02-06T21:10:25.296Z] 21:10:25 INFO - GECKO(1431) | #22 0x7ff02648bc8a in core::ptr::real_drop_in_place::hb7e816efdf0589b7 /rustc/4560ea788cb760f0a34127156c78e2552949f734/src/libcore/ptr/mod.rs:175
[task 2020-02-06T21:10:25.297Z] 21:10:25 INFO - GECKO(1431) | #23 0x7ff02648bc8a in core::ptr::real_drop_in_place::h60573e9529356608 /rustc/4560ea788cb760f0a34127156c78e2552949f734/src/libcore/ptr/mod.rs:175
[task 2020-02-06T21:10:25.297Z] 21:10:25 INFO - GECKO(1431) | #24 0x7ff02648bc8a in core::ptr::real_drop_in_place::h4729a54e0744bc48 /rustc/4560ea788cb760f0a34127156c78e2552949f734/src/libcore/ptr/mod.rs:175
[task 2020-02-06T21:10:25.298Z] 21:10:25 INFO - GECKO(1431) | #25 0x7ff02648bc8a in core::ptr::real_drop_in_place::h84f932839b9c3343 /rustc/4560ea788cb760f0a34127156c78e2552949f734/src/libcore/ptr/mod.rs:175
[task 2020-02-06T21:10:25.299Z] 21:10:25 INFO - GECKO(1431) | #26 0x7ff02648bc8a in servo_arc::Arc$LT$T$GT$::drop_slow::hdb7e1f9fe82f0b84 /builds/worker/workspace/build/src/servo/components/servo_arc/lib.rs:359:41
[task 2020-02-06T21:10:25.303Z] 21:10:25 INFO - GECKO(1431) | #27 0x7ff02662e9b0 in $LT$servo_arc..Arc$LT$T$GT$$u20$as$u20$core..ops..drop..Drop$GT$::drop::h201fabb318270943 /builds/worker/workspace/build/src/servo/components/servo_arc/lib.rs:545:12
[task 2020-02-06T21:10:25.303Z] 21:10:25 INFO - GECKO(1431) | #28 0x7ff02662e9b0 in core::ptr::real_drop_in_place::h0f4ff0f874192268 /rustc/4560ea788cb760f0a34127156c78e2552949f734/src/libcore/ptr/mod.rs:175
[task 2020-02-06T21:10:25.304Z] 21:10:25 INFO - GECKO(1431) | #29 0x7ff02662e9b0 in style::gecko::arc_types::Servo_ComputedStyle_Release::$u7b$$u7b$closure$u7d$$u7d$::h0b838ae5a9056319 /builds/worker/workspace/build/src/servo/components/style/gecko/arc_types.rs:130:49
[task 2020-02-06T21:10:25.305Z] 21:10:25 INFO - GECKO(1431) | #30 0x7ff02662e9b0 in servo_arc::ArcBorrow$LT$T$GT$::with_arc::h3ee4229aed541a27 /builds/worker/workspace/build/src/servo/components/servo_arc/lib.rs:1270:21
[task 2020-02-06T21:10:25.306Z] 21:10:25 INFO - GECKO(1431) | #31 0x7ff02662e9b0 in Servo_ComputedStyle_Release /builds/worker/workspace/build/src/servo/components/style/gecko/arc_types.rs:129:4
[task 2020-02-06T21:10:25.321Z] 21:10:25 INFO - GECKO(1431) | #32 0x7ff0212849f0 in Release /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ServoBindingTypes.h:156:1
[task 2020-02-06T21:10:25.322Z] 21:10:25 INFO - GECKO(1431) | #33 0x7ff0212849f0 in Release /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:379:36
[task 2020-02-06T21:10:25.324Z] 21:10:25 INFO - GECKO(1431) | #34 0x7ff0212849f0 in ~RefPtr /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:81:7

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1606628 - Fix background and border image association to be symmetric. r=mattwoodrow,tnikkel

And add an assertion to ensure callers get it right. The only thing that needs
it AFAICT is the canvas frame background, so remove the border-image caller...

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2b49036045a3
Remove nsStyleImageRequest. r=tnikkel,heycam
https://hg.mozilla.org/integration/autoland/rev/e3382fdd4f54
Fix background and border image association to be symmetric. r=tnikkel

Comment 7

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/2b49036045a3
https://hg.mozilla.org/mozilla-central/rev/e3382fdd4f54

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1606628 - Do track content: url() requests in nsImageFrame. r=tnikkel

Ugh, this is totally an oversight from bug 1606628. In that bug, I removed
basically these two calls (with a different comment).

The reason I thought they weren't needed is because nsStyleImageRequest was
gone, and content started using the same mechanism from the style system as
everything else.

But I didn't notice that content: url() doesn't go through the
AssociateRequestToFrame code path (which is what takes care of associating the
image to the image tracker for other CSS images).

So those changes were just incorrect and this is still needed :(

See bug 1470350 for sadness in animated image testing... I'll try to figure out
how to write some other kind of test for this though...

Comment 9

[Phabricator Automation]

Comment on attachment 9138445 [details]
Bug 1606628 - Do track content: url() requests in nsImageFrame. r=tnikkel

Revision D69767 was moved to bug 1627585. Setting attachment 9138445 [details] to obsolete.