Add Blink setting to block doc.written scripts on 2g-like networks

Add Blink setting to block doc.written scripts on 2g-like networks

Use the NetworkQualityEstimator to add the ability to block synchronous script
loading with document.write if the network conditions are 2G or 2G-like.

This CL also adds TestRunner API to let layout tests override the effective network type.

BUG=633569
Committed: https://crrev.com/3bddb7d26f782d26958aa3afb0777adb1792033d
Cr-Commit-Position: refs/heads/master@{#420117}

[jkarlin, 2016-09-13 18:26:04]

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

[commit-bot: I haz the power, 2016-09-13 18:26:45]

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-09-13 19:13:51]

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

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

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

[jkarlin, 2016-09-16 15:22:57]

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

[commit-bot: I haz the power, 2016-09-16 15:23:17]

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

[jkarlin, 2016-09-16 17:08:11]

jkarlin@chromium.org changed reviewers:
+ japhet@chromium.org

[jkarlin, 2016-09-16 17:08:12]

japhet@ PTAL at everything, many thanks!

[jkarlin, 2016-09-16 17:20:57]

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

[commit-bot: I haz the power, 2016-09-16 17:21:31]

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-09-16 20:54:04]

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

[commit-bot: I haz the power, 2016-09-16 20:54:05]

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_...)

[Nate Chapin, 2016-09-16 21:56:41]

Production code changes look fine, but the test plumbing probably isn't optimal.

https://codereview.chromium.org/2338623004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/testing/Internals.idl (right):

https://codereview.chromium.org/2338623004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/testing/Internals.idl:307: [RaisesException] void
setEffectiveConnectionTypeOverride(DOMString type);
These APIs want to override logic that doesn't reside in blink, so Internals
isn't the right API. These should go on the testRunner API, which is implemented
out in components/test_runner/. That also removes the need for test-only values
in RenderFrameImpl, as WebFrameTestClient can peak at testRunner settings and
defer to RenderFrameImpl iff a test override isn't set.

[jkarlin, 2016-09-19 14:38:25]

https://codereview.chromium.org/2338623004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/testing/Internals.idl (right):

https://codereview.chromium.org/2338623004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/testing/Internals.idl:307: [RaisesException] void
setEffectiveConnectionTypeOverride(DOMString type);
On 2016/09/16 21:56:41, Nate Chapin wrote:
> These APIs want to override logic that doesn't reside in blink, so Internals
> isn't the right API. These should go on the testRunner API, which is
implemented
> out in components/test_runner/. That also removes the need for test-only
values
> in RenderFrameImpl, as WebFrameTestClient can peak at testRunner settings and
> defer to RenderFrameImpl iff a test override isn't set.

With the testRunner API, I'd have to configure the test via test_interfaces.cc
to have the correct override, is that correct? That's non-obvious. I'd much
rather have something configurable from JavaScript, much like we already have
for configuring the network type that the NetworkStateNotifier returns.

[Nate Chapin, 2016-09-19 19:30:13]

On 2016/09/19 14:38:25, jkarlin wrote:
>
https://codereview.chromium.org/2338623004/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/testing/Internals.idl (right):
> 
>
https://codereview.chromium.org/2338623004/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/testing/Internals.idl:307: [RaisesException]
void
> setEffectiveConnectionTypeOverride(DOMString type);
> On 2016/09/16 21:56:41, Nate Chapin wrote:
> > These APIs want to override logic that doesn't reside in blink, so Internals
> > isn't the right API. These should go on the testRunner API, which is
> implemented
> > out in components/test_runner/. That also removes the need for test-only
> values
> > in RenderFrameImpl, as WebFrameTestClient can peak at testRunner settings
and
> > defer to RenderFrameImpl iff a test override isn't set.
> 
> With the testRunner API, I'd have to configure the test via test_interfaces.cc
> to have the correct override, is that correct? That's non-obvious. I'd much
> rather have something configurable from JavaScript, much like we already have
> for configuring the network type that the NetworkStateNotifier returns.

You shouldn't need any magic in test_interfaces.cc, you can add a flag that is
callable from JS that is accessibly on the testRunner object. See, e.g., the
usage of SetXSSAuditorEnabled in components/test_runner/test_runner.cc.

[jkarlin, 2016-09-21 17:09:14]

Patchset #6 (id:100001) has been deleted

[jkarlin, 2016-09-21 17:09:23]

Patchset #6 (id:120001) has been deleted

[jkarlin, 2016-09-21 17:09:30]

Patchset #6 (id:140001) has been deleted

[jkarlin, 2016-09-21 17:09:38]

Patchset #6 (id:160001) has been deleted

[jkarlin, 2016-09-21 17:11:10]

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

[jkarlin, 2016-09-21 17:11:29]

On 2016/09/19 19:30:13, Nate Chapin wrote:
> On 2016/09/19 14:38:25, jkarlin wrote:
> >
>
https://codereview.chromium.org/2338623004/diff/40001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/testing/Internals.idl (right):
> > 
> >
>
https://codereview.chromium.org/2338623004/diff/40001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/testing/Internals.idl:307: [RaisesException]
> void
> > setEffectiveConnectionTypeOverride(DOMString type);
> > On 2016/09/16 21:56:41, Nate Chapin wrote:
> > > These APIs want to override logic that doesn't reside in blink, so
Internals
> > > isn't the right API. These should go on the testRunner API, which is
> > implemented
> > > out in components/test_runner/. That also removes the need for test-only
> > values
> > > in RenderFrameImpl, as WebFrameTestClient can peak at testRunner settings
> and
> > > defer to RenderFrameImpl iff a test override isn't set.
> > 
> > With the testRunner API, I'd have to configure the test via
test_interfaces.cc
> > to have the correct override, is that correct? That's non-obvious. I'd much
> > rather have something configurable from JavaScript, much like we already
have
> > for configuring the network type that the NetworkStateNotifier returns.
> 
> You shouldn't need any magic in test_interfaces.cc, you can add a flag that is
> callable from JS that is accessibly on the testRunner object. See, e.g., the
> usage of SetXSSAuditorEnabled in components/test_runner/test_runner.cc.

Thanks for the pointers. Done. PTAL!

[commit-bot: I haz the power, 2016-09-21 17:11:48]

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-09-21 17:15:31]

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

[commit-bot: I haz the power, 2016-09-21 17:15:32]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jkarlin, 2016-09-21 17:20:56]

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

[commit-bot: I haz the power, 2016-09-21 17:21:34]

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

[Nate Chapin, 2016-09-21 17:26:24]

LGTM, but you'll need a different OWNER for the components/test_runner pieces.

[jkarlin, 2016-09-21 17:26:28]

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

[commit-bot: I haz the power, 2016-09-21 17:26:51]

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

[jkarlin, 2016-09-21 17:37:00]

jkarlin@chromium.org changed reviewers:
+ dpranke@chromium.org

[jkarlin, 2016-09-21 17:37:00]

Thanks Nate. dpranke@ PTAL at components/, thanks!

[Dirk Pranke, 2016-09-21 17:38:18]

lgtm

[jkarlin, 2016-09-21 17:40:12]

Description was changed from

==========
Add Blink setting to block doc.written scripts on 2g-like networks

Use the NetworkQualityEstimator to add the ability to block synchronous script
loading with document.write if the network conditions are 2G or 2G-like.

This CL also adds Internals API to let layout tests override the effective
network type.

BUG=633569
==========

to

==========
Add Blink setting to block doc.written scripts on 2g-like networks

Use the NetworkQualityEstimator to add the ability to block synchronous script
loading with document.write if the network conditions are 2G or 2G-like.

This CL also adds TestRunner API to let layout tests override the effective
network type.

BUG=633569
==========

[jkarlin, 2016-09-21 18:23:16]

Thanks!

[jkarlin, 2016-09-21 18:23:25]

The CQ bit was unchecked by jkarlin@chromium.org

[jkarlin, 2016-09-21 18:23:39]

The CQ bit was checked by jkarlin@chromium.org

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

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-21 18:44:35]

Description was changed from

==========
Add Blink setting to block doc.written scripts on 2g-like networks

Use the NetworkQualityEstimator to add the ability to block synchronous script
loading with document.write if the network conditions are 2G or 2G-like.

This CL also adds TestRunner API to let layout tests override the effective
network type.

BUG=633569
==========

to

==========
Add Blink setting to block doc.written scripts on 2g-like networks

Use the NetworkQualityEstimator to add the ability to block synchronous script
loading with document.write if the network conditions are 2G or 2G-like.

This CL also adds TestRunner API to let layout tests override the effective
network type.

BUG=633569
==========

[commit-bot: I haz the power, 2016-09-21 18:44:37]

Committed patchset #8 (id:220001)

[commit-bot: I haz the power, 2016-09-21 18:46:30]

Description was changed from

==========
Add Blink setting to block doc.written scripts on 2g-like networks

Use the NetworkQualityEstimator to add the ability to block synchronous script
loading with document.write if the network conditions are 2G or 2G-like.

This CL also adds TestRunner API to let layout tests override the effective
network type.

BUG=633569
==========

to

==========
Add Blink setting to block doc.written scripts on 2g-like networks

Use the NetworkQualityEstimator to add the ability to block synchronous script
loading with document.write if the network conditions are 2G or 2G-like.

This CL also adds TestRunner API to let layout tests override the effective
network type.

BUG=633569

Committed: https://crrev.com/3bddb7d26f782d26958aa3afb0777adb1792033d
Cr-Commit-Position: refs/heads/master@{#420117}
==========

[commit-bot: I haz the power, 2016-09-21 18:46:31]

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