Commit eedcfc98 authored by Mustaq Ahmed's avatar Mustaq Ahmed Committed by Commit Bot

Removed activation consumption from MessagingApiTest.

As part of our User Activation v2 work (to replace old
UserGestureIndicator-based code), we are moving all
activation consumption calls to the browser side to
avoid possible double consumptions with OOPIFs.  This
CL replaces one consumption call with a fixed inactivate
state.

Bug: 780556
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Idef304c1d58ccab3d981084c48e61c3d1b6a1ef9
Reviewed-on: https://chromium-review.googlesource.com/1045272
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarNoel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557232}
parent 03814999
...@@ -625,9 +625,10 @@ ExtensionHost* ExtensionBrowserTest::FindHostWithPath(ProcessManager* manager, ...@@ -625,9 +625,10 @@ ExtensionHost* ExtensionBrowserTest::FindHostWithPath(ProcessManager* manager,
std::string ExtensionBrowserTest::ExecuteScriptInBackgroundPage( std::string ExtensionBrowserTest::ExecuteScriptInBackgroundPage(
const std::string& extension_id, const std::string& extension_id,
const std::string& script) { const std::string& script,
return browsertest_util::ExecuteScriptInBackgroundPage(profile(), browsertest_util::ScriptUserActivation script_user_activation) {
extension_id, script); return browsertest_util::ExecuteScriptInBackgroundPage(
profile(), extension_id, script, script_user_activation);
} }
bool ExtensionBrowserTest::ExecuteScriptInBackgroundPageNoWait( bool ExtensionBrowserTest::ExecuteScriptInBackgroundPageNoWait(
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/in_process_browser_test.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "extensions/browser/browsertest_util.h"
#include "extensions/browser/extension_host.h" #include "extensions/browser/extension_host.h"
#include "extensions/browser/extension_protocols.h" #include "extensions/browser/extension_protocols.h"
#include "extensions/browser/extension_system.h" #include "extensions/browser/extension_system.h"
...@@ -309,8 +310,12 @@ class ExtensionBrowserTest : virtual public InProcessBrowserTest { ...@@ -309,8 +310,12 @@ class ExtensionBrowserTest : virtual public InProcessBrowserTest {
// Returns // Returns
// browsertest_util::ExecuteScriptInBackgroundPage(profile(), // browsertest_util::ExecuteScriptInBackgroundPage(profile(),
// extension_id, script). // extension_id, script).
std::string ExecuteScriptInBackgroundPage(const std::string& extension_id, std::string ExecuteScriptInBackgroundPage(
const std::string& script); const std::string& extension_id,
const std::string& script,
extensions::browsertest_util::ScriptUserActivation
script_user_activation =
extensions::browsertest_util::ScriptUserActivation::kActivate);
// Returns // Returns
// browsertest_util::ExecuteScriptInBackgroundPageNoWait( // browsertest_util::ExecuteScriptInBackgroundPageNoWait(
......
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/extensions/api/messaging/incognito_connectability.h" #include "chrome/browser/extensions/api/messaging/incognito_connectability.h"
#include "chrome/browser/extensions/browsertest_util.h"
#include "chrome/browser/extensions/extension_apitest.h" #include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/extensions/extension_util.h" #include "chrome/browser/extensions/extension_util.h"
#include "chrome/browser/infobars/infobar_service.h" #include "chrome/browser/infobars/infobar_service.h"
...@@ -1241,14 +1242,21 @@ IN_PROC_BROWSER_TEST_P(MessagingApiTest, MessagingUserGesture) { ...@@ -1241,14 +1242,21 @@ IN_PROC_BROWSER_TEST_P(MessagingApiTest, MessagingUserGesture) {
const Extension* sender = LoadExtension(sender_dir.UnpackedPath()); const Extension* sender = LoadExtension(sender_dir.UnpackedPath());
ASSERT_TRUE(sender); ASSERT_TRUE(sender);
EXPECT_EQ("false", EXPECT_EQ(
ExecuteScriptInBackgroundPage(sender->id(), "false",
base::StringPrintf( ExecuteScriptInBackgroundPage(
"chrome.test.runWithoutUserGesture(function() {\n" sender->id(),
" chrome.runtime.sendMessage('%s', {}, function(response) {\n" base::StringPrintf(
" window.domAutomationController.send('' + response.result);\n" "if (chrome.test.isProcessingUserGesture()) {\n"
" });\n" " domAutomationController.send("
"});", receiver->id().c_str()))); " 'Error: unexpected user gesture');\n"
"} else {\n"
" chrome.runtime.sendMessage('%s', {}, function(response) {\n"
" domAutomationController.send('' + response.result);\n"
" });\n"
"}",
receiver->id().c_str()),
extensions::browsertest_util::ScriptUserActivation::kDontActivate));
EXPECT_EQ("true", EXPECT_EQ("true",
ExecuteScriptInBackgroundPage(sender->id(), ExecuteScriptInBackgroundPage(sender->id(),
......
...@@ -12,9 +12,11 @@ ...@@ -12,9 +12,11 @@
namespace extensions { namespace extensions {
namespace browsertest_util { namespace browsertest_util {
std::string ExecuteScriptInBackgroundPage(content::BrowserContext* context, std::string ExecuteScriptInBackgroundPage(
const std::string& extension_id, content::BrowserContext* context,
const std::string& script) { const std::string& extension_id,
const std::string& script,
ScriptUserActivation script_user_activation) {
ExtensionHost* host = ExtensionHost* host =
ProcessManager::Get(context)->GetBackgroundHostForExtension(extension_id); ProcessManager::Get(context)->GetBackgroundHostForExtension(extension_id);
if (!host) { if (!host) {
...@@ -23,8 +25,17 @@ std::string ExecuteScriptInBackgroundPage(content::BrowserContext* context, ...@@ -23,8 +25,17 @@ std::string ExecuteScriptInBackgroundPage(content::BrowserContext* context,
} }
std::string result; std::string result;
if (!content::ExecuteScriptAndExtractString(host->host_contents(), script, bool success;
&result)) { if (script_user_activation == ScriptUserActivation::kActivate) {
success = content::ExecuteScriptAndExtractString(host->host_contents(),
script, &result);
} else {
DCHECK_EQ(script_user_activation, ScriptUserActivation::kDontActivate);
success = content::ExecuteScriptWithoutUserGestureAndExtractString(
host->host_contents(), script, &result);
}
if (!success) {
ADD_FAILURE() << "Executing script failed: " << script; ADD_FAILURE() << "Executing script failed: " << script;
result.clear(); result.clear();
} }
......
...@@ -14,13 +14,25 @@ class BrowserContext; ...@@ -14,13 +14,25 @@ class BrowserContext;
namespace extensions { namespace extensions {
namespace browsertest_util { namespace browsertest_util {
// Determine if a user activation notification should be triggered before
// executing a script
enum class ScriptUserActivation {
kActivate,
kDontActivate,
};
// Waits until |script| calls "window.domAutomationController.send(result)", // Waits until |script| calls "window.domAutomationController.send(result)",
// where |result| is a string, and returns |result|. Fails the test and returns // where |result| is a string, and returns |result|. Fails the test and returns
// an empty string if |extension_id| isn't installed in |context| or doesn't // an empty string if |extension_id| isn't installed in |context| or doesn't
// have a background page, or if executing the script fails. // have a background page, or if executing the script fails. The argument
std::string ExecuteScriptInBackgroundPage(content::BrowserContext* context, // |script_user_activation| determines if the script should be executed after a
const std::string& extension_id, // user activation.
const std::string& script); std::string ExecuteScriptInBackgroundPage(
content::BrowserContext* context,
const std::string& extension_id,
const std::string& script,
ScriptUserActivation script_user_activation =
ScriptUserActivation::kActivate);
// Same as ExecuteScriptInBackgroundPage, but doesn't wait for the script // Same as ExecuteScriptInBackgroundPage, but doesn't wait for the script
// to return a result. Fails the test and returns false if |extension_id| // to return a result. Fails the test and returns false if |extension_id|
......
...@@ -352,18 +352,6 @@ ...@@ -352,18 +352,6 @@
} }
] ]
}, },
{
"name": "runWithoutUserGesture",
"type": "function",
"nocompile": true,
"parameters": [
{
"type": "function",
"name": "callback",
"parameters": []
}
]
},
{ {
"name": "waitForRoundTrip", "name": "waitForRoundTrip",
"type": "function", "type": "function",
......
...@@ -375,11 +375,6 @@ binding.registerCustomHook(function(api) { ...@@ -375,11 +375,6 @@ binding.registerCustomHook(function(api) {
return userGestures.RunWithUserGesture(callback); return userGestures.RunWithUserGesture(callback);
}); });
apiFunctions.setHandleRequest('runWithoutUserGesture', function(callback) {
chromeTest.assertEq(typeof(callback), 'function');
return userGestures.RunWithoutUserGesture(callback);
});
apiFunctions.setHandleRequest('setExceptionHandler', function(callback) { apiFunctions.setHandleRequest('setExceptionHandler', function(callback) {
chromeTest.assertEq(typeof(callback), 'function'); chromeTest.assertEq(typeof(callback), 'function');
setExceptionHandler(callback); setExceptionHandler(callback);
......
...@@ -23,10 +23,6 @@ void UserGesturesNativeHandler::AddRoutes() { ...@@ -23,10 +23,6 @@ void UserGesturesNativeHandler::AddRoutes() {
"RunWithUserGesture", "test", "RunWithUserGesture", "test",
base::Bind(&UserGesturesNativeHandler::RunWithUserGesture, base::Bind(&UserGesturesNativeHandler::RunWithUserGesture,
base::Unretained(this))); base::Unretained(this)));
RouteHandlerFunction(
"RunWithoutUserGesture", "test",
base::Bind(&UserGesturesNativeHandler::RunWithoutUserGesture,
base::Unretained(this)));
} }
void UserGesturesNativeHandler::IsProcessingUserGesture( void UserGesturesNativeHandler::IsProcessingUserGesture(
...@@ -46,14 +42,4 @@ void UserGesturesNativeHandler::RunWithUserGesture( ...@@ -46,14 +42,4 @@ void UserGesturesNativeHandler::RunWithUserGesture(
nullptr); nullptr);
} }
void UserGesturesNativeHandler::RunWithoutUserGesture(
const v8::FunctionCallbackInfo<v8::Value>& args) {
blink::WebUserGestureIndicator::ConsumeUserGesture(context()->web_frame());
CHECK_EQ(args.Length(), 1);
CHECK(args[0]->IsFunction());
v8::Local<v8::Value> no_args;
context()->SafeCallFunction(v8::Local<v8::Function>::Cast(args[0]), 0,
nullptr);
}
} // namespace extensions } // namespace extensions
...@@ -19,7 +19,6 @@ class UserGesturesNativeHandler : public ObjectBackedNativeHandler { ...@@ -19,7 +19,6 @@ class UserGesturesNativeHandler : public ObjectBackedNativeHandler {
private: private:
void IsProcessingUserGesture(const v8::FunctionCallbackInfo<v8::Value>& args); void IsProcessingUserGesture(const v8::FunctionCallbackInfo<v8::Value>& args);
void RunWithUserGesture(const v8::FunctionCallbackInfo<v8::Value>& args); void RunWithUserGesture(const v8::FunctionCallbackInfo<v8::Value>& args);
void RunWithoutUserGesture(const v8::FunctionCallbackInfo<v8::Value>& args);
}; };
} // namespace extensions } // namespace extensions
......
...@@ -169,11 +169,6 @@ chrome.test.isProcessingUserGesture = function() {}; ...@@ -169,11 +169,6 @@ chrome.test.isProcessingUserGesture = function() {};
*/ */
chrome.test.runWithUserGesture = function(callback) {}; chrome.test.runWithUserGesture = function(callback) {};
/**
* @param {Function} callback
*/
chrome.test.runWithoutUserGesture = function(callback) {};
/** /**
* Sends a string message one round trip from the renderer to the browser * Sends a string message one round trip from the renderer to the browser
* process and back. * process and back.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment