Constructing an RTCPeerConnection with expired certificates should throw an exception.

Constructing an RTCPeerConnection with expired certificates should throw an exception.

See spec @ Step 9: https://w3c.github.io/webrtc-pc/archives/20151123/webrtc.html#rtcpeerconnection-interface.
It says to throw an InvalidParameter exception, but there is no such DOMException, I believe a mistake by the spec. Instead I throw an InvalidStateError.

BUG=565278
Committed: https://crrev.com/a007fbd29875ab2bc9f2332288a50e81dac62f2d
Cr-Commit-Position: refs/heads/master@{#388470}

[hbos_chromium, 2016-01-27 10:58:29]

hbos@chromium.org changed reviewers:
+ hta@chromium.org, tommi@chromium.org

[hbos_chromium, 2016-01-27 10:58:30]

Please take a look hta, tommi.

I can't create a test for this without first resolving crbug.com/569005 though
(I added a TODO), so I suppose I can wait with this CL if you want? Only tested
manually.

[hbos_chromium, 2016-01-27 10:59:23]

Description was changed from

==========
Constructing an RTCPeerConnection with expired certificates should throw an
exception.

See spec @ Step 9:
https://w3c.github.io/webrtc-pc/archives/20151123/webrtc.html#rtcpeerconnecti....
It says to throw an InvalidParameter exception, but there is no such
DomException, I believe a mistake by the spec. Instead I throw an
InvalidStateError.

BUG=565278
==========

to

==========
Constructing an RTCPeerConnection with expired certificates should throw an
exception.

See spec @ Step 9:
https://w3c.github.io/webrtc-pc/archives/20151123/webrtc.html#rtcpeerconnecti....
It says to throw an InvalidParameter exception, but there is no such
DOMException, I believe a mistake by the spec. Instead I throw an
InvalidStateError.

BUG=565278
==========

[hbos_chromium, 2016-01-27 11:11:32]

On 2016/01/27 10:58:30, hbos_chromium wrote:
> Please take a look hta, tommi.
> 
> I can't create a test for this without first resolving crbug.com/569005 though
> (I added a TODO), so I suppose I can wait with this CL if you want? Only
tested
> manually.

Actually this will make updateIce (which needs to be renamed to
setConfiguration, separate issue) throw the exception as well. The spec does not
say that it/setConfiguration shall throw an exception. I'll update, you don't
need to take a look yet.

[hta - Chromium, 2016-01-29 07:15:14]

If you can make currentTime injectable (perhaps by isolating the validity
checking to its own function), I think you can write tests for this before
setExpirationTime is ready.

https://codereview.chromium.org/1644553002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/mediastream/RTCPeerConnection.cpp
(right):

https://codereview.chromium.org/1644553002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediastream/RTCPeerConnection.cpp:337:
DOMTimeStamp now = convertSecondsToDOMTimeStamp(currentTime());
My usual caveat on depending on currentTime for stuff that you want to test. It
seems that wtf/CurrentTime() *is* injectable, but no current tests inject it, so
it's probably not likely to work well if you manipulate it for tests of this
function.

[tommi (sloooow) - chröme, 2016-02-15 16:22:49]

has this been moved to a new cl or will work continue in this cl?

[hbos_chromium, 2016-02-15 16:42:10]

On 2016/02/15 16:22:49, tommi-chromium wrote:
> has this been moved to a new cl or will work continue in this cl?

I'll probably just update this CL to have a unit test when generateCertificate
has an expires param. It will be on ice for a while.

[blink-reviews, 2016-02-15 17:17:21]

The spec's updated - it's part of the keygen algorithm parameter.

http://w3c.github.io/webrtc-pc/#widl-RTCPeerConnection-generateCertificate-Pr...


On Mon, Feb 15, 2016 at 5:42 PM, <hbos@chromium.org> wrote:

> On 2016/02/15 16:22:49, tommi-chromium wrote:
> > has this been moved to a new cl or will work continue in this cl?
>
> I'll probably just update this CL to have a unit test when
generateCertificate
> has an expires param. It will be on ice for a while.
> https://codereview.chromium.org/1644553002/
>
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-02-15 17:17:21]

The spec's updated - it's part of the keygen algorithm parameter.

http://w3c.github.io/webrtc-pc/#widl-RTCPeerConnection-generateCertificate-Pr...


On Mon, Feb 15, 2016 at 5:42 PM, <hbos@chromium.org> wrote:

> On 2016/02/15 16:22:49, tommi-chromium wrote:
> > has this been moved to a new cl or will work continue in this cl?
>
> I'll probably just update this CL to have a unit test when
generateCertificate
> has an expires param. It will be on ice for a while.
> https://codereview.chromium.org/1644553002/
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[hta - Chromium, 2016-04-11 08:10:23]

Status update?

[hbos_chromium, 2016-04-12 12:35:18]

On 2016/04/11 08:10:23, hta - Chromium wrote:
> Status update?

I want to land https://codereview.chromium.org/1740993002/ first so I can write
a test without having to inject time by other means.

[hbos_chromium, 2016-04-12 12:45:28]

On 2016/04/12 12:35:18, hbos_chromium wrote:
> On 2016/04/11 08:10:23, hta - Chromium wrote:
> > Status update?
> 
> I want to land https://codereview.chromium.org/1740993002/ first so I can
write
> a test without having to inject time by other means.

I'm going to revisit that CL now.

[hbos_chromium, 2016-04-20 11:08:58]

PTAL hta & tommi.

Rebased with master, ready to land this now.

https://codereview.chromium.org/1644553002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/mediastream/RTCPeerConnection.cpp
(right):

https://codereview.chromium.org/1644553002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/mediastream/RTCPeerConnection.cpp:337:
DOMTimeStamp now = convertSecondsToDOMTimeStamp(currentTime());
On 2016/01/29 07:15:14, hta - Chromium wrote:
> My usual caveat on depending on currentTime for stuff that you want to test.
It
> seems that wtf/CurrentTime() *is* injectable, but no current tests inject it,
so
> it's probably not likely to work well if you manipulate it for tests of this
> function.

The ability to generate expired certificates landed, so I could add a test
without injecting. So the js test is like a real usage case in regards to
timing.

[hta - Chromium, 2016-04-20 11:27:31]

Filed https://github.com/w3c/webrtc-pc/issues/586 about the inability to get
InvalidParameter defined.

[hbos_chromium, 2016-04-20 11:31:26]

On 2016/04/20 11:27:31, hta - Chromium wrote:
> Filed https://github.com/w3c/webrtc-pc/issues/586 about the inability to get
> InvalidParameter defined.

Great, thanks! I should have filed but I had forgotten about that.

[hta - Chromium, 2016-04-20 11:33:22]

lgtm

The edge case of a certificate expiring in the same second as it's being checked
is OK to reject, in my opinion (we can regard it as having expired at the
beginning of the second).

[hbos_chromium, 2016-04-20 11:35:57]

PTAL tommi-chrömium

[tommi (sloooow) - chröme, 2016-04-20 11:52:38]

lööks gööd! lgtm

[hbos_chromium, 2016-04-20 11:54:46]

The CQ bit was checked by hbos@chromium.org

[commit-bot: I haz the power, 2016-04-20 11:54:57]

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

[commit-bot: I haz the power, 2016-04-20 12:07:26]

Description was changed from

==========
Constructing an RTCPeerConnection with expired certificates should throw an
exception.

See spec @ Step 9:
https://w3c.github.io/webrtc-pc/archives/20151123/webrtc.html#rtcpeerconnecti....
It says to throw an InvalidParameter exception, but there is no such
DOMException, I believe a mistake by the spec. Instead I throw an
InvalidStateError.

BUG=565278
==========

to

==========
Constructing an RTCPeerConnection with expired certificates should throw an
exception.

See spec @ Step 9:
https://w3c.github.io/webrtc-pc/archives/20151123/webrtc.html#rtcpeerconnecti....
It says to throw an InvalidParameter exception, but there is no such
DOMException, I believe a mistake by the spec. Instead I throw an
InvalidStateError.

BUG=565278
==========

[commit-bot: I haz the power, 2016-04-20 12:07:27]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-22 19:22:34]

Description was changed from

==========
Constructing an RTCPeerConnection with expired certificates should throw an
exception.

See spec @ Step 9:
https://w3c.github.io/webrtc-pc/archives/20151123/webrtc.html#rtcpeerconnecti....
It says to throw an InvalidParameter exception, but there is no such
DOMException, I believe a mistake by the spec. Instead I throw an
InvalidStateError.

BUG=565278
==========

to

==========
Constructing an RTCPeerConnection with expired certificates should throw an
exception.

See spec @ Step 9:
https://w3c.github.io/webrtc-pc/archives/20151123/webrtc.html#rtcpeerconnecti....
It says to throw an InvalidParameter exception, but there is no such
DOMException, I believe a mistake by the spec. Instead I throw an
InvalidStateError.

BUG=565278

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

[commit-bot: I haz the power, 2016-04-22 19:22:35]

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