Check that Service Workers are registered from secure origins.

Check that Service Workers are registered from secure origins.

This is in keeping with the Chromium proposal to provide powerful new
web platform features to secure origins:

http://www.chromium.org/Home/chromium-security/prefer-secure-origins-for-powerful-new-features

This is also in keeping with the Service Worker spec, which recommends
Service Workers use HTTPS, with some exceptions:

https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#security-considerations

BUG=394213
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178728

[dominicc (has gone to gerrit), 2014-07-18 09:23:05]

PTAL

[Michael van Ouwerkerk, 2014-07-18 09:37:57]

https://codereview.chromium.org/400903002/diff/20001/Source/modules/servicewo...
File Source/modules/serviceworkers/MockWebServiceWorkerProvider.h (right):

https://codereview.chromium.org/400903002/diff/20001/Source/modules/servicewo...
Source/modules/serviceworkers/MockWebServiceWorkerProvider.h:9: #include
<gmock/gmock.h>
Drive-by nit: it seems quite uncommon to use gmock in third_party/WebKit - is
this the current best practice?

[dominicc (has gone to gerrit), 2014-07-18 09:49:43]

Best I can discern from email threads is that opinion is mixed.

I could certainly live without it, maybe I should try it that way. When my
keyboard wears out I can just get another one...
On Jul 18, 2014 6:37 PM, <mvanouwerkerk@chromium.org> wrote:

>
> https://codereview.chromium.org/400903002/diff/20001/
> Source/modules/serviceworkers/MockWebServiceWorkerProvider.h
> File Source/modules/serviceworkers/MockWebServiceWorkerProvider.h
> (right):
>
> https://codereview.chromium.org/400903002/diff/20001/
> Source/modules/serviceworkers/MockWebServiceWorkerProvider.h#newcode9
> Source/modules/serviceworkers/MockWebServiceWorkerProvider.h:9: #include
> <gmock/gmock.h>
> Drive-by nit: it seems quite uncommon to use gmock in third_party/WebKit
> - is this the current best practice?
>
> https://codereview.chromium.org/400903002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[horo, 2014-07-22 06:18:23]

https://codereview.chromium.org/400903002/diff/60001/Source/modules/servicewo...
File Source/modules/serviceworkers/ServiceWorkerContainer.cpp (right):

https://codereview.chromium.org/400903002/diff/60001/Source/modules/servicewo...
Source/modules/serviceworkers/ServiceWorkerContainer.cpp:151: if
(!documentOrigin->canAccessFeatureRequiringSecureOrigin()) {
Why do you check it here?
I think we don't need to check it in unregisterServiceWorker().

[dominicc (has gone to gerrit), 2014-07-22 07:07:14]

On 2014/07/22 06:18:23, horo wrote:
>
https://codereview.chromium.org/400903002/diff/60001/Source/modules/servicewo...
> File Source/modules/serviceworkers/ServiceWorkerContainer.cpp (right):
> 
>
https://codereview.chromium.org/400903002/diff/60001/Source/modules/servicewo...
> Source/modules/serviceworkers/ServiceWorkerContainer.cpp:151: if
> (!documentOrigin->canAccessFeatureRequiringSecureOrigin()) {
> Why do you check it here?
> I think we don't need to check it in unregisterServiceWorker().

It's a bug to call unregister from a non-secure origin and expect it to do
anything. Producing the error makes it clearer what's happening than silently
doing nothing.

[horo, 2014-07-22 13:00:49]

On 2014/07/22 07:07:14, dominicc wrote:
> On 2014/07/22 06:18:23, horo wrote:
> >
>
https://codereview.chromium.org/400903002/diff/60001/Source/modules/servicewo...
> > File Source/modules/serviceworkers/ServiceWorkerContainer.cpp (right):
> > 
> >
>
https://codereview.chromium.org/400903002/diff/60001/Source/modules/servicewo...
> > Source/modules/serviceworkers/ServiceWorkerContainer.cpp:151: if
> > (!documentOrigin->canAccessFeatureRequiringSecureOrigin()) {
> > Why do you check it here?
> > I think we don't need to check it in unregisterServiceWorker().
> 
> It's a bug to call unregister from a non-secure origin and expect it to do
> anything. Producing the error makes it clearer what's happening than silently
> doing nothing.

OK, I got it.

lgtm

https://codereview.chromium.org/400903002/diff/60001/Source/modules/servicewo...
File Source/modules/serviceworkers/ServiceWorkerContainerTest.cpp (right):

https://codereview.chromium.org/400903002/diff/60001/Source/modules/servicewo...
Source/modules/serviceworkers/ServiceWorkerContainerTest.cpp:29: using namespace
WebCore;
s/WebCore/blink/

https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/gGqYHU5yvrw

[dominicc (has gone to gerrit), 2014-07-22 22:39:27]

The CQ bit was checked by dominicc@chromium.org

[commit-bot: I haz the power, 2014-07-22 22:40:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dominicc@chromium.org/400903002/80001

[commit-bot: I haz the power, 2014-07-23 00:02:07]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/16626)

[commit-bot: I haz the power, 2014-07-23 01:53:16]

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

[commit-bot: I haz the power, 2014-07-23 01:53:16]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[dominicc (has gone to gerrit), 2014-07-23 02:10:03]

The CQ bit was checked by dominicc@chromium.org

[commit-bot: I haz the power, 2014-07-23 02:10:26]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dominicc@chromium.org/400903002/80001

[commit-bot: I haz the power, 2014-07-23 03:47:17]

Change committed as 178728