Bug 373610 (refdyn)

Bugs found by comparing renderings with and without dynamic changes

NEW
Assigned to

Status

()

defect
13 years ago
2 years ago

People

(Reporter: jruderman, Assigned: jruderman)

Tracking

(Depends on 60 bugs, {meta})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 obsolete attachments)

This reftest-based script exhaustively tests how rendering responds to simple DOM changes.  For example, it tries removing a node, forcing a complete relayout of the page, and putting the node back.    It compares the two screenshots with the node missing to ensure that the removeChild operation is handled properly, and compares the two screenshots with the node present to ensure that the insertBefore operation is handled properly.

It tests removeChild/insertBefore for every node in the document.  It also tests attribute removal/setting and text node changes.

Running it on all the reftest files (excluding the pixel-rounding directory) takes about 10 hours on my MacBook Pro, so it's too slow to be run automatically after every checkin.  About 80% of the time is spent in PNG compression.  Is there a way to avoid PNG compression in this kind of test?

It has found 15 bugs so far, mostly incorrect-rendering bugs and a few bugs in reading the style attribute.  But I'm filing this as security-sensitive for now because I'm worried it might find more severe bugs on branches and because my experience with bug 349611 makes me feel I should be cautious.
Posted file refdyn.js (obsolete) —
Whiteboard: [sg:nse meta]
This is a great idea.
It seems to me, a lot of bugs could also be found by comparing the offsetWidth/offsetHeight/computed width/computed height, not?
In that case, you could avoid the slow image comparison.
Jesse, even if it takes 10 hours, we could (and probably should) still run the test nightly if we had appropriate automation. cc'ing pav/vlad about avoiding PNG compression. It probably wouldn't be that hard to add a parameter to avoid compression.
cool idea, Jesse. We should figure out how to get this running regularly on the test farm. I'll talk to you later about some ideas on irc.
The best way to avoid the PNG compression would be to write a small image comparison function in C++ rather than doing the image comparison via PNG data: URLs.  It's something that's on my mental todo list for speeding up reftest, though there should probably be a bug filed on it.  (We probably would still want to convert to data: URLs when reporting failures, but that's rare.)
Can the image comparison be done with getImageData and in JS?  Not sure what the perf would be like, but something to consider would be to use a much smaller basea size for the tests, or at least have a test flag that indicates that a test needs a large canvas vs. the default (which should be small, like 100x100).  A C++ thing could work fine as well.
I noticed in reftest.js that setAttribute is explicitly called on the Canvas to set its height and width. Does this necessarily need to be set to the size of the window, or can it be set to capture only a small square of the window? Or, can the width of the window be set here

If this size could be made a parameter of the test, it would be most convenient.
Alias: refdyn
Depends on: 375493
Depends on: 375497
Depends on: 381277
Depends on: 381279
Depends on: 382146
Depends on: 385607
Posted file refdyn.js (obsolete) —
Attachment #258275 - Attachment is obsolete: true
Depends on: 387510
Depends on: 395125
Depends on: 395130
Depends on: 377519
Depends on: 395155
Depends on: 398092
Depends on: 398095
Depends on: 398101
Depends on: 398105
Posted file refdyn.zip (obsolete) —
Now much faster thanks to bug 387132.  Also contains updated exclusions.
Attachment #270363 - Attachment is obsolete: true
Depends on: 398681
Depends on: 398682
Depends on: 398685
Depends on: 398686
Depends on: 398809
Depends on: 398820
Depends on: 398830
Depends on: 398884
Depends on: 401580
Depends on: 403733
Depends on: 405517
Depends on: 407095
Depends on: 407106
Depends on: 407115
Depends on: 407397
Depends on: 407419
Depends on: 408782
Depends on: 409051
Depends on: 409056
Depends on: 409065
Depends on: 409089
Depends on: 409125
Depends on: 409494
Depends on: 409577
Posted file refdyn.zip (obsolete) —
Updated to understand the current reftest.list format.  Many new exclusions.
Attachment #283658 - Attachment is obsolete: true
The bugs that force me to exclude the most tests are:

* Bug 162063 - Tables
* Bug 409089 - -moz-box
* Bug 409125 - MathML
* Bug 229915 - CSS + combinator
* Bug 373298 - :-moz-first-node 
* Bug 145419 - dynamically added ::first-letter and ::first-line rules
Depends on: 411367
Depends on: 411374
Depends on: 412352
Depends on: 412365
Depends on: 418574
Depends on: 418756
Depends on: 418766
Depends on: 421234
Depends on: 421239
Depends on: 421419
Depends on: 421425
After finding dozens of harmless layout inconsistencies, refdyn finally found a potential security hole!  Bug 421234 causes a random character to appear, and I'm guessing it comes from uninitialized memory.
Posted file refdyn.zip (obsolete) —
Attachment #294382 - Attachment is obsolete: true
Depends on: 423130
Depends on: 429974
Depends on: 429976
Depends on: 442630
Depends on: 442633
Depends on: 450693
Depends on: 467312
Depends on: 467321
Depends on: 467323
Depends on: 467460
Depends on: 467472
Depends on: 467498
Depends on: 467719
Depends on: 467722
Depends on: 467723
Depends on: 475642
Depends on: 475644
Depends on: 475647
Depends on: 478490
Depends on: 478511
Depends on: 478594
Depends on: 489868
Depends on: 489877
Depends on: 489887
Depends on: 489890
Depends on: 490173
Depends on: 490174
Depends on: 490176
Depends on: 490177
Depends on: 490182
Depends on: 490183
Depends on: 490185
Depends on: 490216
Depends on: 490218
Depends on: 490220
Depends on: 492231
Depends on: 492239
Depends on: 492240
Depends on: 492661
Refdyn has only found two security holes among about a hundred total bugs.  I'd like to make it public eventually, but one of those bugs (bug 467323) isn't fixed on 3.0.x, so it might be a while.
Attachment #307930 - Attachment is obsolete: true
Depends on: 493564
Depends on: 501035
Depends on: 501037
Depends on: 501048
Depends on: 501049
Depends on: 521525
Depends on: 521527
Depends on: 521539
Depends on: 521542
Depends on: 521594
Depends on: 521600
Depends on: 521602
Depends on: 521607
Depends on: 521609
Depends on: 521682
Depends on: 521685
Depends on: 521689
Depends on: 521720
Depends on: 521875
Depends on: 522390
Depends on: 522393
Depends on: 526536
Depends on: 526596
Depends on: 526602
Depends on: 526634
Depends on: 534526
Depends on: 534793
Depends on: 534802
Depends on: 534806
Depends on: 534808
Depends on: 534811
Depends on: 537875
Depends on: 543791
Depends on: 549797
Depends on: 550047
Depends on: 550065
Depends on: 550661
Depends on: 551239
Depends on: 551838
I've revived this tool by making it part of the DOM fuzzer. Now it makes a number of random dynamic changes before checking that the resulting dynamic rendering matches the static rendering of the same DOM tree.
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [sg:nse meta]
Depends on: 726548
Depends on: 728100
Depends on: 731521
Depends on: 732740
Depends on: 752779
Depends on: 763560
Depends on: 764256
Depends on: 767233
Depends on: 767279
Depends on: 1036750
Depends on: 1156244
Depends on: 1258076
Component: Tracking → Platform Fuzzing Team
You need to log in before you can comment on or make changes to this bug.