0

Kill EvalJsWithManualReply

It already exists as an EvalJsOption.

Bug: 1157718
Change-Id: Idb1cbc25eca44eb09dbf5845906005974bad47d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2815622
Owners-Override: Avi Drissman <avi@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#871017}
This commit is contained in:
Avi Drissman
2021-04-09 17:23:30 +00:00
committed by Chromium LUCI CQ
parent 022fc996b0
commit 3e40e53511
13 changed files with 79 additions and 89 deletions

@@ -310,7 +310,7 @@ class CookieSettingsTest
// Read a cookie with JavaScript cookie-store API
std::string JSAsyncReadCookie(Browser* browser) {
return content::EvalJsWithManualReply(
return content::EvalJs(
browser->tab_strip_model()->GetActiveWebContents(),
"async function doGet() {"
" const cookies = await window.cookieStore.getAll();"
@@ -319,7 +319,8 @@ class CookieSettingsTest
" cookie_str += `${cookie.name}=${cookie.value};`;"
" window.domAutomationController.send(cookie_str);"
"}"
"doGet()")
"doGet()",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY)
.ExtractString();
}
@@ -343,17 +344,18 @@ class CookieSettingsTest
// Set a cookie with JavaScript cookie-store api.
void JSAsyncWriteCookie(Browser* browser) {
content::EvalJsResult result = content::EvalJsWithManualReply(
browser->tab_strip_model()->GetActiveWebContents(),
"async function doSet() {"
" await window.cookieStore.set("
" { name: 'name',"
" value: 'Good',"
" expires: Date.now() + 3600*1000,"
" sameSite: 'none' });"
" window.domAutomationController.send(true);"
"}"
"doSet()");
content::EvalJsResult result =
content::EvalJs(browser->tab_strip_model()->GetActiveWebContents(),
"async function doSet() {"
" await window.cookieStore.set("
" { name: 'name',"
" value: 'Good',"
" expires: Date.now() + 3600*1000,"
" sameSite: 'none' });"
" window.domAutomationController.send(true);"
"}"
"doSet()",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
// Failure ignored here since some tests purposefully try to set disallowed
// cookies.
}

@@ -368,10 +368,9 @@ IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest,
EXPECT_TRUE(content::WaitForLoadStop(contents));
// Execute in isolated world; where all distiller scripts are run.
EXPECT_EQ(true, content::EvalJsWithManualReply(
contents, kTestDistillerObject,
content::EXECUTE_SCRIPT_DEFAULT_OPTIONS,
ISOLATED_WORLD_ID_CHROME_INTERNAL));
EXPECT_EQ(true, content::EvalJs(contents, kTestDistillerObject,
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY,
ISOLATED_WORLD_ID_CHROME_INTERNAL));
}
IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest,

@@ -553,7 +553,8 @@ IN_PROC_BROWSER_TEST_F(TabStatsTrackerBrowserTest, AddObserverAudibleTab) {
// Start the audio.
base::RunLoop run_loop;
AudioStartObserver audio_start_observer(web_contents, run_loop.QuitClosure());
EXPECT_EQ("OK", EvalJsWithManualReply(web_contents, "StartOscillator();"));
EXPECT_EQ("OK", content::EvalJs(web_contents, "StartOscillator();",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY));
run_loop.Run();
// Adding an observer now should receive the OnTabIsAudibleChanged() call.

@@ -1134,7 +1134,8 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
"video.onplaying = () => { "
"window.domAutomationController.send('true'); };"
"video.play();";
EXPECT_EQ("true", content::EvalJsWithManualReply(ad_frame, play_script));
EXPECT_EQ("true", content::EvalJs(ad_frame, play_script,
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY));
ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));

@@ -3121,11 +3121,11 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, PortalActivation) {
EXPECT_TRUE(
ExecJs(outer_contents, "document.querySelector('portal').activate()"));
EXPECT_EQ(true,
EvalJsWithManualReply(portal_contents,
EXPECT_EQ(true, content::EvalJs(portal_contents,
"activatePromise.then(r => { "
" window.domAutomationController.send(r);"
"});"));
"});",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY));
// The activated portal contents should be the currently active contents.
EXPECT_EQ(portal_contents,

@@ -72,17 +72,18 @@ class PaymentHandlerChangeShippingAddressOptionTest
IN_PROC_BROWSER_TEST_P(PaymentHandlerChangeShippingAddressOptionTest, Test) {
EXPECT_EQ("instruments.set(): Payment handler installed.",
content::EvalJsWithManualReply(
content::EvalJs(
GetActiveWebContents(),
"install('change_shipping_" + getTestType() + "_app.js');"));
"install('change_shipping_" + getTestType() + "_app.js');",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY));
EXPECT_TRUE(
content::ExecJs(GetActiveWebContents(), GetParam().init_test_code));
std::string actual_output =
content::EvalJsWithManualReply(
GetActiveWebContents(),
"outputChangeShippingAddressOptionReturnValue(request);")
content::EvalJs(GetActiveWebContents(),
"outputChangeShippingAddressOptionReturnValue(request);",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY)
.ExtractString();
// The test expectations are hard-coded, but the embedded test server changes

@@ -883,15 +883,16 @@ IN_PROC_BROWSER_TEST_F(
permissions::PermissionRequestManager::AutoResponseType::ACCEPT_ONCE);
// Request 'geolocation' permission.
std::string result = content::EvalJsWithManualReply(GetActiveMainFrame(),
kQueryCurrentPosition)
.ExtractString();
std::string result =
content::EvalJs(GetActiveMainFrame(), kQueryCurrentPosition,
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY)
.ExtractString();
EXPECT_EQ("success", result);
EXPECT_EQ(1, bubble_factory()->TotalRequestCount());
// Request 'geolocation' permission. There should not be a 2nd prompt.
result = content::EvalJsWithManualReply(GetActiveMainFrame(),
kQueryCurrentPosition)
result = content::EvalJs(GetActiveMainFrame(), kQueryCurrentPosition,
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY)
.ExtractString();
EXPECT_EQ("success", result);
EXPECT_EQ(1, bubble_factory()->TotalRequestCount());
@@ -909,8 +910,8 @@ IN_PROC_BROWSER_TEST_F(
GetPermissionRequestManager()));
// Request 'geolocation' permission.
result = content::EvalJsWithManualReply(GetActiveMainFrame(),
kQueryCurrentPosition)
result = content::EvalJs(GetActiveMainFrame(), kQueryCurrentPosition,
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY)
.ExtractString();
EXPECT_EQ("success", result);
// There should be no permission prompt.
@@ -946,8 +947,8 @@ IN_PROC_BROWSER_TEST_F(
permissions::PermissionRequestManager::AutoResponseType::ACCEPT_ONCE);
// Request 'geolocation' permission. We should get a prompt.
result = content::EvalJsWithManualReply(GetActiveMainFrame(),
kQueryCurrentPosition)
result = content::EvalJs(GetActiveMainFrame(), kQueryCurrentPosition,
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY)
.ExtractString();
EXPECT_EQ("success", result);

@@ -164,10 +164,11 @@ IN_PROC_BROWSER_TEST_F(CreateShortcutBrowserTest, WorksAfterDelayedIFrameLoad) {
iframe.srcdoc = 'inner page';
document.body.appendChild(iframe);
)";
EXPECT_EQ(content::EvalJsWithManualReply(
browser()->tab_strip_model()->GetActiveWebContents(), script)
.ExtractString(),
"success");
EXPECT_EQ(
content::EvalJs(browser()->tab_strip_model()->GetActiveWebContents(),
script, content::EXECUTE_SCRIPT_USE_MANUAL_REPLY)
.ExtractString(),
"success");
InstallShortcutAppForCurrentUrl();
}

@@ -242,10 +242,8 @@ IN_PROC_BROWSER_TEST_F(SourceUrlRecorderWebContentsObserverBrowserTest,
window.domAutomationController.send(true);
}, 10);
)";
// EvalJsWithManualReply returns an EvalJsResult, whose docs say to use
// EXPECT_EQ(true, ...) rather than EXPECT_TRUE(), as the latter does not
// compile.
EXPECT_EQ(true, EvalJsWithManualReply(portal_contents, activated_poll));
EXPECT_EQ(true, EvalJs(portal_contents, activated_poll,
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY));
// The activated portal contents should be the currently active contents.
EXPECT_EQ(portal_contents, shell()->web_contents());

@@ -14759,7 +14759,8 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTestWithSadFrameTabReload,
GURL out_of_view_url(
embedded_test_server()->GetURL("a.com", "/iframe_out_of_view.html"));
EXPECT_TRUE(NavigateToURL(shell(), out_of_view_url));
EXPECT_EQ("LOADED", EvalJsWithManualReply(shell(), "notifyWhenLoaded();"));
EXPECT_EQ("LOADED", EvalJs(shell(), "notifyWhenLoaded();",
EXECUTE_SCRIPT_USE_MANUAL_REPLY));
NavigateIframeToURL(web_contents(), "test_iframe",
embedded_test_server()->GetURL("b.com", "/title1.html"));
@@ -15801,18 +15802,19 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTest,
ASSERT_TRUE(NavigateToURL(shell(), main_frame_url));
GURL cross_origin(embedded_test_server()->GetURL("b.com", "/title1.html"));
std::string msg =
EvalJsWithManualReply(
shell(), JsReplace("var object = document.createElement('object');"
"document.body.appendChild(object);"
"object.data = $1;"
"object.type='text/html';"
"object.notify = true;"
"object.onload = () => {"
" if (!object.notify) return;"
" object.notify = false;"
" window.domAutomationController.send('done');"
"};",
cross_origin))
EvalJs(shell(),
JsReplace("var object = document.createElement('object');"
"document.body.appendChild(object);"
"object.data = $1;"
"object.type='text/html';"
"object.notify = true;"
"object.onload = () => {"
" if (!object.notify) return;"
" object.notify = false;"
" window.domAutomationController.send('done');"
"};",
cross_origin),
EXECUTE_SCRIPT_USE_MANUAL_REPLY)
.ExtractString();
ASSERT_EQ("done", msg);
// To track the frame's visibility an EmbeddedContentView is needed. The

@@ -1601,10 +1601,11 @@ EvalJsResult EvalRunnerScript(const ToRenderFrameHost& execution_target,
<< "['" << token << "', [result]]; but got instead: " << *response
<< " ... This is potentially because a script tried to call "
"domAutomationController.send itself -- that is only allowed "
"when using EvalJsWithManualReply(). When using EvalJs(), result "
"values are just the result of calling eval() on the script -- "
"the completion value is the value of the last executed "
"statement. When using ExecJs(), there is no result value.";
"when using EXECUTE_SCRIPT_USE_MANUAL_REPLY. When using "
"EvalJs(), result values are just the result of calling eval() on "
"the script -- the completion value is the value of the last "
"executed statement. When using ExecJs(), there is no result "
"value.";
}
}
@@ -1698,14 +1699,6 @@ EvalJsResult EvalJs(const ToRenderFrameHost& execution_target,
token);
}
EvalJsResult EvalJsWithManualReply(const ToRenderFrameHost& execution_target,
const std::string& script,
int options,
int32_t world_id) {
return EvalJs(execution_target, script,
options | EXECUTE_SCRIPT_USE_MANUAL_REPLY, world_id);
}
EvalJsResult EvalJsAfterLifecycleUpdate(
const ToRenderFrameHost& execution_target,
const std::string& raf_script,

@@ -445,7 +445,7 @@ class ToRenderFrameHost {
RenderFrameHost* ConvertToRenderFrameHost(RenderFrameHost* render_view_host);
RenderFrameHost* ConvertToRenderFrameHost(WebContents* web_contents);
// Semi-deprecated: in new code, prefer ExecJs() -- it works the same, but has
// Deprecated: in new code, prefer ExecJs() -- it works the same, but has
// better error handling. (Note: still use ExecuteScript() on pages with a
// Content Security Policy).
//
@@ -490,9 +490,7 @@ void ExecuteScriptAsync(const ToRenderFrameHost& adapter,
// the executed script. They return true on success, false if the script
// execution failed or did not evaluate to the expected type.
//
// Semi-deprecated: Consider using EvalJs() or EvalJsWithManualReply() instead,
// which handle errors better and don't require an out-param. If the target
// document doesn't have a CSP. See the comment on EvalJs() for migration tips.
// Deprecated: Use EvalJs().
bool ExecuteScriptAndExtractDouble(const ToRenderFrameHost& adapter,
const std::string& script,
double* result) WARN_UNUSED_RESULT;
@@ -788,8 +786,8 @@ enum EvalJsOptions {
// * Preferred, but more rewriting: Use EvalJs with a Promise which
// resolves to the value you previously passed to send().
// * Less rewriting of |script|, but with some drawbacks: Use
// EXECUTE_SCRIPT_USE_MANUAL_REPLY in |options|, or EvalJsWithManualReply.
// When specified, this means that |script| must continue to call
// EXECUTE_SCRIPT_USE_MANUAL_REPLY in |options|. When specified, this
// means that |script| must continue to call
// domAutomationController.send(). Note that this option option disables
// some error-catching safeguards, but you still get the benefit of having
// an EvalJsResult that can be passed to EXPECT.
@@ -819,15 +817,6 @@ EvalJsResult EvalJs(const ToRenderFrameHost& execution_target,
int32_t world_id = ISOLATED_WORLD_ID_GLOBAL)
WARN_UNUSED_RESULT;
// Like EvalJs(), except that |script| must call domAutomationController.send()
// itself. This is the same as specifying the EXECUTE_SCRIPT_USE_MANUAL_REPLY
// option to EvalJs.
EvalJsResult EvalJsWithManualReply(const ToRenderFrameHost& execution_target,
const std::string& script,
int options = EXECUTE_SCRIPT_DEFAULT_OPTIONS,
int32_t world_id = ISOLATED_WORLD_ID_GLOBAL)
WARN_UNUSED_RESULT;
// Like EvalJs(), but runs |raf_script| inside a requestAnimationFrame handler,
// and runs |script| after the rendering update has completed. By the time
// this method returns, any IPCs sent from the renderer process to the browser

@@ -155,8 +155,9 @@ IN_PROC_BROWSER_TEST_F(EvalJsBrowserTest, EvalJsWithManualReply) {
std::string script = "window.domAutomationController.send(20); 'hi';";
// Calling domAutomationController is required for EvalJsWithManualReply.
EXPECT_EQ(20, EvalJsWithManualReply(shell(), script));
// Calling domAutomationController is required for
// EXECUTE_SCRIPT_USE_MANUAL_REPLY.
EXPECT_EQ(20, EvalJs(shell(), script, EXECUTE_SCRIPT_USE_MANUAL_REPLY));
// Calling domAutomationController is an error with EvalJs.
auto result = EvalJs(shell(), script);
@@ -169,9 +170,10 @@ IN_PROC_BROWSER_TEST_F(EvalJsBrowserTest, EvalJsWithManualReply) {
result.error,
::testing::EndsWith("This is potentially because a script tried to call "
"domAutomationController.send itself -- that is only "
"allowed when using EvalJsWithManualReply(). When "
"using EvalJs(), result values are just the result "
"of calling eval() on the script -- the completion "
"allowed when using "
"EXECUTE_SCRIPT_USE_MANUAL_REPLY. When using "
"EvalJs(), result values are just the result of "
"calling eval() on the script -- the completion "
"value is the value of the last executed statement. "
"When using ExecJs(), there is no result value."));
}