Bug 143731 – Make sure media element loads hit content filter extensions

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Brady Eidson]

Make sure media element loads hit content filter extensions

rdar://problem/20014012

[Brady Eidson]

Created attachment 250746 [details]
Patch v1

[Jer Noble]

Comment on attachment 250746 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=250746&action=review

> LayoutTests/http/tests/contentextensions/media-filtered.html:3
> +if (window.testRunner)
> +    testRunner.waitUntilDone();

This should probably also call 'testRunner.dumpAsText()' so that the test doesn't start failing when there's an unrelated font or layout change.

> LayoutTests/http/tests/contentextensions/media-filtered.html:17
> +function videoError()
> +{
> +    console.log("Video error");
> +    if (window.testRunner)
> +        testRunner.notifyDone();
> +}

video-test.js has these nice functions called `waitForEventAndEnd()` and `waitForEventAndFail()` which would sure be handy here.

> LayoutTests/http/tests/contentextensions/text-track-blocked.html:3
> +    testRunner.waitUntilDone();

Ditto.

> LayoutTests/http/tests/contentextensions/text-track-blocked.html:17
> +function vttLoaded()
> +{
> +    console.log("vttLoaded");
> +    if (window.testRunner)
> +        testRunner.notifyDone();
> +}
> +
> +function vttError()
> +{
> +    console.log("vttError");
> +    if (window.testRunner)
> +        testRunner.notifyDone();
> +}

Ditto about `waitForEventAndEnd()` and `waitForEventAndFail()`.

[Brady Eidson]

(In reply to comment #2)
> Comment on attachment 250746 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=250746&action=review
> 
> > LayoutTests/http/tests/contentextensions/media-filtered.html:3
> > +if (window.testRunner)
> > +    testRunner.waitUntilDone();
> 
> This should probably also call 'testRunner.dumpAsText()' so that the test
> doesn't start failing when there's an unrelated font or layout change.

I wanted to also capture the default size of RenderVideo as opposed to the actual video size as further evidence of load/not load. We've had bugs where a resource loads but sends onerror instead of onload, and this helps.
> 
> > LayoutTests/http/tests/contentextensions/media-filtered.html:17
> > +function videoError()
> > +{
> > +    console.log("Video error");
> > +    if (window.testRunner)
> > +        testRunner.notifyDone();
> > +}
> 
> video-test.js has these nice functions called `waitForEventAndEnd()` and
> `waitForEventAndFail()` which would sure be handy here.

I'm totally fine with (and prefer) keeping all the code for a test self contained for small tests like these :)

Thanks for the review!

[WebKit Commit Bot]

Comment on attachment 250746 [details]
Patch v1

Clearing flags on attachment: 250746

Committed r182820: <http://trac.webkit.org/changeset/182820>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.