third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-reload.html - Issue 1883873002: Do not block document.written scripts in page reloads

Side by Side Diff: third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-reload.html

Issue 1883873002: Do not block document.written scripts in page reloads (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Feedback incorporated. Created 4 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« no previous file with comments | « third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-conn-type.html ('k') | third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-reload-expected.txt » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <script src="../resources/testharness.js"></script>

	3 <script src="../resources/testharnessreport.js"></script>

	4 <script>

	5 var reloadTestSuccess = false;

	6 </script>

	7
	jkarlin 2016/04/18 17:42:27 This script won't work without window.internals. This script won't work without window.internals. You might as well assert_true(window.internals); up here and not check for it below. shivanisha 2016/04/18 19:33:42 done. Show quoted text On 2016/04/18 at 17:42:27, jkarlin wrote: > This script won't work without window.internals. > > You might as well assert_true(window.internals); up here and not check for it below. done.
	8 <script>

	9 if (window.internals) {

	10 internals.settings.setDisallowFetchForDocWrittenScriptsInMainFrameOnSlow Connections(true);

	11 internals.setNetworkStateNotifierTestOnly(true);

	12 internals.setNetworkConnectionInfo('cellular2g', 1.0);

	13 internals.evictAllResources();

	14

	15 window.addEventListener('beforeunload', function() {

	16 internals.settings.setDisallowFetchForDocWrittenScriptsInMainFrameOn SlowConnections(false);

	17 internals.setNetworkStateNotifierTestOnly(false);

	18 // Remove localStorage items, just in case they haven't been

	19 // already removed, due to test failure.

	20 if (window.localStorage.getItem("errCount") !== null) {

	21 window.localStorage.removeItem("errCount");

	22 }

	23

	24 if (window.localStorage.getItem("succCount") !== null) {

	25 window.localStorage.removeItem("succCount");

	26 }

	27 }, false);

	28 }

	29

	30 if (window.localStorage.getItem("errCount") === null) {

	31 window.localStorage.setItem("errCount", 0);

	32 }

	33

	34 if (window.localStorage.getItem("succCount") === null) {

	35 window.localStorage.setItem("succCount", 0);

	36 }

	37

	38 var crossOrigin = 'http://localhost:8000';

	39 var filePath = '/loading/resources/js-loaded.js?reload';

	40

	41 // First time the script will be blocked and onError will reload the page.

	42 // On reload the script should not be blocked.

	43 // This tests two types of reload, one with and one without cache bypass.

	44 // The script should not be blocked in both cases.

	45 src = crossOrigin + filePath;

	46 document.write('<scr' + 'ipt src="' + src + '" onload="succTest()" onError=" errTest()"></scr' + 'ipt>');

	47

	48 function succTest() {
	jkarlin 2016/04/18 17:42:26 Layout tests have looser style, but can you use fu Layout tests have looser style, but can you use full names here instead of abbreviations to be consistent with the rest of the file? That's for succTest, succCount, errTest, and errCount. shivanisha 2016/04/18 19:33:42 done. Show quoted text On 2016/04/18 at 17:42:26, jkarlin wrote: > Layout tests have looser style, but can you use full names here instead of abbreviations to be consistent with the rest of the file? > > That's for succTest, succCount, errTest, and errCount. done.
	49 console.assert(window.localStorage.getItem("errCount") == 1,"errCount is not one");
	jkarlin 2016/04/18 17:42:26 I've not seen console.assert used in tests much. C I've not seen console.assert used in tests much. Can you use assert_equals(window.localStorage.getItem("errCount"), 1, "errCount is not one"); instead? That's provided by testharness.js. shivanisha 2016/04/18 19:33:43 done. Show quoted text On 2016/04/18 at 17:42:26, jkarlin wrote: > I've not seen console.assert used in tests much. Can you use assert_equals(window.localStorage.getItem("errCount"), 1, "errCount is > not one"); instead? That's provided by testharness.js. done.
	50

	51 var succCount = window.localStorage.getItem("succCount");

	52 console.assert(succCount >= 0,"succCount is not >= zero");
	jkarlin 2016/04/18 17:42:26 assert_greather_than_equal assert_greather_than_equal shivanisha 2016/04/18 19:33:42 done. Show quoted text On 2016/04/18 at 17:42:26, jkarlin wrote: > assert_greather_than_equal done.
	53

	54 if (succCount == 0)

	55 {

	56 window.localStorage.setItem("succCount", 1);

	57 if (window.internals) {

	58 internals.forceReload(true);

	59 }

	60 }

	61

	62 else if (succCount == 1) {
	jkarlin 2016/04/18 17:42:26 It's an error if succCount isn't 1. So either retu It's an error if succCount isn't 1. So either return early if succCount == 0 and remove the condition here (preferred) or make this an else { assert_equals(succCount, 1); ... } jkarlin 2016/04/18 17:43:37 The assert_equals(succCount, 1) should be there in Show quoted text On 2016/04/18 17:42:26, jkarlin wrote: > It's an error if succCount isn't 1. So either return early if succCount == 0 and > remove the condition here (preferred) or make this an else { > assert_equals(succCount, 1); ... } The assert_equals(succCount, 1) should be there in either approach. shivanisha 2016/04/18 19:33:43 Adding the return. It will never be reachable beca Show quoted text On 2016/04/18 at 17:42:26, jkarlin wrote: > It's an error if succCount isn't 1. So either return early if succCount == 0 and remove the condition here (preferred) or make this an else { assert_equals(succCount, 1); ... } Adding the return. It will never be reachable because of the preceeding forceReload but makes it clear that the subsequent code is only reachable if successCount is not 0.
	63 window.localStorage.removeItem("errCount");

	64 window.localStorage.removeItem("succCount");

	65 reloadTestSuccess=true;

	66 }

	67 }

	68

	69 function errTest() {

	70 console.assert(window.localStorage.getItem("errCount") == 0,"errCount is not zero");
	jkarlin 2016/04/18 17:42:26 assert_equals assert_equals shivanisha 2016/04/18 19:33:43 done. Show quoted text On 2016/04/18 at 17:42:26, jkarlin wrote: > assert_equals done.
	71 console.assert(window.localStorage.getItem("succCount") == 0,"succCount is not zero");
	jkarlin 2016/04/18 17:42:26 assert_equals assert_equals shivanisha 2016/04/18 19:33:43 done. Show quoted text On 2016/04/18 at 17:42:26, jkarlin wrote: > assert_equals done.
	72 window.localStorage.setItem("errCount", 1);

	73 if (window.internals) {

	74 internals.forceReload(false);

	75 }

	76 }

	77

	78 </script>

	79

	80 <script>

	81 test(function () {

	82 assert_true(reloadTestSuccess);

	83 }, "cross origin doc.written scripts are not blocked in a page reload");

	84 </script>

OLD	NEW