Revert "Deduplicate document policy violation report"

This reverts commit a95e3a91191d90cbbdf322b85537b61d1da6f121. Reason for revert: Likely culprit of https://crbug.com/1087277 Original change's description: > Deduplicate document policy violation report > > Most image policies can report multiple violations for a single cause(same image not following the rule) during layout, e.g. An unoptimized-lossless-image's bpp(byte per pixel) value can change multiple times if the image is being scaled in an animation. > > To avoid unnecessary duplicated reports being generated. This CL adds Hash method for DocumentPolicyViolationReport to uniquely identify each report and avoid the duplication by remembering these hash values in LocalDOMWindow and filter out reports that are in record. > > Some existing web tests rely on the fact that multiple copies of same report generated in same document. This CL also worked around that to either help make reports generated unique or isolate test cases to different documents(iframes). > > Bug: 924684, 926199 > Change-Id: I230e32801c77980573b5ed55064d46f94aed3060 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2078837 > Commit-Queue: Charlie Hu <[email protected]> > Reviewed-by: Daniel Cheng <[email protected]> > Reviewed-by: Jason Chase <[email protected]> > Cr-Commit-Position: refs/heads/master@{#772406} [email protected],[email protected],[email protected] Change-Id: I92e14abb1538a8983fa0c6b1248b355fab024707 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 924684, 926199, 1087277 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2218833 Reviewed-by: Jan Wilken Dörrie <[email protected]> Commit-Queue: Jan Wilken Dörrie <[email protected]> Cr-Commit-Position: refs/heads/master@{#772589}
web-platform-tests · May 28, 2020 · ffd5294 · ffd5294
1 parent 5c34fc6
commit ffd5294
Show file tree

Hide file tree

Showing 6 changed files with 106 additions and 206 deletions.
diff --git a/document-policy/font-display/font-display-document-policy-report-only.tentative.html b/document-policy/font-display/font-display-document-policy-report-only.tentative.html
@@ -1,86 +1,72 @@
 <!DOCTYPE html>
 <html>
-
-<head>
-  <title>Test for font-display-late-swap feature policy set to report-only</title>
-  <link rel="help"
-    href="https://github.com/w3c/webappsec-feature-policy/blob/master/policies/font-display-late-swap.md">
-  <script src='/resources/testharness.js'></script>
-  <script src='/resources/testharnessreport.js'></script>
-  <style>
-  </style>
-</head>
-
-<body>
-  <p>
-    Tests if the correct number of violation reports are generated and each report corresponds to this feature.
-    4 reports should be created out of the 6 options below (reports for all except for 'fallback' and 'optional').
-  </p>
-  <table id="container">
-    <tr>
+  <head>
+    <title>Test for font-display-late-swap feature policy set to report-only</title>
+    <link rel="help" href="https://github.com/w3c/webappsec-feature-policy/blob/master/policies/font-display-late-swap.md">
+    <script src='/resources/testharness.js'></script>
+    <script src='/resources/testharnessreport.js'></script>
+    <style>
+    </style>
+  </head>
+  <body>
+    <p>
+      Tests if the correct number of violation reports are generated and each report corresponds to this feature.
+      4 reports should be created out of the 6 options below (reports for all except for 'fallback' and 'optional').
+    </p>
+    <table id="container">
+     <tr>
       <th>not-set</th>
       <th>auto</th>
       <th>block</th>
       <th>swap</th>
       <th>fallback</th>
       <th>optional</th>
-    </tr>
-  </table>
-  <script>
-    const fontDisplayValues = ['', 'auto', 'block', 'swap', 'fallback', 'optional'];
-    const table = document.getElementById('container');
-    const t = async_test('font-display-late-swap Report Format');
+     </tr>
+    </table>
+    <script>
+const fontDisplayValues = ['', 'auto', 'block', 'swap', 'fallback', 'optional'];
+const table = document.getElementById('container');
 
-    function makeFontFaceDeclaration(family, display) {
-      url = '/fonts/Ahem.ttf?pipe=trickle(d1)'; // Before the swap period is over
-      return `@font-face { font-family: ${family}; src: url("${url}"); font-display: ${display}; }`;
-    }
+function makeFontFaceDeclaration(family, display) {
+    url = '/fonts/Ahem.ttf?pipe=trickle(d1)'; // Before the swap period is over
+    return '@font-face { font-family: ' + family + '; src: url("' + url + '"); font-display: ' + display + '; }';
+}
 
-    window.onload = () => {
-      let tr = document.createElement('tr');
-      for (let display of fontDisplayValues) {
+window.onload = () => {
+    let tr = document.createElement('tr');
+    for (let display of fontDisplayValues) {
         const family = display + '-face';
         const rule = makeFontFaceDeclaration(family, display);
-
-        // Create a separate iframe for testing purpose.
-        // For same document, violation reports with same content might be deduped.
-        let iframe = document.createElement('iframe');
-        iframe.src = 'font-display-document-policy-report-only.tentative.sub.html';
-        iframe.onload = t.step_func(() => {
-          iframe.contentWindow.postMessage({
-            family,
-            rule
-          }, '*');
-        });
-
+        document.styleSheets[0].insertRule(rule, 0);
         let td = document.createElement('td');
-        td.appendChild(iframe);
+        td.textContent = 'a';
+        td.style.fontFamily = family + ', Arial';
         tr.appendChild(td);
-      }
-      table.appendChild(tr);
     }
+    table.appendChild(tr);
+}
 
-    let reportCounter = 4;
-    window.onmessage = t.step_func((e) => {
-      const reports = JSON.parse(e.data);
-      assert_equals(reports.length, 1);
-      check_report_format(reports[0]);
-    });
+let reportCounter = 4;
+let t = async_test('font-display-late-swap Report Format');
 
-    let check_report_format = (report) => {
-      reportCounter--;
-      assert_equals(report.type, 'document-policy-violation');
-      assert_equals(report.url, document.getElementsByTagName('iframe')[0].contentWindow.location.href);
-      assert_equals(report.body.featureId, 'font-display-late-swap');
-      assert_equals(report.body.disposition, 'report');
-      assert_true('sourceFile' in report.body);
-      assert_true('lineNumber' in report.body);
-      assert_true('columnNumber' in report.body);
-      // Test is done when we have exactly 4 reports for the following
-      // font-display values: not set, 'auto', 'block', 'swap'
-      if (reportCounter == 0) t.done();
-    };
-  </script>
-</body>
+let check_report_format = (reports, observer) => {
+  reportCounter -= reports.length;
+  for (let report of reports) {
+    assert_equals(report.type, 'document-policy-violation');
+    assert_equals(report.url, document.location.href, 'Report URL');
+    assert_equals(report.body.featureId, 'font-display-late-swap');
+    assert_equals(report.body.disposition, 'report');
+    assert_true('sourceFile' in report.body);
+    assert_true('lineNumber' in report.body);
+    assert_true('columnNumber' in report.body);
+  }
+  // Test is done when we have exactly 4 reports for the following
+  // font-display values: not set, 'auto', 'block', 'swap'
+  if (reportCounter == 0) t.done();
+};
 
+new ReportingObserver(t.step_func(check_report_format),
+                      {types: ['document-policy-violation'], buffered: true}).observe();
+    </script>
+  </body>
 </html>
diff --git a/...cy-report-only.tentative.sub.html.headers → ...policy-report-only.tentative.html.headers b/...cy-report-only.tentative.sub.html.headers → ...policy-report-only.tentative.html.headers
diff --git a/document-policy/font-display/font-display-document-policy-report-only.tentative.sub.html b/document-policy/font-display/font-display-document-policy-report-only.tentative.sub.html
diff --git a/document-policy/font-display/font-display-document-policy-reporting.tentative.html b/document-policy/font-display/font-display-document-policy-reporting.tentative.html
@@ -1,86 +1,72 @@
 <!DOCTYPE html>
 <html>
-
-<head>
-  <title>Test for font-display-late-swap feature policy set to reporting</title>
-  <link rel="help"
-    href="https://github.com/w3c/webappsec-feature-policy/blob/master/policies/font-display-late-swap.md">
-  <script src='/resources/testharness.js'></script>
-  <script src='/resources/testharnessreport.js'></script>
-  <style>
-  </style>
-</head>
-
-<body>
-  <p>
-    Tests if the correct number of violation reports are generated and each report corresponds to this feature.
-    4 reports should be created out of the 6 options below (reports for all except for 'fallback' and 'optional').
-  </p>
-  <table id="container">
-    <tr>
+  <head>
+    <title>Test for font-display-late-swap feature policy set to reporting</title>
+    <link rel="help" href="https://github.com/w3c/webappsec-feature-policy/blob/master/policies/font-display-late-swap.md">
+    <script src='/resources/testharness.js'></script>
+    <script src='/resources/testharnessreport.js'></script>
+    <style>
+    </style>
+  </head>
+  <body>
+    <p>
+      Tests if the correct number of violation reports are generated and each report corresponds to this feature.
+      4 reports should be created out of the 6 options below (reports for all except for 'fallback' and 'optional').
+    </p>
+    <table id="container">
+     <tr>
       <th>not-set</th>
       <th>auto</th>
       <th>block</th>
       <th>swap</th>
       <th>fallback</th>
       <th>optional</th>
-    </tr>
-  </table>
-  <script>
-    const fontDisplayValues = ['', 'auto', 'block', 'swap', 'fallback', 'optional'];
-    const table = document.getElementById('container');
-    const t = async_test('font-display-late-swap Report Format');
+     </tr>
+    </table>
+    <script>
+const fontDisplayValues = ['', 'auto', 'block', 'swap', 'fallback', 'optional'];
+const table = document.getElementById('container');
 
-    function makeFontFaceDeclaration(family, display) {
-      url = '/fonts/Ahem.ttf?pipe=trickle(d1)'; // Before the swap period is over
-      return `@font-face { font-family: ${family}; src: url("${url}"); font-display: ${display}; }`;
-    }
+function makeFontFaceDeclaration(family, display) {
+    url = '/fonts/Ahem.ttf?pipe=trickle(d1)'; // Before the swap period is over
+    return '@font-face { font-family: ' + family + '; src: url("' + url + '"); font-display: ' + display + '; }';
+}
 
-    window.onload = () => {
-      let tr = document.createElement('tr');
-      for (let display of fontDisplayValues) {
+window.onload = () => {
+    let tr = document.createElement('tr');
+    for (let display of fontDisplayValues) {
         const family = display + '-face';
         const rule = makeFontFaceDeclaration(family, display);
-
-        // Create a separate iframe for testing purpose.
-        // For same document, violation reports with same content might be deduped.
-        let iframe = document.createElement('iframe');
-        iframe.src = 'font-display-document-policy-reporting.tentative.sub.html';
-        iframe.onload = t.step_func(() => {
-          iframe.contentWindow.postMessage({
-            family,
-            rule
-          }, '*');
-        });
-
+        document.styleSheets[0].insertRule(rule, 0);
         let td = document.createElement('td');
-        td.appendChild(iframe);
+        td.textContent = 'a';
+        td.style.fontFamily = family + ', Arial';
         tr.appendChild(td);
-      }
-      table.appendChild(tr);
     }
+    table.appendChild(tr);
+}
 
-    let reportCounter = 4;
-    window.onmessage = t.step_func((e) => {
-      const reports = JSON.parse(e.data);
-      assert_equals(reports.length, 1);
-      check_report_format(reports[0]);
-    });
+let reportCounter = 4;
+let t = async_test('font-display-late-swap Report Format');
 
-    let check_report_format = (report) => {
-      reportCounter--;
-      assert_equals(report.type, 'document-policy-violation');
-      assert_equals(report.url, document.getElementsByTagName('iframe')[0].contentWindow.location.href);
-      assert_equals(report.body.featureId, 'font-display-late-swap');
-      assert_equals(report.body.disposition, 'enforce');
-      assert_true('sourceFile' in report.body);
-      assert_true('lineNumber' in report.body);
-      assert_true('columnNumber' in report.body);
-      // Test is done when we have exactly 4 reports for the following
-      // font-display values: not set, 'auto', 'block', 'swap'
-      if (reportCounter == 0) t.done();
-    };
-  </script>
-</body>
+let check_report_format = (reports, observer) => {
+  reportCounter -= reports.length;
+  for (let report of reports) {
+    assert_equals(report.type, 'document-policy-violation');
+    assert_equals(report.url, document.location.href, 'Report URL');
+    assert_equals(report.body.featureId, 'font-display-late-swap');
+    assert_equals(report.body.disposition, 'enforce');
+    assert_true('sourceFile' in report.body);
+    assert_true('lineNumber' in report.body);
+    assert_true('columnNumber' in report.body);
+  }
+  // Test is done when we have exactly 4 reports for the following
+  // font-display values: not set, 'auto', 'block', 'swap'
+  if (reportCounter == 0) t.done();
+};
 
+new ReportingObserver(t.step_func(check_report_format),
+                      {types: ['document-policy-violation'], buffered: true}).observe();
+    </script>
+  </body>
 </html>
diff --git a/...licy-reporting.tentative.sub.html.headers → ...t-policy-reporting.tentative.html.headers b/...licy-reporting.tentative.sub.html.headers → ...t-policy-reporting.tentative.html.headers
diff --git a/document-policy/font-display/font-display-document-policy-reporting.tentative.sub.html b/document-policy/font-display/font-display-document-policy-reporting.tentative.sub.html