RESOLVED FIXED 77487
Make js-test-pre.js more worker friendly
https://bugs.webkit.org/show_bug.cgi?id=77487
Summary Make js-test-pre.js more worker friendly
David Grogan
Reported 2012-01-31 16:02:58 PST
Make js-test-pre.js more worker friendly
Attachments
Patch (2.67 KB, patch)
2012-01-31 16:06 PST, David Grogan
no flags
Patch (2.65 KB, patch)
2012-02-02 15:38 PST, David Grogan
no flags
David Grogan
Comment 1 2012-01-31 16:06:34 PST
David Grogan
Comment 2 2012-01-31 16:19:47 PST
Ojan, could you review this?
Ojan Vafai
Comment 3 2012-01-31 16:31:55 PST
Comment on attachment 124840 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=124840&action=review > LayoutTests/fast/js/resources/js-test-pre.js:3 > +if (!isWorker() && window.layoutTestController) > layoutTestController.dumpAsText(window.enablePixelTesting); Why not just use self.layoutTestController and self.enablePixelTesting? Then you don't need to check or move isWorker. Similarly, can you change all uses of window in this file to self for consistency?
David Grogan
Comment 4 2012-02-02 15:38:43 PST
David Grogan
Comment 5 2012-02-02 15:42:18 PST
(In reply to comment #3) > (From update of attachment 124840 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=124840&action=review > > > LayoutTests/fast/js/resources/js-test-pre.js:3 > > +if (!isWorker() && window.layoutTestController) > > layoutTestController.dumpAsText(window.enablePixelTesting); > > Why not just use self.layoutTestController and self.enablePixelTesting? Then you don't need to check or move isWorker. Similarly, can you change all uses of window in this file to self for consistency? I was thinking that 'window' is clearer than 'self' so wanted to leave 'window' if possible. But you're right, the consistency of 'self' everywhere is better.
WebKit Review Bot
Comment 6 2012-02-03 12:46:53 PST
Comment on attachment 125200 [details] Patch Clearing flags on attachment: 125200 Committed r106684: <http://trac.webkit.org/changeset/106684>
WebKit Review Bot
Comment 7 2012-02-03 12:46:58 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.