Closed Bug 1488599 Opened 2 years ago Closed 2 years ago

Deleted frames are not removed from will-change and AGR budgets when retaining display list

Categories

(Core :: Web Painting, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: miko, Assigned: miko)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Frames may be added to these budgets during display list building, and they remain there after being deleted.
Blocks: RDLbugs
Depends on D5245
Comment on attachment 9007172 [details]
Bug 1488599 - Part 1: Add RetainedDisplayListData that will store frame invalidation information

Matt Woodrow (:mattwoodrow) has approved the revision.
Attachment #9007172 - Flags: review+
Comment on attachment 9007174 [details]
Bug 1488599 - Part 2: Fix will-change budget

Matt Woodrow (:mattwoodrow) has approved the revision.
Attachment #9007174 - Flags: review+
Pushed by mikokm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0f40b48ed3b4
Part 1: Add RetainedDisplayListData that will store frame invalidation information r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/ed780c8fd413
Part 2: Fix will-change budget r=mattwoodrow
Backout by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/88b67d6b6a46
Backed out 2 changesets for bustages at /layout/painting/nsDisplayList.cpp on a CLOSED TREE
Pushed by mikokm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/bba3a8028837
Part 1: Add RetainedDisplayListData that will store frame invalidation information r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/b08b9f2693cd
Part 2: Fix will-change budget r=mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/bba3a8028837
https://hg.mozilla.org/mozilla-central/rev/b08b9f2693cd
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Depends on: 1492034
Depends on: 1492053
Backout by aciure@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/30f9c0bd088b
Backed out 2 changesets for causing crashes a=backout
Backed out 2 changesets (Bug 1488599) for causing crashes as requested by jcristau a=backout

1492031 1492034 and 1492053 are the crash bugs

Backout push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&resultStatus=testfailed,busted,exception,usercancel,runnable&revision=30f9c0bd088b4180a6b9ebd38144597b1b01e1cd
Status: RESOLVED → REOPENED
Flags: needinfo?(mikokm)
Resolution: FIXED → ---
Target Milestone: mozilla64 → ---
(In reply to Eliza Balazs [:ebalazs_] from comment #10)
> Backed out 2 changesets (Bug 1488599) for causing crashes as requested by
> jcristau a=backout
> 
> 1492031 1492034 and 1492053 are the crash bugs
> 
> Backout push:
> https://treeherder.mozilla.org/#/jobs?repo=mozilla-
> central&resultStatus=testfailed,busted,exception,usercancel,
> runnable&revision=30f9c0bd088b4180a6b9ebd38144597b1b01e1cd

I will fix the bugs here before relanding.
Flags: needinfo?(mikokm)
Pushed by mikokm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/26c600be85c2
Part 1: Add RetainedDisplayListData that will store frame invalidation information r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/7a209f0db01a
Part 2: Fix will-change budget r=mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/26c600be85c2
https://hg.mozilla.org/mozilla-central/rev/7a209f0db01a
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Hi Matt, is there a plan for adding automated tests that can catch this problem before it hits our end-users in future? Thanks!
Flags: needinfo?(matt.woodrow)
(In reply to Ritu Kothari (:ritu) from comment #14)
> Hi Matt, is there a plan for adding automated tests that can catch this
> problem before it hits our end-users in future? Thanks!

Answering on behalf of Matt, since I am the author of this patch. The problem was a UAF bug that was not caught during review, static analysis, or two passes of try runs[1][2]. Due to random nature of these bugs, they can be extremely difficult to find and may be intermittent. We have ASAN builds and fuzzing in place to catch these early in the Nightly.

Aside from switching to Rust or other more memory safe language, there is probably not much that can be done.

[1]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=baeb17af157d42f4596a39306fe713429bab06de
[2]: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b08b9f2693cd4b0d57cd6899deb541a93616c3f4
Flags: needinfo?(matt.woodrow)
(In reply to Ritu Kothari (:ritu) from comment #14)
> Hi Matt, is there a plan for adding automated tests that can catch this
> problem before it hits our end-users in future? Thanks!

This specific bug might have been caught with automated testing suite simulating browsing with enough web sites (say Alexa 500), for example with geckodriver. Running this kind of suite per patch basis would probably take too much resources, but it could be used to decide whether to build Nightly or not.
Depends on: 1509579
You need to log in before you can comment on or make changes to this bug.