Bug 77487 – Make js-test-pre.js more worker friendly

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[David Grogan]

Make js-test-pre.js more worker friendly

[David Grogan]

Created attachment 124840 [details]
Patch

[David Grogan]

Ojan, could you review this?

[Ojan Vafai]

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]

Created attachment 125200 [details]
Patch

[David Grogan]

(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 on attachment 125200 [details]
Patch

Clearing flags on attachment: 125200

Committed r106684: <http://trac.webkit.org/changeset/106684>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.