Commit da588d38 authored by David Bertoni's avatar David Bertoni Committed by Commit Bot

[Extensions] Fix a flaky declarativeContent API test.

DeclarativeContentApiTest.Overview was flaky because it did not
properly wait for page action icon visibility changes before
checking the visibility state.

Bug: 606574
Change-Id: Iaffc72868df454ceabb3c5d6447c9aa37820a5b6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2229581Reviewed-by: default avatarKaran Bhatia <karandeepb@chromium.org>
Commit-Queue: David Bertoni <dbertoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776820}
parent 0bf26137
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/bookmarks/bookmark_model_factory.h" #include "chrome/browser/bookmarks/bookmark_model_factory.h"
#include "chrome/browser/extensions/chrome_extension_test_notification_observer.h"
#include "chrome/browser/extensions/chrome_test_extension_loader.h" #include "chrome/browser/extensions/chrome_test_extension_loader.h"
#include "chrome/browser/extensions/extension_action_manager.h" #include "chrome/browser/extensions/extension_action_manager.h"
#include "chrome/browser/extensions/extension_action_test_util.h" #include "chrome/browser/extensions/extension_action_test_util.h"
...@@ -257,8 +258,7 @@ void DeclarativeContentApiTest::CheckBookmarkEvents(bool match_is_bookmarked) { ...@@ -257,8 +258,7 @@ void DeclarativeContentApiTest::CheckBookmarkEvents(bool match_is_bookmarked) {
EXPECT_EQ(!match_is_bookmarked, action->GetIsVisible(tab_id)); EXPECT_EQ(!match_is_bookmarked, action->GetIsVisible(tab_id));
} }
// Disabled due to flake. https://crbug.com/606574. IN_PROC_BROWSER_TEST_F(DeclarativeContentApiTest, Overview) {
IN_PROC_BROWSER_TEST_F(DeclarativeContentApiTest, DISABLED_Overview) {
ext_dir_.WriteManifest(kDeclarativeContentManifest); ext_dir_.WriteManifest(kDeclarativeContentManifest);
ext_dir_.WriteFile( ext_dir_.WriteFile(
FILE_PATH_LITERAL("background.js"), FILE_PATH_LITERAL("background.js"),
...@@ -295,14 +295,20 @@ IN_PROC_BROWSER_TEST_F(DeclarativeContentApiTest, DISABLED_Overview) { ...@@ -295,14 +295,20 @@ IN_PROC_BROWSER_TEST_F(DeclarativeContentApiTest, DISABLED_Overview) {
browser()->tab_strip_model()->GetWebContentsAt(0); browser()->tab_strip_model()->GetWebContentsAt(0);
const int tab_id = ExtensionTabUtil::GetTabId(tab); const int tab_id = ExtensionTabUtil::GetTabId(tab);
NavigateInRenderer(tab, GURL("http://test1/")); // Observer to track page action visibility. This helps avoid
// flakes by waiting to check visibility until there is an
// actual update to the page action.
ChromeExtensionTestNotificationObserver test_observer(browser());
NavigateInRenderer(tab, GURL("http://test1/"));
// The declarative API should show the page action instantly, rather // The declarative API should show the page action instantly, rather
// than waiting for the extension to run. // than waiting for the extension to run.
test_observer.WaitForPageActionVisibilityChangeTo(1);
EXPECT_TRUE(action->GetIsVisible(tab_id)); EXPECT_TRUE(action->GetIsVisible(tab_id));
// Make sure leaving a matching page unshows the page action. // Make sure leaving a matching page unshows the page action.
NavigateInRenderer(tab, GURL("http://not_checked/")); NavigateInRenderer(tab, GURL("http://not_checked/"));
test_observer.WaitForPageActionVisibilityChangeTo(0);
EXPECT_FALSE(action->GetIsVisible(tab_id)); EXPECT_FALSE(action->GetIsVisible(tab_id));
// Insert a password field to make sure that's noticed. // Insert a password field to make sure that's noticed.
...@@ -311,10 +317,7 @@ IN_PROC_BROWSER_TEST_F(DeclarativeContentApiTest, DISABLED_Overview) { ...@@ -311,10 +317,7 @@ IN_PROC_BROWSER_TEST_F(DeclarativeContentApiTest, DISABLED_Overview) {
tab, "document.body.innerHTML = '<input type=\"password\">';" tab, "document.body.innerHTML = '<input type=\"password\">';"
"document.body.offsetTop;")); "document.body.offsetTop;"));
// Give the style match a chance to run and send back the matching-selector test_observer.WaitForPageActionVisibilityChangeTo(1);
// update.
ASSERT_TRUE(content::ExecuteScript(tab, std::string()));
EXPECT_TRUE(action->GetIsVisible(tab_id)) EXPECT_TRUE(action->GetIsVisible(tab_id))
<< "Adding a matching element should show the page action."; << "Adding a matching element should show the page action.";
...@@ -324,10 +327,7 @@ IN_PROC_BROWSER_TEST_F(DeclarativeContentApiTest, DISABLED_Overview) { ...@@ -324,10 +327,7 @@ IN_PROC_BROWSER_TEST_F(DeclarativeContentApiTest, DISABLED_Overview) {
tab, "document.body.innerHTML = 'Hello world';" tab, "document.body.innerHTML = 'Hello world';"
"document.body.offsetTop;")); "document.body.offsetTop;"));
// Give the style match a chance to run and send back the matching-selector test_observer.WaitForPageActionVisibilityChangeTo(0);
// update.
ASSERT_TRUE(content::ExecuteScript(tab, std::string()));
EXPECT_FALSE(action->GetIsVisible(tab_id)) EXPECT_FALSE(action->GetIsVisible(tab_id))
<< "Removing the matching element should hide the page action again."; << "Removing the matching element should hide the page action again.";
} }
......
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