Make required dictionary members non-nullable again

Make required dictionary members non-nullable again

These were changed to non-nullable to preserve existing behavior.
Because they are required, the risk here is that developers have noticed
they were required but successfully found and used the null loophole. At
the slightest sign of trouble with any of these, it should be reverted
and perhaps the specs should be changed to allow null.

BlobEvent test will be imported from:
https://github.com/w3c/web-platform-tests/pull/3749

PresentationConnectionAvailableEvent is untested:
https://crbug.com/648922

BUG=647693
Committed: https://crrev.com/bb539825927adfa6f082056d63a9a0c47be8a3b4
Cr-Commit-Position: refs/heads/master@{#420285}

[foolip, 2016-09-16 16:29:59]

foolip@chromium.org changed reviewers:
+ rbyers@chromium.org

[foolip, 2016-09-16 16:30:00]

Depends on https://codereview.chromium.org/2342393002/

Do you think we should tread carefully with this, with use counters and launch
process? These are all fairly new things and I'd expect even non-throwing usage
to be low. My hunch is that TouchInit is the riskiest, WDYT?

[haraken, 2016-09-17 00:01:06]

On 2016/09/16 16:30:00, foolip wrote:
> Depends on https://codereview.chromium.org/2342393002/
> 
> Do you think we should tread carefully with this, with use counters and launch
> process? These are all fairly new things and I'd expect even non-throwing
usage
> to be low. My hunch is that TouchInit is the riskiest, WDYT?

How do other browsers behave?

[Rick Byers, 2016-09-17 19:48:44]

I couldn't find the history of this issue by looking at the bug and related
bugs.  Is it the case that we've long enforced 'required' dictionary members,
but we've incorrectly allowed them to have the null value - and have done so for
at least a stable version or more?  I.e. when you say "relatively recent" you
just mean these APIs are new, not that we had a recent regression that started
to allow 'null', right?

I'd be surprised if anyone was relying on this intentionally.  It's possible
someone might, for example, accidentally copy null node value into TouchInit,
but still I think it's pretty unlikely.  We make bug fixes to argument
validation logic all the time, and it would be crazy to go through a full UMA /
intent process for each of those.

So, assuming at least 2 other browsers don't allow 'null' here then I'd say the
risk is tiny and this qualifies as a "trivial" web platform change (just a bug
fix, no doc updates, etc.).  So LGTM assuming that appears (from one or two
spot-checks) to be the case.

If null is allowed by the other 3 major browsers then maybe it's worth a little
more care?

[foolip, 2016-09-21 12:32:01]

Description was changed from

==========
Make required dictionary members non-nullable again

These were changed to non-nullable to preserve existing behavior.
Because they are required, the risk here is that developers have noticed
they were required but successfully found and used the null loophole. At
the slightest sign of trouble with any of these, it should be reverted
and perhaps the specs should be changed to allow null.

BUG=647693
==========

to

==========
Make required dictionary members non-nullable again

These were changed to non-nullable to preserve existing behavior.
Because they are required, the risk here is that developers have noticed
they were required but successfully found and used the null loophole. At
the slightest sign of trouble with any of these, it should be reverted
and perhaps the specs should be changed to allow null.

BlobEvent test will be imported from:
https://github.com/w3c/web-platform-tests/pull/3749

BUG=647693
==========

[foolip, 2016-09-21 12:32:44]

Description was changed from

==========
Make required dictionary members non-nullable again

These were changed to non-nullable to preserve existing behavior.
Because they are required, the risk here is that developers have noticed
they were required but successfully found and used the null loophole. At
the slightest sign of trouble with any of these, it should be reverted
and perhaps the specs should be changed to allow null.

BlobEvent test will be imported from:
https://github.com/w3c/web-platform-tests/pull/3749

BUG=647693
==========

to

==========
Make required dictionary members non-nullable again

These were changed to non-nullable to preserve existing behavior.
Because they are required, the risk here is that developers have noticed
they were required but successfully found and used the null loophole. At
the slightest sign of trouble with any of these, it should be reverted
and perhaps the specs should be changed to allow null.

BlobEvent test will be imported from:
https://github.com/w3c/web-platform-tests/pull/3749

PresentationConnectionAvailableEvent is untested:
https://crbug.com/648922

BUG=647693
==========

[foolip, 2016-09-21 13:18:56]

The CQ bit was checked by foolip@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-21 13:19:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[foolip, 2016-09-21 13:22:24]

On 2016/09/17 19:48:44, Rick Byers (slow until 9-26) wrote:
> I couldn't find the history of this issue by looking at the bug and related
> bugs.  Is it the case that we've long enforced 'required' dictionary members,
> but we've incorrectly allowed them to have the null value - and have done so
for
> at least a stable version or more?  I.e. when you say "relatively recent" you
> just mean these APIs are new, not that we had a recent regression that started
> to allow 'null', right?
> 
> I'd be surprised if anyone was relying on this intentionally.  It's possible
> someone might, for example, accidentally copy null node value into TouchInit,
> but still I think it's pretty unlikely.  We make bug fixes to argument
> validation logic all the time, and it would be crazy to go through a full UMA
/
> intent process for each of those.
> 
> So, assuming at least 2 other browsers don't allow 'null' here then I'd say
the
> risk is tiny and this qualifies as a "trivial" web platform change (just a bug
> fix, no doc updates, etc.).  So LGTM assuming that appears (from one or two
> spot-checks) to be the case.
> 
> If null is allowed by the other 3 major browsers then maybe it's worth a
little
> more care?

I backed out the TouchInit change in favor of
https://codereview.chromium.org/2352333002

For the rest, I did HTTP Archive searches for their use as constructors and
found literally nothing, so I think it's very low risk. I didn't do a full
survey of support in other browsers, but did find that Firefox has BlobEvent and
will fail https://github.com/w3c/web-platform-tests/pull/3749 (can you LGTM that
too?)

The test related to ForeignFetchResponse would actually crash before this
change, see assumptions made in
ForeignFetchRespondWithObserver::responseWasFulfilled

[commit-bot: I haz the power, 2016-09-21 14:24:21]

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

[commit-bot: I haz the power, 2016-09-21 14:24:22]

Dry run: This issue passed the CQ dry run.

[mlamouri (slow - plz ping), 2016-09-22 08:48:16]

mlamouri@chromium.org changed reviewers:
+ mlamouri@chromium.org

[mlamouri (slow - plz ping), 2016-09-22 08:48:17]

lgtm

[foolip, 2016-09-22 08:49:08]

The CQ bit was checked by foolip@chromium.org

[foolip, 2016-09-22 08:49:08]

The patchset sent to the CQ was uploaded after l-g-t-m from rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/2347473005/#ps20001
(title: "rm TouchInit, add tests")

[commit-bot: I haz the power, 2016-09-22 08:49:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-22 08:54:28]

Description was changed from

==========
Make required dictionary members non-nullable again

These were changed to non-nullable to preserve existing behavior.
Because they are required, the risk here is that developers have noticed
they were required but successfully found and used the null loophole. At
the slightest sign of trouble with any of these, it should be reverted
and perhaps the specs should be changed to allow null.

BlobEvent test will be imported from:
https://github.com/w3c/web-platform-tests/pull/3749

PresentationConnectionAvailableEvent is untested:
https://crbug.com/648922

BUG=647693
==========

to

==========
Make required dictionary members non-nullable again

These were changed to non-nullable to preserve existing behavior.
Because they are required, the risk here is that developers have noticed
they were required but successfully found and used the null loophole. At
the slightest sign of trouble with any of these, it should be reverted
and perhaps the specs should be changed to allow null.

BlobEvent test will be imported from:
https://github.com/w3c/web-platform-tests/pull/3749

PresentationConnectionAvailableEvent is untested:
https://crbug.com/648922

BUG=647693
==========

[commit-bot: I haz the power, 2016-09-22 08:54:29]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-22 08:56:36]

Description was changed from

==========
Make required dictionary members non-nullable again

These were changed to non-nullable to preserve existing behavior.
Because they are required, the risk here is that developers have noticed
they were required but successfully found and used the null loophole. At
the slightest sign of trouble with any of these, it should be reverted
and perhaps the specs should be changed to allow null.

BlobEvent test will be imported from:
https://github.com/w3c/web-platform-tests/pull/3749

PresentationConnectionAvailableEvent is untested:
https://crbug.com/648922

BUG=647693
==========

to

==========
Make required dictionary members non-nullable again

These were changed to non-nullable to preserve existing behavior.
Because they are required, the risk here is that developers have noticed
they were required but successfully found and used the null loophole. At
the slightest sign of trouble with any of these, it should be reverted
and perhaps the specs should be changed to allow null.

BlobEvent test will be imported from:
https://github.com/w3c/web-platform-tests/pull/3749

PresentationConnectionAvailableEvent is untested:
https://crbug.com/648922

BUG=647693

Committed: https://crrev.com/bb539825927adfa6f082056d63a9a0c47be8a3b4
Cr-Commit-Position: refs/heads/master@{#420285}
==========

[commit-bot: I haz the power, 2016-09-22 08:56:37]

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