Index: chrome/browser/ui/app_list/arc/arc_app_unittest.cc |
diff --git a/chrome/browser/ui/app_list/arc/arc_app_unittest.cc b/chrome/browser/ui/app_list/arc/arc_app_unittest.cc |
index 508bac33da23ea4d6d563e321a85e67e41768423..a30fa54e229c7d7e38c1fe25cd6d9a3a4188edc7 100644 |
--- a/chrome/browser/ui/app_list/arc/arc_app_unittest.cc |
+++ b/chrome/browser/ui/app_list/arc/arc_app_unittest.cc |
@@ -66,6 +66,18 @@ class FakeAppIconLoaderDelegate : public AppIconLoaderDelegate { |
DISALLOW_COPY_AND_ASSIGN(FakeAppIconLoaderDelegate); |
}; |
+void WaitForIconReady(ArcAppListPrefs* prefs, |
+ const std::string& app_id, |
+ ui::ScaleFactor scale_factor) { |
+ const base::FilePath icon_path = prefs->GetIconPath(app_id, scale_factor); |
+ // Process pending tasks. This performs multiple thread hops, so we need |
+ // to run it continuously until it is resolved. |
+ do { |
+ content::BrowserThread::GetBlockingPool()->FlushForTesting(); |
+ base::RunLoop().RunUntilIdle(); |
+ } while (!base::PathExists(icon_path)); |
+} |
+ |
} // namespace |
class ArcAppModelBuilderTest : public AppListTestBase { |
@@ -80,6 +92,9 @@ class ArcAppModelBuilderTest : public AppListTestBase { |
AppListTestBase::SetUp(); |
arc_test_.SetUp(profile_.get()); |
CreateBuilder(); |
+ |
+ // Validating decoded content does not fit well for unit tests. |
+ ArcAppIcon::DisableSafeDecodingForTesting(); |
} |
void TearDown() override { |
@@ -547,9 +562,9 @@ TEST_F(ArcAppModelBuilderTest, LaunchApps) { |
const ScopedVector<arc::FakeAppInstance::Request>& launch_requests = |
app_instance()->launch_requests(); |
ASSERT_EQ(3u, launch_requests.size()); |
- EXPECT_EQ(true, launch_requests[0]->IsForApp(app_first)); |
- EXPECT_EQ(true, launch_requests[1]->IsForApp(app_last)); |
- EXPECT_EQ(true, launch_requests[2]->IsForApp(app_first)); |
+ EXPECT_TRUE(launch_requests[0]->IsForApp(app_first)); |
+ EXPECT_TRUE(launch_requests[1]->IsForApp(app_last)); |
+ EXPECT_TRUE(launch_requests[2]->IsForApp(app_first)); |
// Test an attempt to launch of a not-ready app. |
bridge_service()->SetStopped(); |
@@ -587,9 +602,9 @@ TEST_F(ArcAppModelBuilderTest, LaunchShortcuts) { |
const ScopedVector<mojo::String>& launch_intents = |
app_instance()->launch_intents(); |
ASSERT_EQ(3u, launch_intents.size()); |
- EXPECT_EQ(true, app_first.intent_uri == *launch_intents[0]); |
- EXPECT_EQ(true, app_last.intent_uri == *launch_intents[1]); |
- EXPECT_EQ(true, app_first.intent_uri == *launch_intents[2]); |
+ EXPECT_EQ(app_first.intent_uri, *launch_intents[0]); |
+ EXPECT_EQ(app_last.intent_uri, *launch_intents[1]); |
+ EXPECT_EQ(app_first.intent_uri, *launch_intents[2]); |
// Test an attempt to launch of a not-ready shortcut. |
bridge_service()->SetStopped(); |
@@ -622,12 +637,14 @@ TEST_F(ArcAppModelBuilderTest, RequestIcons) { |
ASSERT_NE(nullptr, app_item); |
const float scale = ui::GetScaleForScaleFactor(scale_factor); |
app_item->icon().GetRepresentation(scale); |
+ |
+ // This does not result in an icon being loaded, so WaitForIconReady |
+ // cannot be used. |
+ content::BrowserThread::GetBlockingPool()->FlushForTesting(); |
+ base::RunLoop().RunUntilIdle(); |
} |
} |
- // Process pending tasks. |
- content::BrowserThread::GetBlockingPool()->FlushForTesting(); |
- base::RunLoop().RunUntilIdle(); |
// Normally just one call to RunUntilIdle() suffices to make sure |
// all RequestAppIcon() calls are delivered, but on slower machines |
// (especially when running under Valgrind), they might not get |
@@ -662,8 +679,7 @@ TEST_F(ArcAppModelBuilderTest, RequestIcons) { |
} |
} |
-// TODO(crbug.com/624446) - reenable once this test is fixed |
-TEST_F(ArcAppModelBuilderTest, DISABLED_RequestShortcutIcons) { |
+TEST_F(ArcAppModelBuilderTest, RequestShortcutIcons) { |
// Make sure we are on UI thread. |
ASSERT_TRUE(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); |
@@ -686,15 +702,12 @@ TEST_F(ArcAppModelBuilderTest, DISABLED_RequestShortcutIcons) { |
const float scale = ui::GetScaleForScaleFactor(scale_factor); |
const base::FilePath icon_path = |
prefs->GetIconPath(ArcAppTest::GetAppId(shortcut), scale_factor); |
- EXPECT_EQ(true, !base::PathExists(icon_path)); |
+ EXPECT_FALSE(base::PathExists(icon_path)); |
app_item->icon().GetRepresentation(scale); |
+ WaitForIconReady(prefs, ArcAppTest::GetAppId(shortcut), scale_factor); |
} |
- // Process pending tasks. |
- content::BrowserThread::GetBlockingPool()->FlushForTesting(); |
- base::RunLoop().RunUntilIdle(); |
- |
// At this moment we should receive all requests for icon loading. |
const size_t expected_size = scale_factors.size(); |
const ScopedVector<arc::FakeAppInstance::ShortcutIconRequest>& icon_requests = |
@@ -719,7 +732,7 @@ TEST_F(ArcAppModelBuilderTest, DISABLED_RequestShortcutIcons) { |
for (auto& scale_factor : scale_factors) { |
const base::FilePath icon_path = |
prefs->GetIconPath(ArcAppTest::GetAppId(shortcut), scale_factor); |
- EXPECT_EQ(true, base::PathExists(icon_path)); |
+ EXPECT_TRUE(base::PathExists(icon_path)); |
} |
} |
@@ -740,37 +753,26 @@ TEST_F(ArcAppModelBuilderTest, InstallIcon) { |
const float scale = ui::GetScaleForScaleFactor(scale_factor); |
const base::FilePath icon_path = prefs->GetIconPath(ArcAppTest::GetAppId(app), |
scale_factor); |
- EXPECT_EQ(true, !base::PathExists(icon_path)); |
+ EXPECT_FALSE(base::PathExists(icon_path)); |
const ArcAppItem* app_item = FindArcItem(ArcAppTest::GetAppId(app)); |
EXPECT_NE(nullptr, app_item); |
// This initiates async loading. |
app_item->icon().GetRepresentation(scale); |
- // Process pending tasks. |
- content::BrowserThread::GetBlockingPool()->FlushForTesting(); |
- base::RunLoop().RunUntilIdle(); |
- |
- // Validating decoded content does not fit well for unit tests. |
- ArcAppIcon::DisableSafeDecodingForTesting(); |
- |
// Now send generated icon for the app. |
std::string png_data; |
- EXPECT_EQ(true, app_instance()->GenerateAndSendIcon( |
- app, static_cast<arc::mojom::ScaleFactor>(scale_factor), |
- &png_data)); |
- |
- // Process pending tasks. |
- content::BrowserThread::GetBlockingPool()->FlushForTesting(); |
- base::RunLoop().RunUntilIdle(); |
+ EXPECT_TRUE(app_instance()->GenerateAndSendIcon( |
+ app, static_cast<arc::mojom::ScaleFactor>(scale_factor), &png_data)); |
+ WaitForIconReady(prefs, ArcAppTest::GetAppId(app), scale_factor); |
// Validate that icons are installed, have right content and icon is |
// refreshed for ARC app item. |
- EXPECT_EQ(true, base::PathExists(icon_path)); |
+ EXPECT_TRUE(base::PathExists(icon_path)); |
std::string icon_data; |
// Read the file from disk and compare with reference data. |
- EXPECT_EQ(true, base::ReadFileToString(icon_path, &icon_data)); |
+ EXPECT_TRUE(base::ReadFileToString(icon_path, &icon_data)); |
ASSERT_EQ(icon_data, png_data); |
} |
@@ -792,24 +794,29 @@ TEST_F(ArcAppModelBuilderTest, RemoveAppCleanUpFolder) { |
const ui::ScaleFactor scale_factor = ui::GetSupportedScaleFactors()[0]; |
// No app folder by default. |
- EXPECT_EQ(true, !base::PathExists(app_path)); |
+ base::RunLoop().RunUntilIdle(); |
+ EXPECT_FALSE(base::PathExists(app_path)); |
// Request icon, this will create app folder. |
prefs->RequestIcon(app_id, scale_factor); |
// Now send generated icon for the app. |
std::string png_data; |
- EXPECT_EQ(true, app_instance()->GenerateAndSendIcon( |
- app, static_cast<arc::mojom::ScaleFactor>(scale_factor), |
- &png_data)); |
- content::BrowserThread::GetBlockingPool()->FlushForTesting(); |
- base::RunLoop().RunUntilIdle(); |
- EXPECT_EQ(true, base::PathExists(app_path)); |
+ EXPECT_TRUE(app_instance()->GenerateAndSendIcon( |
+ app, static_cast<arc::mojom::ScaleFactor>(scale_factor), &png_data)); |
+ WaitForIconReady(prefs, app_id, scale_factor); |
+ EXPECT_TRUE(base::PathExists(app_path)); |
// Send empty app list. This will delete app and its folder. |
app_instance()->SendRefreshAppList(std::vector<arc::mojom::AppInfo>()); |
- content::BrowserThread::GetBlockingPool()->FlushForTesting(); |
- base::RunLoop().RunUntilIdle(); |
- EXPECT_EQ(true, !base::PathExists(app_path)); |
+ // This cannot be WaitForIconReady since it needs to wait until the icon is |
+ // removed, not added. |
+ // Process pending tasks. This performs multiple thread hops, so we need |
+ // to run it continuously until it is resolved. |
+ do { |
+ content::BrowserThread::GetBlockingPool()->FlushForTesting(); |
+ base::RunLoop().RunUntilIdle(); |
+ } while (base::PathExists(app_path)); |
+ EXPECT_FALSE(base::PathExists(app_path)); |
} |
TEST_F(ArcAppModelBuilderTest, LastLaunchTime) { |
@@ -856,13 +863,14 @@ TEST_F(ArcAppModelBuilderTest, LastLaunchTime) { |
ASSERT_GE(time_after, app_info->last_launch_time); |
} |
-TEST_F(ArcAppModelBuilderTest, IconLoader) { |
- // Validating decoded content does not fit well for unit tests. |
- ArcAppIcon::DisableSafeDecodingForTesting(); |
- |
+// TODO(crbug.com/628425) -- reenable once this test is less flaky. |
+TEST_F(ArcAppModelBuilderTest, DISABLED_IconLoader) { |
const arc::mojom::AppInfo& app = fake_apps()[0]; |
const std::string app_id = ArcAppTest::GetAppId(app); |
+ ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_.get()); |
+ ASSERT_NE(nullptr, prefs); |
+ |
bridge_service()->SetReady(); |
app_instance()->RefreshAppList(); |
app_instance()->SendRefreshAppList(std::vector<arc::mojom::AppInfo>( |
@@ -882,20 +890,17 @@ TEST_F(ArcAppModelBuilderTest, IconLoader) { |
const std::vector<ui::ScaleFactor>& scale_factors = |
ui::GetSupportedScaleFactors(); |
+ ArcAppItem* app_item = FindArcItem(app_id); |
for (auto& scale_factor : scale_factors) { |
std::string png_data; |
EXPECT_TRUE(app_instance()->GenerateAndSendIcon( |
app, static_cast<arc::mojom::ScaleFactor>(scale_factor), &png_data)); |
+ const float scale = ui::GetScaleForScaleFactor(scale_factor); |
+ // Force the icon to be loaded. |
+ app_item->icon().GetRepresentation(scale); |
+ WaitForIconReady(prefs, app_id, scale_factor); |
} |
- // Process pending tasks, this installs icons. |
- content::BrowserThread::GetBlockingPool()->FlushForTesting(); |
- base::RunLoop().RunUntilIdle(); |
- |
- // Allow one more circle to read and decode installed icons. |
- content::BrowserThread::GetBlockingPool()->FlushForTesting(); |
- base::RunLoop().RunUntilIdle(); |
- |
// Validate loaded image. |
EXPECT_EQ(1 + scale_factors.size(), delegate.update_image_cnt()); |
EXPECT_EQ(app_id, delegate.app_id()); |