[webkit-reviews] review denied: [Bug 61834] Web Inspector: CRASH if Expanding Event Listener on document : [Attachment 96009] [PATCH] Now With a Better Test

Sat Jun 4 22:13:03 PDT 2011

Pavel Feldman <pfeldman at chromium.org> has denied Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 61834: Web Inspector: CRASH if Expanding Event Listener on document
https://bugs.webkit.org/show_bug.cgi?id=61834

Attachment 96009: [PATCH] Now With a Better Test
https://bugs.webkit.org/attachment.cgi?id=96009&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=96009&action=review

The reason I prefer sniffer to the runAfterPendingDispatches is that it tests
exactly what I expect. runAfterPendingDispatches is called not after a
particular action I am testing, but after its potential implications. Also, it
just makes us run more code while in the test (all subsequent updates for
styles and such) and hence reduce the test robustness.

> LayoutTests/inspector/elements/event-listener-sidebar.html:27
> +	   function testSetupEventListeners(next)

You no longer need neither testSetupEventListeners nor the testSuite sequence
(just selectNode followed with expandAndDump)

> LayoutTests/inspector/elements/event-listener-sidebar.html:51
> +<body onload="runTest()">

you want onloadHandler call here.