Skip to content

Commit

Permalink
Use EvalJS without EXECUTE_SCRIPT_USE_MANUAL_REPLY (#18754)
Browse files Browse the repository at this point in the history 
This PR changes the use of EvalJs in a similar fashion done upstream,
removing on the uses of `content::EXECUTE_SCRIPT_USE_MANUAL_REPLY`, and
modifying the scripts used in tests to return values directly, rather
than relying on `domAutomationController` to transmit back results,
which can be finicky.

With the new approach, values are returned through promises, or directly
as a return value.
  • Loading branch information
@cdesouza-chromium @emerick
cdesouza-chromium authored and emerick committed Jun 20, 2023
1 parent 8fd55f4 commit fc30f80
Show file tree
Hide file tree
Showing 24 changed files with 530 additions and 677 deletions.
15 changes: 5 additions & 10 deletions browser/autoplay/autoplay_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,31 +103,26 @@ class AutoplayBrowserTest : public InProcessBrowserTest {

void GotoAutoplayByAttr(bool muted) {
if (muted) {
ASSERT_EQ(true, EvalJs(contents(), "clickAutoplayByAttrMuted()",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY));
ASSERT_EQ(true, EvalJs(contents(), "clickAutoplayByAttrMuted()"));
} else {
ASSERT_EQ(true, EvalJs(contents(), "clickAutoplayByAttr()",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY));
ASSERT_EQ(true, EvalJs(contents(), "clickAutoplayByAttr()"));
}
ASSERT_TRUE(WaitForLoadStop(contents()));
WaitForCanPlay();
}

void GotoAutoplayByMethod(bool muted) {
if (muted) {
ASSERT_EQ(true, EvalJs(contents(), "clickAutoplayByMethodMuted()",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY));
ASSERT_EQ(true, EvalJs(contents(), "clickAutoplayByMethodMuted()"));
} else {
ASSERT_EQ(true, EvalJs(contents(), "clickAutoplayByMethod()",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY));
ASSERT_EQ(true, EvalJs(contents(), "clickAutoplayByMethod()"));
}
ASSERT_TRUE(WaitForLoadStop(contents()));
WaitForCanPlay();
}

void WaitForCanPlay() {
ASSERT_EQ("CANPLAY", EvalJs(contents(), "notifyWhenCanPlay()",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY));
ASSERT_EQ("CANPLAY", EvalJs(contents(), "notifyWhenCanPlay()"));
}

private:
Expand Down
34 changes: 18 additions & 16 deletions browser/brave_search/brave_search_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,18 @@ const char kScriptDefaultAPIGetValue[] = R"(
)";

std::string GetChromeFetchBackupResultsAvailScript() {
return base::StringPrintf(R"(function waitForFunction() {
setTimeout(waitForFunction, 200);
}
navigator.serviceWorker.addEventListener('message', msg => {
if (msg.data && msg.data.result === 'INJECTED') {
window.domAutomationController.send(msg.data.response === '%s');
} else if (msg.data && msg.data.result === 'FAILED') {
window.domAutomationController.send(false);
}});
waitForFunction();)",
return base::StringPrintf(R"(
new Promise(resolve => {
setTimeout(function () {
navigator.serviceWorker.addEventListener('message', msg => {
if (msg.data && msg.data.result === 'INJECTED') {
resolve(msg.data.response === '%s');
} else if (msg.data && msg.data.result === 'FAILED') {
resolve(false);
}});
}, 200)
});
)",
kBackupSearchContent);
}

Expand Down Expand Up @@ -195,8 +197,8 @@ IN_PROC_BROWSER_TEST_F(BraveSearchTest, CheckForAFunction) {
browser()->tab_strip_model()->GetActiveWebContents();
WaitForLoadStop(contents);

auto result_first = EvalJs(contents, GetChromeFetchBackupResultsAvailScript(),
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
auto result_first =
EvalJs(contents, GetChromeFetchBackupResultsAvailScript());
EXPECT_EQ(base::Value(true), result_first.value);
}

Expand All @@ -207,8 +209,8 @@ IN_PROC_BROWSER_TEST_F(BraveSearchTest, CheckForAFunctionDev) {
browser()->tab_strip_model()->GetActiveWebContents();
WaitForLoadStop(contents);

auto result_first = EvalJs(contents, GetChromeFetchBackupResultsAvailScript(),
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
auto result_first =
EvalJs(contents, GetChromeFetchBackupResultsAvailScript());
EXPECT_EQ(base::Value(true), result_first.value);
}

Expand All @@ -219,8 +221,8 @@ IN_PROC_BROWSER_TEST_F(BraveSearchTest, CheckForAnUndefinedFunction) {
browser()->tab_strip_model()->GetActiveWebContents();
WaitForLoadStop(contents);

auto result_first = EvalJs(contents, GetChromeFetchBackupResultsAvailScript(),
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
auto result_first =
EvalJs(contents, GetChromeFetchBackupResultsAvailScript());
EXPECT_EQ(base::Value(false), result_first.value);
}

Expand Down
70 changes: 26 additions & 44 deletions browser/brave_shields/ad_block_service_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1940,20 +1940,17 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringSimple) {
browser()->tab_strip_model()->GetActiveWebContents();

auto result_first =
EvalJs(contents, R"(waitCSSSelector('#ad-banner', 'display', 'none'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
EvalJs(contents, R"(waitCSSSelector('#ad-banner', 'display', 'none'))");
ASSERT_TRUE(result_first.error.empty());
EXPECT_EQ(base::Value(true), result_first.value);

auto result_second =
EvalJs(contents, R"(waitCSSSelector('.ad-banner', 'display', 'block'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
EvalJs(contents, R"(waitCSSSelector('.ad-banner', 'display', 'block'))");
ASSERT_TRUE(result_second.error.empty());
EXPECT_EQ(base::Value(true), result_second.value);

auto result_third =
EvalJs(contents, R"(waitCSSSelector('.ad', 'display', 'none'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
EvalJs(contents, R"(waitCSSSelector('.ad', 'display', 'none'))");
ASSERT_TRUE(result_third.error.empty());
EXPECT_EQ(base::Value(true), result_third.value);
}
Expand Down Expand Up @@ -2097,8 +2094,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringHide1pContent) {
browser()->tab_strip_model()->GetActiveWebContents();

auto result =
EvalJs(contents, R"(waitCSSSelector('.fpsponsored', 'display', 'none'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
EvalJs(contents, R"(waitCSSSelector('.fpsponsored', 'display', 'none'))");
ASSERT_TRUE(result.error.empty());
EXPECT_EQ(base::Value(true), result.value);
}
Expand All @@ -2117,22 +2113,19 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringDynamic) {

auto result_first = EvalJs(contents,
R"(addElementsDynamically();
waitCSSSelector('.blockme', 'display', 'none'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
waitCSSSelector('.blockme', 'display', 'none'))");
ASSERT_TRUE(result_first.error.empty());
EXPECT_EQ(base::Value(true), result_first.value);

auto result_second =
EvalJs(contents, R"(waitCSSSelector('.dontblockme', 'display', 'block'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
auto result_second = EvalJs(
contents, R"(waitCSSSelector('.dontblockme', 'display', 'block'))");
ASSERT_TRUE(result_second.error.empty());
EXPECT_EQ(base::Value(true), result_second.value);

// this class is added by setting an element's innerHTML, which doesn't
// trigger a MutationObserver update
auto result_third = EvalJs(
contents, R"(waitCSSSelector('.hide-innerhtml', 'display', 'none'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
contents, R"(waitCSSSelector('.hide-innerhtml', 'display', 'none'))");
ASSERT_TRUE(result_third.error.empty());
EXPECT_EQ(base::Value(true), result_third.value);
}
Expand All @@ -2154,14 +2147,12 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringDynamicCustom) {

auto result_first = EvalJs(contents,
R"(addElementsDynamically();
waitCSSSelector('.blockme', 'display', 'none'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
waitCSSSelector('.blockme', 'display', 'none'))");
ASSERT_TRUE(result_first.error.empty());
EXPECT_EQ(base::Value(true), result_first.value);

auto result_second =
EvalJs(contents, R"(waitCSSSelector('.dontblockme', 'display', 'block'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
auto result_second = EvalJs(
contents, R"(waitCSSSelector('.dontblockme', 'display', 'block'))");
ASSERT_TRUE(result_second.error.empty());
EXPECT_EQ(base::Value(true), result_second.value);
}
Expand Down Expand Up @@ -2206,8 +2197,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringCustomStyle) {
browser()->tab_strip_model()->GetActiveWebContents();

auto result =
EvalJs(contents, R"(waitCSSSelector('.ad', 'padding-bottom', '10px'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
EvalJs(contents, R"(waitCSSSelector('.ad', 'padding-bottom', '10px'))");
ASSERT_TRUE(result.error.empty());
EXPECT_EQ(base::Value(true), result.value);
}
Expand All @@ -2229,14 +2219,12 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringUnhide) {
browser()->tab_strip_model()->GetActiveWebContents();

auto result_first =
EvalJs(contents, R"(waitCSSSelector('.ad', 'display', 'block'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
EvalJs(contents, R"(waitCSSSelector('.ad', 'display', 'block'))");
ASSERT_TRUE(result_first.error.empty());
EXPECT_EQ(base::Value(true), result_first.value);

auto result_second =
EvalJs(contents, R"(waitCSSSelector('#ad-banner', 'display', 'none'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
EvalJs(contents, R"(waitCSSSelector('#ad-banner', 'display', 'none'))");
ASSERT_TRUE(result_second.error.empty());
EXPECT_EQ(base::Value(true), result_second.value);
}
Expand Down Expand Up @@ -2269,9 +2257,8 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringWindowScriptlet) {
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();

auto result =
EvalJs(contents, R"(waitCSSSelector('.ad', 'color', 'Impossible value'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
auto result = EvalJs(
contents, R"(waitCSSSelector('.ad', 'color', 'Impossible value'))");
ASSERT_TRUE(result.error.empty());
EXPECT_EQ(base::Value(true), result.value);
}
Expand Down Expand Up @@ -2305,8 +2292,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CheckForDeAmpPref) {
embedded_test_server()->GetURL("b.com", "/cosmetic_filtering.html");
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
auto result1 =
EvalJs(web_contents(), R"(waitCSSSelector('body', 'color', 'green'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
EvalJs(web_contents(), R"(waitCSSSelector('body', 'color', 'green'))");
ASSERT_TRUE(result1.error.empty());
EXPECT_EQ(base::Value(true), result1.value);

Expand All @@ -2317,8 +2303,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CheckForDeAmpPref) {
EXPECT_TRUE(WaitForLoadStop(web_contents()));

auto result2 =
EvalJs(web_contents(), R"(waitCSSSelector('body', 'color', 'red'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
EvalJs(web_contents(), R"(waitCSSSelector('body', 'color', 'red'))");
ASSERT_TRUE(result2.error.empty());
EXPECT_EQ(base::Value(true), result2.value);
}
Expand Down Expand Up @@ -2365,10 +2350,9 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest,
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();

auto result_first =
EvalJs(contents,
R"(waitCSSSelector('#inline-block-important', 'display', 'none'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
auto result_first = EvalJs(
contents,
R"(waitCSSSelector('#inline-block-important', 'display', 'none'))");
ASSERT_TRUE(result_first.error.empty());
EXPECT_EQ(base::Value(true), result_first.value);
}
Expand All @@ -2388,10 +2372,9 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest,
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();

auto result_first =
EvalJs(contents,
R"(waitCSSSelector('#inline-block-important', 'display', 'none'))",
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY);
auto result_first = EvalJs(
contents,
R"(waitCSSSelector('#inline-block-important', 'display', 'none'))");
ASSERT_TRUE(result_first.error.empty());
EXPECT_EQ(base::Value(true), result_first.value);
}
Expand Down Expand Up @@ -2493,9 +2476,8 @@ class AdBlockServiceTestJsPerformance : public AdBlockServiceTest {
const std::string& selector) const {
const char kTemplate[] = R"(waitCSSSelector($1, 'display', 'none'))";

ASSERT_TRUE(EvalJs(target, content::JsReplace(kTemplate, selector),
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY)
.ExtractBool());
ASSERT_TRUE(
EvalJs(target, content::JsReplace(kTemplate, selector)).ExtractBool());
}

void NonBlockingDelay(const base::TimeDelta& delay) {
Expand Down
Loading

0 comments on commit fc30f80

Please sign in to comment.