[Extensions Bindings] Remove TestConfig.nativeCrxBindingsEnabled

Remove nativeCrxBindingsEnabled from the TestConfig object in the chrome.test extension API. Since native bindings are now always enabled, we don't need this boolean. Update JS files that were using it. Bug: 938998, 792602 Change-Id: Ica1e78b4c7a2e86258977e9c7939181ff3a8cec8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1518147 Reviewed-by: Karan Bhatia <karandeepb@chromium.org> Commit-Queue: Devlin <rdevlin.cronin@chromium.org> Cr-Commit-Position: refs/heads/master@{#640951}
Denger-Network · Mar 14, 2019 · 6b1606c · 6b1606c
1 parent 0a59002
commit 6b1606c
Show file tree

Hide file tree

Showing 6 changed files with 9 additions and 118 deletions.
diff --git a/chrome/browser/extensions/extension_apitest.cc b/chrome/browser/extensions/extension_apitest.cc
@@ -52,7 +52,6 @@ const char kTestDataDirectory[] = "testDataDirectory";
 const char kTestWebSocketPort[] = "testWebSocketPort";
 const char kFtpServerPort[] = "ftpServer.port";
 const char kEmbeddedTestServerPort[] = "testServer.port";
-const char kNativeCrxBindingsEnabled[] = "nativeCrxBindingsEnabled";
 
 }  // namespace
 
@@ -75,8 +74,6 @@ void ExtensionApiTest::SetUpOnMainThread() {
     test_config_->SetInteger(kEmbeddedTestServerPort,
                              embedded_test_server()->port());
   }
-  // TODO(devlin): Remove this.
-  test_config_->SetBoolean(kNativeCrxBindingsEnabled, true);
 
   TestGetConfigFunction::set_test_config_state(test_config_.get());
 }

diff --git a/chrome/test/data/extensions/api_test/activity_log_private/test/test.js b/chrome/test/data/extensions/api_test/activity_log_private/test/test.js
@@ -58,11 +58,7 @@ testCases.push({
     chrome.runtime.sendMessage(FRIEND_EXTENSION_ID,
                                'message_self', function response() { });
   },
-  expected_activity_js: [
-    'runtime.connect',
-    'runtime.sendMessage'
-  ],
-  expected_activity_native: [
+  expected_activity: [
     'runtime.sendMessage'
   ]
 });
@@ -71,11 +67,7 @@ testCases.push({
     chrome.runtime.sendMessage(FRIEND_EXTENSION_ID,
                                'message_other', function response() { });
   },
-  expected_activity_js: [
-    'runtime.connect',
-    'runtime.sendMessage'
-  ],
-  expected_activity_native: [
+  expected_activity: [
     'runtime.sendMessage'
   ]
 });
@@ -182,17 +174,7 @@ testCases.push({
     chrome.runtime.sendMessage(FRIEND_EXTENSION_ID,
                                'api_tab_updated', function response() { });
   },
-  expected_activity_js: [
-    'tabs.onUpdated',
-    'tabs.onUpdated',
-    'tabs.onUpdated',
-    'tabs.connect',
-    'tabs.sendMessage',
-    'tabs.executeScript',
-    'tabs.executeScript',
-    'tabs.remove'
-  ],
-  expected_activity_native: [
+  expected_activity: [
     'tabs.onUpdated',
     'tabs.onUpdated',
     'tabs.onUpdated',
@@ -209,18 +191,7 @@ testCases.push({
                                function response() { });
   },
   is_incognito: true,
-  expected_activity_js: [
-    'windows.create',
-    'tabs.onUpdated',
-    'tabs.onUpdated',
-    'tabs.onUpdated',
-    'tabs.connect',
-    'tabs.sendMessage',
-    'tabs.executeScript',
-    'tabs.executeScript',
-    'tabs.remove'
-  ],
-  expected_activity_native: [
+  expected_activity: [
     'windows.create',
     'tabs.onUpdated',
     'tabs.onUpdated',
@@ -556,16 +527,6 @@ function setupTestCasesAndRun() {
             console.log('Expecting OS specific activity for: ' + info.os);
             enabledTestCase.expected_activity =
                 enabledTestCase[activityListForOS];
-          } else if ('expected_activity_js' in enabledTestCase) {
-            // Some tests have different activity depending on whether native
-            // bindings are being used. This is in the case of the extension
-            // using the sendMessage() API. With JS bindings, this is
-            // implemented using connect(), so both API calls are seen. With
-            // native bindings, we fix this, and only the sendMessage call is
-            // seen.
-            var key = config.nativeCrxBindingsEnabled ?
-                'expected_activity_native' : 'expected_activity_js';
-            enabledTestCase.expected_activity = enabledTestCase[key];
           }
 
           enabledTestCases.push(enabledTestCase);

diff --git a/chrome/test/data/extensions/api_test/bindings/invalidate_context/background.js b/chrome/test/data/extensions/api_test/bindings/invalidate_context/background.js
@@ -6,7 +6,6 @@ var frame;
 var frameRuntime;
 var frameStorage;
 var frameTabs;
-var nativeBindingsEnabled;
 
 function createFrame() {
   frame = document.createElement('iframe');
@@ -54,12 +53,6 @@ function testPort(port, expectEventsValid) {
   chrome.test.assertTrue(result.postMessageThrow);
   chrome.test.assertTrue(result.disconnectThrow);
 
-  // With native bindings, the event object instantiated on a Port is set as a
-  // lazy data property, and thus is safe to access even after the context has
-  // been removed. JS bindings always throw errors when trying to access them
-  // after context invalidation.
-  expectEventsValid &= nativeBindingsEnabled;
-
   if (expectEventsValid) {
     chrome.test.assertFalse(result.getOnMessageThrow);
     chrome.test.assertFalse(result.getOnDisconnectThrow);
@@ -73,7 +66,7 @@ function testPort(port, expectEventsValid) {
   }
 }
 
-const tests = [
+chrome.test.runTests([
   function useFrameStorageAndRuntime() {
     createFrame().then(() => {
       frameRuntime = frame.contentWindow.chrome.runtime;
@@ -150,9 +143,4 @@ const tests = [
       chrome.test.succeed();
     });
   },
-];
-
-chrome.test.getConfig((config) => {
-  nativeBindingsEnabled = config.nativeCrxBindingsEnabled;
-  chrome.test.runTests(tests);
-});
+]);
diff --git a/chrome/test/data/extensions/api_test/messaging/connect/page.js b/chrome/test/data/extensions/api_test/messaging/connect/page.js
@@ -2,30 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-function clobberJSON() {
-  JSON.parse = function() {
-    return "JSON.parse clobbered by content script.";
-  };
-
-  JSON.stringify = function() {
-    return "JSON.stringify clobbered by content script.";
-  };
-
-  Array.prototype.toJSON = function() {
-    return "Array.prototype.toJSON clobbered by content script.";
-  };
-
-  Object.prototype.toJSON = function() {
-    return "Object.prototype.toJSON clobbered by content script.";
-  };
-}
-
-chrome.test.getConfig((config) => {
-  // We don't clobber JSON with native bindings. See https://crbug.com/792602.
-  if (!config.nativeCrxBindingsEnabled)
-    clobberJSON();
-});
-
 // For complex connect tests.
 chrome.runtime.onConnect.addListener(function onConnect(port) {
   console.log('connected');

diff --git a/chrome/test/data/extensions/api_test/messaging/connect/test.js b/chrome/test/data/extensions/api_test/messaging/connect/test.js
@@ -5,24 +5,6 @@
 var listenOnce = chrome.test.listenOnce;
 var listenForever = chrome.test.listenForever;
 
-function clobberJSON() {
-  JSON.parse = function() {
-    return "JSON.parse clobbered by extension.";
-  };
-
-  JSON.stringify = function() {
-    return "JSON.stringify clobbered by extension.";
-  };
-
-  Array.prototype.toJSON = function() {
-    return "Array.prototype.toJSON clobbered by extension.";
-  };
-
-  Object.prototype.toJSON = function() {
-    return "Object.prototype.toJSON clobbered by extension.";
-  };
-}
-
 // Keep track of the tab that we're running tests in, for simplicity.
 var testTab = null;
 
@@ -45,10 +27,6 @@ function compareSenders(expected, actual) {
 }
 
 chrome.test.getConfig(function(config) {
-  // We don't clobber JSON with native bindings. See https://crbug.com/792602.
-  if (!config.nativeCrxBindingsEnabled)
-    clobberJSON();
-
   chrome.test.runTests([
     function setupTestTab() {
       chrome.test.log("Creating tab...");
@@ -279,20 +257,16 @@ chrome.test.getConfig(function(config) {
     // Tests that a message which fails to serialize prints an error and
     // doesn't send (http://crbug.com/263077).
     function unserializableMessage() {
-      // Unserializable messages throw an error with native bindings, and only
-      // log a warning with JS bindings.
-      var expectThrow = config.nativeCrxBindingsEnabled;
       try {
         chrome.tabs.connect(testTab.id).postMessage(function() {
           // This shouldn't ever be called, so it's a bit pointless.
           chrome.test.fail();
         });
-        // Didn't crash.
-        chrome.test.assertFalse(expectThrow);
+        // The call above should have thrown an error.
+        chrome.test.fail();
       } catch (e) {
-        chrome.test.assertTrue(expectThrow);
+        chrome.test.succeed();
       }
-      chrome.test.succeed();
     },
 
     // Tests that reloading a child frame disconnects the port if it was the

diff --git a/extensions/common/api/test.json b/extensions/common/api/test.json
@@ -61,11 +61,6 @@
                     "minimum": 0,
                     "maximum": 65535
                   },
-                  "nativeCrxBindingsEnabled": {
-                    "type": "boolean",
-                    "optional": true,
-                    "description": "Whether native extension bindings are enabled."
-                  },
                   "loginStatus": {
                     "type": "object",
                     "optional": true,