From 6b1606cdc4dbf926c205bb6664c9e326c91437ef Mon Sep 17 00:00:00 2001 From: Devlin Cronin Date: Thu, 14 Mar 2019 22:15:18 +0000 Subject: [PATCH] [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 Commit-Queue: Devlin Cr-Commit-Position: refs/heads/master@{#640951} --- .../browser/extensions/extension_apitest.cc | 3 -- .../activity_log_private/test/test.js | 47 ++----------------- .../bindings/invalidate_context/background.js | 16 +------ .../api_test/messaging/connect/page.js | 24 ---------- .../api_test/messaging/connect/test.js | 32 ++----------- extensions/common/api/test.json | 5 -- 6 files changed, 9 insertions(+), 118 deletions(-) diff --git a/chrome/browser/extensions/extension_apitest.cc b/chrome/browser/extensions/extension_apitest.cc index b3c74da4aed8e4..65051af5252283 100644 --- 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 index aaa4300851d8c3..6774e9bd3a2319 100644 --- 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 index 5ea3ef46b4b904..9280fae6c5561a 100644 --- 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 index f349a3553b7053..02c7675d4cbe38 100644 --- 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 index 48ab64254e0e84..0310b73ac41d97 100644 --- 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 index 7e046cf3be96c0..5772972db3df96 100644 --- 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,