Drop [LegacyInterfaceTypeChecking] for URL.createObjectURL(blob)

Drop [LegacyInterfaceTypeChecking] for URL.createObjectURL(blob)

The change to the generated code is such that only calls to
URL.createObjectURL(null) and URL.createObjectURL(undefined) are
affected, as those would previously match the nullable Blob argument.

This is very low risk, due to the behavior of other browsers:

Firefox and IE11 throw for both URL.createObjectURL(null) and
URL.createObjectURL(undefined). Edge presumably matches IE11.

Safari throws for URL.createObjectURL(undefined) but returns null for
URL.createObjectURL(null), which was our behavior before this change.

There are no internal calls to DOMURL::createObjectURL, so the ASSERT
will hold.

BUG=561338
Committed: https://crrev.com/1143a0b24fb3489a358f9f88cda216a039b8463d
Cr-Commit-Position: refs/heads/master@{#363031}

[philipj_slow, 2015-12-03 10:15:19]

The CQ bit was checked by philipj@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-03 10:15:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1492093002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1492093002/20001

[commit-bot: I haz the power, 2015-12-03 11:19:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-03 11:19:11]

Dry run: This issue passed the CQ dry run.

[philipj_slow, 2015-12-03 11:30:28]

philipj@opera.com changed reviewers:
+ jsbell@chromium.org

[philipj_slow, 2015-12-03 11:30:29]

PTAL. The wpt changes because it needs a server for XHR, doesn't get a Blob in
response, and instead calls createObjectURL(null). Would presumably work if
there was a server.

[jsbell, 2015-12-03 17:15:04]

lgtm

Usually for a "this test is broken w/o the correct server" we'd add [Skip] to
LayoutTests/W3CImportExpectations (and re-do the import to drop the file) *or*
[Skip] to TestExpectations to ignore the test - in either case it can reference
a general "needs a server" bug - but I think this is fine in this case, since
the failure gets us some useful coverage.

[philipj_slow, 2015-12-03 19:02:36]

On 2015/12/03 17:15:04, jsbell wrote:
> lgtm
> 
> Usually for a "this test is broken w/o the correct server" we'd add [Skip] to
> LayoutTests/W3CImportExpectations (and re-do the import to drop the file) *or*
> [Skip] to TestExpectations to ignore the test - in either case it can
reference
> a general "needs a server" bug - but I think this is fine in this case, since
> the failure gets us some useful coverage.

It was actually because you added this test expectation in
https://codereview.chromium.org/1236713002 that I asked you for review. Do you
have a rule of thumb for when to add -expected.txt and when to [ Skip ]? I'm
trying to import wpt/dom/ and there are many tests that fail but where adding an
-expected.txt would at least give some coverage of the parts that don't fail.

Will land this as-is.

[philipj_slow, 2015-12-03 19:02:42]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2015-12-03 19:05:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1492093002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1492093002/20001

[jsbell, 2015-12-03 19:13:33]

On 2015/12/03 19:02:36, philipj wrote:
> It was actually because you added this test expectation in
> https://codereview.chromium.org/1236713002 that I asked you for review.

Ha!

> Do you
> have a rule of thumb for when to add -expected.txt and when to [ Skip ]?

If it's an interop bug, -expected.txt. If it's a test harness limitation (e.g.
requires server) or we think the test is wrong, [Skip]. But we're not consistent
since it's a lot of tests to manually inspect. :(

Looking at the CL, I skipped one case but missed this one. *sigh*

[commit-bot: I haz the power, 2015-12-03 19:17:56]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-12-03 19:18:50]

Description was changed from

==========
Drop [LegacyInterfaceTypeChecking] for URL.createObjectURL(blob)

The change to the generated code is such that only calls to
URL.createObjectURL(null) and URL.createObjectURL(undefined) are
affected, as those would previously match the nullable Blob argument.

This is very low risk, due to the behavior of other browsers:

Firefox and IE11 throw for both URL.createObjectURL(null) and
URL.createObjectURL(undefined). Edge presumably matches IE11.

Safari throws for URL.createObjectURL(undefined) but returns null for
URL.createObjectURL(null), which was our behavior before this change.

There are no internal calls to DOMURL::createObjectURL, so the ASSERT
will hold.

BUG=561338
==========

to

==========
Drop [LegacyInterfaceTypeChecking] for URL.createObjectURL(blob)

The change to the generated code is such that only calls to
URL.createObjectURL(null) and URL.createObjectURL(undefined) are
affected, as those would previously match the nullable Blob argument.

This is very low risk, due to the behavior of other browsers:

Firefox and IE11 throw for both URL.createObjectURL(null) and
URL.createObjectURL(undefined). Edge presumably matches IE11.

Safari throws for URL.createObjectURL(undefined) but returns null for
URL.createObjectURL(null), which was our behavior before this change.

There are no internal calls to DOMURL::createObjectURL, so the ASSERT
will hold.

BUG=561338

Committed: https://crrev.com/1143a0b24fb3489a358f9f88cda216a039b8463d
Cr-Commit-Position: refs/heads/master@{#363031}
==========

[commit-bot: I haz the power, 2015-12-03 19:18:51]

Patchset 2 (id:??) landed as
https://crrev.com/1143a0b24fb3489a358f9f88cda216a039b8463d
Cr-Commit-Position: refs/heads/master@{#363031}

[philipj_slow, 2015-12-03 19:27:45]

On 2015/12/03 19:13:33, jsbell wrote:
> On 2015/12/03 19:02:36, philipj wrote:
> > Do you
> > have a rule of thumb for when to add -expected.txt and when to [ Skip ]?
> 
> If it's an interop bug, -expected.txt. If it's a test harness limitation (e.g.
> requires server) or we think the test is wrong, [Skip]. But we're not
consistent
> since it's a lot of tests to manually inspect. :(

That sounds good, I'll try to apply that rule of thumb to my WIP CL to import
wpt/dom/, it should result it a lot fewer skipped tests.