Skip to content

Commit

Permalink
[Extensions] Remove NOTIFICATION_EXTENSION_HOST_DESTROYED from //apps
Browse files Browse the repository at this point in the history
Remove the test usages of NOTIFICATION_EXTENSION_HOST_DESTROYED from
//apps and //chrome/browser/apps. These used a
content::WindowedNotificationObserver to wait for the notification to
be fired; this can be easily replaced with
ExtensionHostTestHelper::WaitForExtensionHostDestroyed().

Bug: 1174738
Change-Id: Ia2f5d2af06e9dad20a20773102bac093087e7008
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3171916
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Auto-Submit: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/main@{#923666}
  • Loading branch information
rdcronin authored and Chromium LUCI CQ committed Sep 22, 2021
1 parent 518519a commit 6823036
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 27 deletions.
26 changes: 7 additions & 19 deletions apps/app_restore_service_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,11 @@
#include "base/threading/thread_restrictions.h"
#include "chrome/browser/apps/platform_apps/app_browsertest_util.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/notification_service.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/test_utils.h"
#include "extensions/browser/api/file_system/file_system_api.h"
#include "extensions/browser/api/file_system/saved_file_entry.h"
#include "extensions/browser/extension_host_test_helper.h"
#include "extensions/browser/extension_prefs.h"
#include "extensions/browser/notification_types.h"
#include "extensions/common/extension.h"
#include "extensions/test/extension_test_message_listener.h"

Expand All @@ -32,10 +30,7 @@ namespace apps {

// Tests that a running app is recorded in the preferences as such.
IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, RunningAppsAreRecorded) {
content::WindowedNotificationObserver extension_suspended(
extensions::NOTIFICATION_EXTENSION_HOST_DESTROYED,
content::NotificationService::AllSources());

extensions::ExtensionHostTestHelper host_helper(profile());
const Extension* extension = LoadExtension(
test_data_dir_.AppendASCII("platform_apps/restart_test"));
ASSERT_TRUE(extension);
Expand All @@ -45,7 +40,7 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, RunningAppsAreRecorded) {
ASSERT_TRUE(extension_prefs->IsExtensionRunning(extension->id()));

// Wait for the extension to get suspended.
extension_suspended.Wait();
host_helper.WaitForExtensionHostDestroyed();

// App isn't running because it got suspended.
ASSERT_FALSE(extension_prefs->IsExtensionRunning(extension->id()));
Expand Down Expand Up @@ -114,10 +109,6 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, ActiveAppsAreRecorded) {
}

IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, FileAccessIsSavedToPrefs) {
content::WindowedNotificationObserver extension_suspended(
extensions::NOTIFICATION_EXTENSION_HOST_DESTROYED,
content::NotificationService::AllSources());

base::ScopedAllowBlockingForTesting allow_blocking;
base::ScopedTempDir temp_directory;
ASSERT_TRUE(temp_directory.CreateUniqueTempDir());
Expand All @@ -130,6 +121,7 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, FileAccessIsSavedToPrefs) {
FileSystemChooseEntryFunction::RegisterTempExternalFileSystemForTest(
"temp", temp_directory.GetPath());

extensions::ExtensionHostTestHelper host_helper(profile());
const Extension* extension = LoadAndLaunchPlatformApp(
"file_access_saved_to_prefs_test", "fileWritten");
ASSERT_TRUE(extension);
Expand All @@ -141,7 +133,7 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, FileAccessIsSavedToPrefs) {
// One for the read-only file entry and one for the writable file entry.
ASSERT_EQ(2u, file_entries.size());

extension_suspended.Wait();
host_helper.WaitForExtensionHostDestroyed();
file_entries = saved_files_service->GetAllFileEntries(extension->id());
// File entries should be cleared when the extension is suspended.
ASSERT_TRUE(file_entries.empty());
Expand All @@ -155,10 +147,6 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, FileAccessIsSavedToPrefs) {
#endif

IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, MAYBE_FileAccessIsRestored) {
content::WindowedNotificationObserver extension_suspended(
extensions::NOTIFICATION_EXTENSION_HOST_DESTROYED,
content::NotificationService::AllSources());

base::ScopedAllowBlockingForTesting allow_blocking;
base::ScopedTempDir temp_directory;
ASSERT_TRUE(temp_directory.CreateUniqueTempDir());
Expand All @@ -171,9 +159,9 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, MAYBE_FileAccessIsRestored) {
FileSystemChooseEntryFunction::RegisterTempExternalFileSystemForTest(
"temp", temp_directory.GetPath());

extensions::ExtensionHostTestHelper host_helper(profile());
ExtensionTestMessageListener access_ok_listener(
"restartedFileAccessOK", false);

const Extension* extension =
LoadAndLaunchPlatformApp("file_access_restored_test", "fileWritten");
ASSERT_TRUE(extension);
Expand All @@ -183,7 +171,7 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, MAYBE_FileAccessIsRestored) {
SavedFilesService* saved_files_service = SavedFilesService::Get(profile());
std::vector<SavedFileEntry> file_entries =
saved_files_service->GetAllFileEntries(extension->id());
extension_suspended.Wait();
host_helper.WaitForExtensionHostDestroyed();

// Simulate a restart by populating the preferences as if the browser didn't
// get time to clean itself up.
Expand Down
11 changes: 3 additions & 8 deletions chrome/browser/apps/platform_apps/event_page_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
// found in the LICENSE file.

#include "chrome/browser/apps/platform_apps/app_browsertest_util.h"
#include "content/public/browser/notification_service.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/test_utils.h"
#include "extensions/browser/notification_types.h"
#include "extensions/browser/extension_host_test_helper.h"
#include "extensions/test/extension_test_message_listener.h"

using extensions::Extension;
Expand All @@ -21,18 +19,15 @@ class AppEventPageTest : public PlatformAppBrowserTest {
const Extension* extension = LoadAndLaunchPlatformApp(app_path, "launched");
ASSERT_TRUE(extension);

content::WindowedNotificationObserver event_page_suspended(
extensions::NOTIFICATION_EXTENSION_HOST_DESTROYED,
content::NotificationService::AllSources());

extensions::ExtensionHostTestHelper host_helper(profile(), extension->id());
// Close the app window.
EXPECT_EQ(1U, GetAppWindowCount());
extensions::AppWindow* app_window = GetFirstAppWindow();
ASSERT_TRUE(app_window);
CloseAppWindow(app_window);

// Verify that the event page is destroyed.
event_page_suspended.Wait();
host_helper.WaitForExtensionHostDestroyed();
}
};

Expand Down

0 comments on commit 6823036

Please sign in to comment.