CSP3: Implement 'worker-src'.

CSP3: Implement 'worker-src'.

As a drive-by, this also renames `allowChildFrameFromSource` to
`allowFrameFromSource`, and `allowChildContextFromSource` to
`allowWorkerFromSource` to reflect their usage.

Intent to Ship: https://groups.google.com/a/chromium.org/d/msg/blink-dev/1UkZE-vOROc/gEj7psewAAAJ

BUG=662930
Committed: https://crrev.com/586705638164b07863229f70fba19fa2571b48af
Cr-Commit-Position: refs/heads/master@{#431228}

[Mike West, 2016-11-07 16:09:25]

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

[commit-bot: I haz the power, 2016-11-07 16:09:46]

Dry run: 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-11-07 17:48:23]

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

[commit-bot: I haz the power, 2016-11-07 17:48:24]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Mike West, 2016-11-08 08:45:17]

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

[commit-bot: I haz the power, 2016-11-08 08:45:28]

Dry run: 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-11-08 11:33:03]

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

[commit-bot: I haz the power, 2016-11-08 11:33:04]

Dry run: This issue passed the CQ dry run.

[Mike West, 2016-11-08 12:03:41]

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

[commit-bot: I haz the power, 2016-11-08 12:03:57]

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

[Mike West, 2016-11-08 12:07:49]

mkwst@chromium.org changed reviewers:
+ estark@chromium.org, foolip@chromium.org, jochen@chromium.org

[Mike West, 2016-11-08 12:07:50]

Looping in reviewers while folks review the Intent to Implement and Ship at
https://groups.google.com/a/chromium.org/d/msg/blink-dev/1UkZE-vOROc/gEj7psew...

foolip@: Would you mind looking at the tests? I intend to upstream them, which
more or less boils down to replacing the references to `http://127.0.0.1:8000`
with `http://{{host}}:{{ports[http][0]}}` and renaming to `.sub.html`. WDYT?

jochen@, estark@: When y'all wake up, would you mind looking through the changes
to CSP? It's ~20 lines of actual code (much of which is renaming), and a bit
more for tests. Thanks!

[Mike West, 2016-11-08 12:08:59]

Description was changed from

==========
CSP3: Implement 'worker-src'.

BUG=662930
==========

to

==========
CSP3: Implement 'worker-src'.

As a drive-by, this also renames `allowChildFrameFromSource` to
`allowFrameFromSource`, and `allowChildContextFromSource` to
`allowWorkerFromSource` to reflect their usage.

Intent to Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/1UkZE-vOROc/gEj7psew...

BUG=662930
==========

[commit-bot: I haz the power, 2016-11-08 13:38:17]

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

[commit-bot: I haz the power, 2016-11-08 13:38:20]

Dry run: This issue passed the CQ dry run.

[estark, 2016-11-09 06:16:32]

Sorry Mike, I didn't quite get to this today and now I can't really process it
through my tears and misery. Left a question inline and will take a closer look
tomorrow.

https://codereview.chromium.org/2480303002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right):

https://codereview.chromium.org/2480303002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:436: else if
(ContentSecurityPolicy::WorkerSrc == effectiveDirective)
Are you intentionally not handling child-src here? Not obvious to me why.

[Mike West, 2016-11-09 08:37:21]

> Sorry Mike, I didn't quite get to this today and now I can't really process it
through my tears and misery. Left a question inline and will take a closer look
tomorrow.

Yup. Not a great day.

https://codereview.chromium.org/2480303002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right):

https://codereview.chromium.org/2480303002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:436: else if
(ContentSecurityPolicy::WorkerSrc == effectiveDirective)
On 2016/11/09 at 06:16:32, estark wrote:
> Are you intentionally not handling child-src here? Not obvious to me why.

Yes. There's now no case in which `child-src` would be the effective directive,
as `frame-src` is the effective directive for nested contexts, and `worker-src`
is the effective directive for workers.

[foolip, 2016-11-09 16:17:52]

A sad time, hang in there everyone.

tests lgtm % possible nit

https://codereview.chromium.org/2480303002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/ping.js
(right):

https://codereview.chromium.org/2480303002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/ping.js:16:
// Dedicated web workers:
No typeof DedicatedWorkerGlobalScope? Does this rely on none of the others have
a global postMessage? Possibly fragile?

https://codereview.chromium.org/2480303002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/testharness-helper.js
(right):

https://codereview.chromium.org/2480303002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/testharness-helper.js:58:
assert_no_csp_event_for_url(t, url);
Yep, this actually works. It looked to me like a mix of async_test and
promise_test, but test is started by a test.step, so it looks like everything
that works in an async_tests also works before the return of the promise.

[estark, 2016-11-09 16:48:05]

LGTM with a few nits. Also, maybe you could add a test case for when you have
both worker-src and child-src?

https://codereview.chromium.org/2480303002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/testharness-helper.js
(right):

https://codereview.chromium.org/2480303002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/resources/testharness-helper.js:29:
// function builds a test that asserts that the that the ping is
nit: extra "that the"

https://codereview.chromium.org/2480303002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/worker-src/dedicated-none.html
(right):

https://codereview.chromium.org/2480303002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/worker-src/dedicated-none.html:8:
assert_worker_is_blocked(url, "Same-origin dedicated worker allowed by
host-source expression.");
s/allowed/blocked/ (here and line 12)

https://codereview.chromium.org/2480303002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp (right):

https://codereview.chromium.org/2480303002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:436: else if
(ContentSecurityPolicy::WorkerSrc == effectiveDirective)
On 2016/11/09 08:37:21, Mike West wrote:
> On 2016/11/09 at 06:16:32, estark wrote:
> > Are you intentionally not handling child-src here? Not obvious to me why.
> 
> Yes. There's now no case in which `child-src` would be the effective
directive,
> as `frame-src` is the effective directive for nested contexts, and
`worker-src`
> is the effective directive for workers.

Ah, gotcha, thanks for the explanation. In that case, I'll optionally nit that
you could add a DCHECK_NE(ChildSrc, effectiveDirective).

https://codereview.chromium.org/2480303002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp:751: if
(url.protocolIsAbout())
Is this necessary? I see why we need it for allowFrameFromSource() (to allow
newly added frames with about:blank) but don't see why for workers.

[Mike West, 2016-11-10 08:20:56]

The CQ bit was checked by mkwst@chromium.org

[Mike West, 2016-11-10 08:20:57]

The patchset sent to the CQ was uploaded after l-g-t-m from foolip@chromium.org,
estark@chromium.org
Link to the patchset: https://codereview.chromium.org/2480303002/#ps60001
(title: "feedback")

[commit-bot: I haz the power, 2016-11-10 08:21:13]

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-11-10 10:06:51]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-10 10:08:55]

Description was changed from

==========
CSP3: Implement 'worker-src'.

As a drive-by, this also renames `allowChildFrameFromSource` to
`allowFrameFromSource`, and `allowChildContextFromSource` to
`allowWorkerFromSource` to reflect their usage.

Intent to Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/1UkZE-vOROc/gEj7psew...

BUG=662930
==========

to

==========
CSP3: Implement 'worker-src'.

As a drive-by, this also renames `allowChildFrameFromSource` to
`allowFrameFromSource`, and `allowChildContextFromSource` to
`allowWorkerFromSource` to reflect their usage.

Intent to Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/1UkZE-vOROc/gEj7psew...

BUG=662930

Committed: https://crrev.com/586705638164b07863229f70fba19fa2571b48af
Cr-Commit-Position: refs/heads/master@{#431228}
==========

[commit-bot: I haz the power, 2016-11-10 10:08:56]

Patchset 4 (id:??) landed as
https://crrev.com/586705638164b07863229f70fba19fa2571b48af
Cr-Commit-Position: refs/heads/master@{#431228}