[DNR] Unflake DeclarativeNetRequestAPItest.OnRulesMatchedDebug

For one of the subtests, an xhr request which matches a rule was sent
before the event listener for OnRuleMatchedDebug was registered. The fix
involves waiting for a round trip between the extension and browser to
complete after the listener has been added for the request to be sent,
which prevents the race condition causing the flake.

Bug: 1029233
Change-Id: I1e1c011159dfa9c463612b4f837ce9ce4316a234
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2209601Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Commit-Queue: Kelvin Jiang <kelvinjiang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771998}

chrome/browser/extensions/api/declarative_net_request/declarative_net_request_apitest.cc

@@ -52,10 +52,7 @@ IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestAPItest, DynamicRules) {
   ASSERT_TRUE(RunExtensionTest("dynamic_rules")) << message_;
 }
 
-// TODO(crbug.com/1029233) Restore this test. This is disabled due to
-// flakiness.
-IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestAPItest,
-                       DISABLED_OnRulesMatchedDebug) {
+IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestAPItest, OnRulesMatchedDebug) {
   ASSERT_TRUE(RunExtensionTest("on_rules_matched_debug")) << message_;
 }
 

chrome/test/data/extensions/api_test/declarative_net_request/on_rules_matched_debug/background.js

@@ -26,15 +26,8 @@ function getServerURL(host) {
   return `http://${host}:${testServerPort}/`;
 }
 
-function addRuleMatchedListener() {
-  chrome.declarativeNetRequest.onRuleMatchedDebug.addListener(
-      onRuleMatchedDebugCallback);
-}
-
-function removeRuleMatchedListener() {
+function resetMatchedRules() {
   matchedRules = [];
-  chrome.declarativeNetRequest.onRuleMatchedDebug.removeListener(
-      onRuleMatchedDebugCallback);
 }
 
 function verifyExpectedRuleInfo(expectedRuleInfo) {
@@ -49,7 +42,21 @@ function verifyExpectedRuleInfo(expectedRuleInfo) {
 }
 
 var tests = [
+  function setup() {
+    chrome.declarativeNetRequest.onRuleMatchedDebug.addListener(
+        onRuleMatchedDebugCallback);
+
+    // This test was known to flake as reported in crbug.com/1029233 due to a
+    // race condition where a request was sent and a declarative rule was
+    // matched before the onRuleMatchedDebug listener was properly added.
+    // Sending the request after waiting for a round trip should fix the
+    // flakiness.
+    chrome.test.waitForRoundTrip('msg', chrome.test.succeed);
+  },
+
   function testDynamicRule() {
+    resetMatchedRules();
+
     const ruleIdsToRemove = [];
     const rule = {
       id: 1,
@@ -57,7 +64,7 @@ var tests = [
       condition: {urlFilter: 'def', 'resourceTypes': ['main_frame']},
       action: {type: 'block'},
     };
-    addRuleMatchedListener();
+
     chrome.declarativeNetRequest.updateDynamicRules(
         ruleIdsToRemove, [rule], function() {
           chrome.test.assertNoLastError();
@@ -79,16 +86,14 @@ var tests = [
               }
             };
             verifyExpectedRuleInfo(expectedRuleInfo);
-            removeRuleMatchedListener();
             chrome.test.succeed();
           });
         });
   },
+
   function testBlockRule() {
-    addRuleMatchedListener();
-    // TODO(crbug.com/1029233): Can adding the listener race with the network
-    // request such that when the browser receives the network request, the
-    // listener is not added?
+    resetMatchedRules();
+
     const url = getServerURL('abc.com');
     navigateTab(url, url, (tab) => {
       const expectedRuleInfo = {
@@ -104,7 +109,6 @@ var tests = [
         rule: {ruleId: 1, rulesetId: 'rules1'}
       };
       verifyExpectedRuleInfo(expectedRuleInfo);
-      removeRuleMatchedListener();
       chrome.test.succeed();
     });
   },
@@ -112,14 +116,13 @@ var tests = [
   // Ensure that requests that don't originate from a tab (such as those from
   // the extension background page) trigger the listener.
   function testBackgroundPageRequest() {
-    addRuleMatchedListener();
+    resetMatchedRules();
 
     const url = getServerURL('abc.com');
     let xhr = new XMLHttpRequest();
     xhr.open('GET', url);
 
     xhr.onload = () => {
-      removeRuleMatchedListener();
       chrome.test.fail('Request should be blocked by rule with ID 1');
     };
 
@@ -132,8 +135,6 @@ var tests = [
 
       // Tab ID should be -1 since this request was made from a background page.
       chrome.test.assertEq(-1, matchedRule.request.tabId);
-
-      removeRuleMatchedListener();
       chrome.test.succeed();
     };
 
@@ -141,17 +142,16 @@ var tests = [
   },
 
   function testNoRuleMatched() {
-    addRuleMatchedListener();
+    resetMatchedRules();
     const url = getServerURL('nomatch.com');
     navigateTab(url, url, (tab) => {
       chrome.test.assertEq(0, matchedRules.length);
-      removeRuleMatchedListener();
       chrome.test.succeed();
     });
   },
 
   function testAllowRule() {
-    addRuleMatchedListener();
+    resetMatchedRules();
 
     const url = getServerURL('abcde.com');
     navigateTab(url, url, (tab) => {
@@ -162,13 +162,12 @@ var tests = [
       chrome.test.assertEq(4, matchedRule.rule.ruleId);
       chrome.test.assertEq('rules2', matchedRule.rule.rulesetId);
 
-      removeRuleMatchedListener();
       chrome.test.succeed();
     });
   },
 
   function testMultipleRules() {
-    addRuleMatchedListener();
+    resetMatchedRules();
 
     // redir1.com --> redir2.com --> abc.com (blocked)
     // 3 rules are matched from the above sequence of actions.
@@ -186,7 +185,6 @@ var tests = [
             expectedMatches[i].rulesetId, matchedRules[i].rule.rulesetId);
       }
 
-      removeRuleMatchedListener();
       chrome.test.succeed();
     });
   }