Skip to content

Commit

Permalink
[Extensions] Add WARN_UNUSED_RESULT on waiting for a test message
Browse files Browse the repository at this point in the history
ExtensionTestMessageListener::WaitUntilSatisfied() is used to wait for
a specific message from an extension and return a result.
WaitUntilSatisfied() returns a bool indicating success, but this can be
false (i.e., failure) in certain circumstances. Add WARN_UNUSED_RESULT
so that callers must handle the returned bool, rather than only waiting
for the listener to be satisfied.

Bug: None
TBR=benwells@chromium.org (apps/, c/b/apps/)
TBR=stevenjb@chromium.org (c/b/chromeos, c/b/ui/ash/launcher/)

Change-Id: Ied7b2cd5e5006531a117158812dad5d39b673786
Reviewed-on: https://chromium-review.googlesource.com/989278
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548283}
  • Loading branch information
rdcronin authored and Commit Bot committed Apr 5, 2018
1 parent aba5ce4 commit f34c8ee
Show file tree
Hide file tree
Showing 21 changed files with 51 additions and 52 deletions.
4 changes: 2 additions & 2 deletions apps/app_restore_service_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, RunningAppsAreRecorded) {
ExtensionTestMessageListener restart_listener("onRestarted", false);
apps::AppRestoreServiceFactory::GetForBrowserContext(browser()->profile())
->HandleStartup(true);
restart_listener.WaitUntilSatisfied();
EXPECT_TRUE(restart_listener.WaitUntilSatisfied());
}

// Tests that apps are recorded in the preferences as active when and only when
Expand Down Expand Up @@ -196,7 +196,7 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, MAYBE_FileAccessIsRestored) {
apps::AppRestoreServiceFactory::GetForBrowserContext(browser()->profile())
->HandleStartup(true);

access_ok_listener.WaitUntilSatisfied();
EXPECT_TRUE(access_ok_listener.WaitUntilSatisfied());
}

} // namespace apps
2 changes: 1 addition & 1 deletion chrome/browser/apps/app_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1381,7 +1381,7 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, AppsIgnoreDefaultZoom) {
// made it through.
ExtensionTestMessageListener launched_listener("Launched", false);
LaunchPlatformApp(extension);
launched_listener.WaitUntilSatisfied();
EXPECT_TRUE(launched_listener.WaitUntilSatisfied());

// Now check that the app window's default zoom, and actual zoom level,
// have not been changed from the default.
Expand Down
23 changes: 11 additions & 12 deletions chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,7 @@ IN_PROC_BROWSER_TEST_P(WebViewFocusInteractiveTest,
std::unique_ptr<ExtensionTestMessageListener> done_listener(
RunAppHelper("testFocusTracksEmbedder", "web_view/focus", NO_TEST_SERVER,
&embedder_web_contents));
done_listener->WaitUntilSatisfied();
EXPECT_TRUE(done_listener->WaitUntilSatisfied());

ExtensionTestMessageListener next_step_listener("TEST_STEP_PASSED", false);
next_step_listener.set_failure_message("TEST_STEP_FAILED");
Expand All @@ -884,7 +884,7 @@ IN_PROC_BROWSER_TEST_P(WebViewFocusInteractiveTest, Focus_AdvanceFocus) {
std::unique_ptr<ExtensionTestMessageListener> done_listener(
RunAppHelper("testAdvanceFocus", "web_view/focus", NO_TEST_SERVER,
&embedder_web_contents));
done_listener->WaitUntilSatisfied();
EXPECT_TRUE(done_listener->WaitUntilSatisfied());
}

{
Expand Down Expand Up @@ -1129,7 +1129,7 @@ IN_PROC_BROWSER_TEST_P(WebViewInteractiveTest, NewWindow_OpenInNewTab) {
RunAppHelper("testNewWindowOpenInNewTab", "web_view/newwindow",
NEEDS_TEST_SERVER, &embedder_web_contents));

loaded_listener.WaitUntilSatisfied();
EXPECT_TRUE(loaded_listener.WaitUntilSatisfied());
#if defined(OS_MACOSX)
ASSERT_TRUE(ui_test_utils::SendKeyPressToWindowSync(
GetPlatformAppWindow(), ui::VKEY_RETURN,
Expand Down Expand Up @@ -1217,7 +1217,7 @@ IN_PROC_BROWSER_TEST_F(WebViewBrowserPluginInteractiveTest,
// Embedder should be focused.
EXPECT_EQ(guest_web_contents,
content::GetFocusedWebContents(guest_web_contents));
listener.WaitUntilSatisfied();
EXPECT_TRUE(listener.WaitUntilSatisfied());

// Check that the inner contents is correctly focused.
bool result;
Expand All @@ -1232,7 +1232,7 @@ IN_PROC_BROWSER_TEST_F(WebViewBrowserPluginInteractiveTest,
listener.Reset();
EXPECT_TRUE(
content::ExecuteScript(embedder_web_contents, "reloadWebview();"));
listener.WaitUntilSatisfied();
EXPECT_TRUE(listener.WaitUntilSatisfied());

// Check that the inner contents is correctly focused after a reload.
EXPECT_TRUE(content::ExecuteScriptAndExtractBool(
Expand Down Expand Up @@ -1676,7 +1676,7 @@ IN_PROC_BROWSER_TEST_P(WebViewFocusInteractiveTest, MAYBE_FocusAndVisibility) {
ExtensionTestMessageListener test_init_listener(
"WebViewInteractiveTest.WebViewInitialized", false);
SendMessageToEmbedder(GetParam() ? "init-oopif" : "init");
test_init_listener.WaitUntilSatisfied();
EXPECT_TRUE(test_init_listener.WaitUntilSatisfied());

// Send several tab-keys. The button inside webview should receive focus at
// least once.
Expand Down Expand Up @@ -1706,12 +1706,11 @@ IN_PROC_BROWSER_TEST_P(WebViewFocusInteractiveTest, MAYBE_FocusAndVisibility) {
ExtensionTestMessageListener reset_listener("WebViewInteractiveTest.DidReset",
false);
SendMessageToEmbedder("reset");
reset_listener.WaitUntilSatisfied();
EXPECT_TRUE(reset_listener.WaitUntilSatisfied());
ExtensionTestMessageListener did_hide_webview_listener(
"WebViewInteractiveTest.DidHideWebView", false);
SendMessageToEmbedder("hide-webview");
did_hide_webview_listener.WaitUntilSatisfied();

EXPECT_TRUE(did_hide_webview_listener.WaitUntilSatisfied());

// Send the same number of keys and verify that the webview button was not
// this time.
Expand Down Expand Up @@ -1872,7 +1871,7 @@ IN_PROC_BROWSER_TEST_P(WebViewImeInteractiveTest,
content::SimulateMouseClickAt(target_web_contents, 0,
blink::WebMouseEvent::Button::kLeft,
gfx::Point(50, 50));
focus_listener.WaitUntilSatisfied();
EXPECT_TRUE(focus_listener.WaitUntilSatisfied());

// Verify the text inside the <input> is "A B X D".
std::string value;
Expand All @@ -1893,7 +1892,7 @@ IN_PROC_BROWSER_TEST_P(WebViewImeInteractiveTest,
content::SendImeCommitTextToWidget(
target_rwh_for_input, base::UTF8ToUTF16("C"),
std::vector<ui::ImeTextSpan>(), gfx::Range(4, 5), 0);
input_listener.WaitUntilSatisfied();
EXPECT_TRUE(input_listener.WaitUntilSatisfied());

// Get the input value from the guest.
value.clear();
Expand Down Expand Up @@ -1939,7 +1938,7 @@ IN_PROC_BROWSER_TEST_P(WebViewImeInteractiveTest, CompositionRangeUpdates) {
content::SimulateMouseClickAt(target_web_contents, 0,
blink::WebMouseEvent::Button::kLeft,
gfx::Point(50, 50));
focus_listener.WaitUntilSatisfied();
EXPECT_TRUE(focus_listener.WaitUntilSatisfied());

// Clear the string as it already contains some text. Then verify the text in
// the <input> is empty.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ IN_PROC_BROWSER_TEST_F(KioskAppUpdateServiceTest, AppUpdate) {

ExtensionTestMessageListener listener("app_update", false);
FireAppUpdateAvailable();
listener.WaitUntilSatisfied();
EXPECT_TRUE(listener.WaitUntilSatisfied());
}

// Verifies that the app is notified a reboot is required when an OS update is
Expand All @@ -189,7 +189,7 @@ IN_PROC_BROWSER_TEST_F(KioskAppUpdateServiceTest, OsUpdate) {
g_browser_process->local_state()->SetBoolean(prefs::kRebootAfterUpdate, true);
ExtensionTestMessageListener listener("os_update", false);
FireUpdatedNeedReboot();
listener.WaitUntilSatisfied();
EXPECT_TRUE(listener.WaitUntilSatisfied());
}

// Verifies that the app is notified a reboot is required when a periodic reboot
Expand All @@ -199,7 +199,7 @@ IN_PROC_BROWSER_TEST_F(KioskAppUpdateServiceTest, Periodic) {

ExtensionTestMessageListener listener("periodic", false);
RequestPeriodicReboot();
listener.WaitUntilSatisfied();
EXPECT_TRUE(listener.WaitUntilSatisfied());
}

// Verifies that the app is notified a reboot is required when an OS update was
Expand All @@ -211,7 +211,7 @@ IN_PROC_BROWSER_TEST_F(KioskAppUpdateServiceTest, StartAfterOsUpdate) {

ExtensionTestMessageListener listener("os_update", false);
CreateKioskAppUpdateService();
listener.WaitUntilSatisfied();
EXPECT_TRUE(listener.WaitUntilSatisfied());
}

// Verifies that the app is notified a reboot is required when a periodic reboot
Expand All @@ -221,7 +221,7 @@ IN_PROC_BROWSER_TEST_F(KioskAppUpdateServiceTest, StartAfterPeriodic) {

ExtensionTestMessageListener listener("periodic", false);
CreateKioskAppUpdateService();
listener.WaitUntilSatisfied();
EXPECT_TRUE(listener.WaitUntilSatisfied());
}

} // namespace chromeos
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ IN_PROC_BROWSER_TEST_F(ActionHandlersBrowserTest, LaunchAppWithNewNote) {
ASSERT_TRUE(app);
EXPECT_TRUE(extensions::ActionHandlersInfo::HasActionHandler(
app, app_runtime::ACTION_TYPE_NEW_NOTE));
loader.WaitUntilSatisfied();
EXPECT_TRUE(loader.WaitUntilSatisfied());

// Fire a "new_note" action type, assert that app has received it.
ExtensionTestMessageListener new_note("hasNewNote = true", false);
auto action_data = std::make_unique<app_runtime::ActionData>();
action_data->action_type = app_runtime::ActionType::ACTION_TYPE_NEW_NOTE;
apps::LaunchPlatformAppWithAction(profile(), app, std::move(action_data),
base::FilePath());
new_note.WaitUntilSatisfied();
EXPECT_TRUE(new_note.WaitUntilSatisfied());
}
2 changes: 1 addition & 1 deletion chrome/browser/chromeos/login/kiosk_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -836,7 +836,7 @@ IN_PROC_BROWSER_TEST_F(KioskTest, ZoomSupport) {
ExtensionTestMessageListener app_window_loaded_listener("appWindowLoaded",
false);
StartAppLaunchFromLoginScreen(SimulateNetworkOnlineClosure());
app_window_loaded_listener.WaitUntilSatisfied();
EXPECT_TRUE(app_window_loaded_listener.WaitUntilSatisfied());

Profile* app_profile = ProfileManager::GetPrimaryUserProfile();
ASSERT_TRUE(app_profile);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@ IN_PROC_BROWSER_TEST_F(BluetoothLowEnergyApiTest, ReadCharacteristicValue) {
listener.set_failure_message("fail");
ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII(
"bluetooth_low_energy/read_characteristic_value")));
listener.WaitUntilSatisfied();
EXPECT_TRUE(listener.WaitUntilSatisfied());

listener.Reply("go");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ testing::AssertionResult RequestContentScriptAPITest::CreateAndLoadExtension(
extension_ = extension;

// Wait for rules to be setup before navigating to trigger script injection.
injection_setup_listener.WaitUntilSatisfied();
EXPECT_TRUE(injection_setup_listener.WaitUntilSatisfied());

return testing::AssertionSuccess();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest,

Browser* incognito_browser =
OpenURLOffTheRecord(profile(), GURL("chrome://newtab/"));
listener.WaitUntilSatisfied();
EXPECT_TRUE(listener.WaitUntilSatisfied());
EXPECT_EQ(std::string("opened"), listener.message());
auto test_util = BrowserActionTestUtil::Create(incognito_browser);
EXPECT_TRUE(test_util->HasPopup());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class InlineInstallPrivateApiTestBase : public WebstoreInstallerTest {
ExtensionTestMessageListener ready_listener("ready", true);
ExtensionTestMessageListener success_listener("success", false);
LoadExtension(test_driver_path);
ready_listener.WaitUntilSatisfied();
EXPECT_TRUE(ready_listener.WaitUntilSatisfied());
ready_listener.Reply(testName);
ASSERT_TRUE(success_listener.WaitUntilSatisfied());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementTest, MAYBE_AutoUpdate) {
ASSERT_TRUE(registry->disabled_extensions().is_empty());
const Extension* extension = InstallExtension(v1_path, 1);
ASSERT_TRUE(extension);
listener1.WaitUntilSatisfied();
EXPECT_TRUE(listener1.WaitUntilSatisfied());
ASSERT_EQ(size_before + 1, registry->enabled_extensions().size());
ASSERT_EQ("ogjcoiohnmldgjemafoockdghcjciccf", extension->id());
ASSERT_EQ("1.0", extension->VersionString());
Expand All @@ -380,7 +380,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementTest, MAYBE_AutoUpdate) {
extensions::TestExtensionRegistryObserver install_observer(registry);
service->updater()->CheckNow(params);
install_observer.WaitForExtensionWillBeInstalled();
listener2.WaitUntilSatisfied();
EXPECT_TRUE(listener2.WaitUntilSatisfied());
ASSERT_EQ(size_before + 1, registry->enabled_extensions().size());
extension = service->GetExtensionById(
"ogjcoiohnmldgjemafoockdghcjciccf", false);
Expand Down Expand Up @@ -466,7 +466,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementTest,
const size_t disabled_size_before = registry->disabled_extensions().size();
const Extension* extension = InstallExtension(v1_path, 1);
ASSERT_TRUE(extension);
listener1.WaitUntilSatisfied();
EXPECT_TRUE(listener1.WaitUntilSatisfied());
DisableExtension(extension->id());
ASSERT_EQ(disabled_size_before + 1, registry->disabled_extensions().size());
ASSERT_EQ(enabled_size_before, registry->enabled_extensions().size());
Expand Down Expand Up @@ -497,7 +497,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementTest,
// When we enabled it, it should then make the callback.
ASSERT_FALSE(listener2.was_satisfied());
EnableExtension(extension->id());
listener2.WaitUntilSatisfied();
EXPECT_TRUE(listener2.WaitUntilSatisfied());
ASSERT_TRUE(notification_listener.started());
ASSERT_TRUE(notification_listener.finished());
ASSERT_TRUE(base::ContainsKey(notification_listener.updates(),
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/extensions/content_script_apitest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ IN_PROC_BROWSER_TEST_F(ContentScriptApiTest,
false);
LoadExtension(data_dir.AppendASCII("script_a_com"));
LoadExtension(data_dir.AppendASCII("background_page_iframe"));
iframe_loaded_listener.WaitUntilSatisfied();
EXPECT_TRUE(iframe_loaded_listener.WaitUntilSatisfied());
EXPECT_FALSE(content_script_listener.was_satisfied());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ testing::AssertionResult ActiveScriptTester::Verify() {
// If the extension has permission, we should be able to simply wait for it
// to execute.
if (requires_consent_ == DOES_NOT_REQUIRE_CONSENT) {
inject_success_listener_->WaitUntilSatisfied();
EXPECT_TRUE(inject_success_listener_->WaitUntilSatisfied());
return testing::AssertionSuccess();
}

Expand All @@ -286,7 +286,7 @@ testing::AssertionResult ActiveScriptTester::Verify() {
runner->RunAction(extension_, true);

// Now, the extension should be able to inject the script.
inject_success_listener_->WaitUntilSatisfied();
EXPECT_TRUE(inject_success_listener_->WaitUntilSatisfied());

// The extension should no longer want to run.
wants_to_run = WantsToRun();
Expand Down Expand Up @@ -388,7 +388,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionActionRunnerBrowserTest,
false /* won't reply */));
inject_success_listener.set_extension_id(extension1->id());
action_runner->RunAction(extension1, true);
inject_success_listener.WaitUntilSatisfied();
EXPECT_TRUE(inject_success_listener.WaitUntilSatisfied());
}

// Test that granting the extension all urls permission allows it to run on
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionContextMenuBrowserTest, Separators) {
ExtensionTestMessageListener listener1("test1 create finished", false);
ui_test_utils::NavigateToURL(browser(),
GURL(extension->GetResourceURL("test1.html")));
listener1.WaitUntilSatisfied();
EXPECT_TRUE(listener1.WaitUntilSatisfied());

GURL url("http://www.google.com/");
std::unique_ptr<TestRenderViewContextMenu> menu(
Expand Down Expand Up @@ -642,7 +642,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionContextMenuBrowserTest, Separators) {
ExtensionTestMessageListener listener2("test2 create finished", false);
ui_test_utils::NavigateToURL(browser(),
GURL(extension->GetResourceURL("test2.html")));
listener2.WaitUntilSatisfied();
EXPECT_TRUE(listener2.WaitUntilSatisfied());
menu =
TestRenderViewContextMenu::Create(GetWebContents(), url, GURL(), GURL());
ASSERT_TRUE(menu->GetMenuModelAndItemIndex(
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/extensions/extension_keybinding_apitest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ IN_PROC_BROWSER_TEST_F(CommandsApiTest, PageAction) {
ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
browser(), ui::VKEY_F, false, true, true, false));

test_listener.WaitUntilSatisfied();
EXPECT_TRUE(test_listener.WaitUntilSatisfied());
EXPECT_EQ("clicked", test_listener.message());
}

Expand Down Expand Up @@ -281,7 +281,7 @@ IN_PROC_BROWSER_TEST_F(CommandsApiTest, PageActionKeyUpdated) {
ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
browser(), ui::VKEY_G, false, true, true, false));

test_listener.WaitUntilSatisfied();
EXPECT_TRUE(test_listener.WaitUntilSatisfied());
EXPECT_EQ("clicked", test_listener.message());
}

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/extensions/lazy_background_page_apitest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ IN_PROC_BROWSER_TEST_F(LazyBackgroundPageApiTest, NaClInBackgroundPage) {
ExtensionTestMessageListener nacl_module_loaded("nacl_module_loaded",
false);
BrowserActionTestUtil::Create(browser())->Press(0);
nacl_module_loaded.WaitUntilSatisfied();
EXPECT_TRUE(nacl_module_loaded.WaitUntilSatisfied());
content::RunAllTasksUntilIdle();
EXPECT_TRUE(IsBackgroundPageAlive(last_loaded_extension_id()));
}
Expand Down
Loading

0 comments on commit f34c8ee

Please sign in to comment.