Commit 00eb4b9c authored by Alan Cutter's avatar Alan Cutter Committed by Commit Bot

Fix incorrect PWA window/tab closing on uninstallation

This CL ensures the TabHelper of a PWA WebContents is updated to indicate
whether it has it's own "extension" window or not to ensure it gets
cleaned up appropriately on uninstall.

Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=354491&signed_aid=mELqu3AB-QBQpk1qojP7qg==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=354490&signed_aid=4RqFYkoMkmsndSMnrq5inA==&inline=1

Bug: 877009
Change-Id: I81e7888260a40df68f5e9a5443ea2a815b8a686f
Reviewed-on: https://chromium-review.googlesource.com/1186388
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: default avatarMatt Giuca <mgiuca@chromium.org>
Reviewed-by: default avatarMichael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585732}
parent cdcf168c
...@@ -1165,15 +1165,17 @@ IN_PROC_BROWSER_TEST_F(ShelfAppBrowserTestNoDefaultBrowser, ...@@ -1165,15 +1165,17 @@ IN_PROC_BROWSER_TEST_F(ShelfAppBrowserTestNoDefaultBrowser,
EXPECT_EQ(0u, items); EXPECT_EQ(0u, items);
EXPECT_EQ(0u, running_browser); EXPECT_EQ(0u, running_browser);
LoadAndLaunchExtension("app1", extensions::LAUNCH_CONTAINER_WINDOW, const Extension* extension =
WindowOpenDisposition::NEW_WINDOW); LoadAndLaunchExtension("app1", extensions::LAUNCH_CONTAINER_WINDOW,
WindowOpenDisposition::NEW_WINDOW);
// No new browser should get detected, even though one more is running. // No new browser should get detected, even though one more is running.
EXPECT_EQ(0u, NumberOfDetectedLauncherBrowsers(false)); EXPECT_EQ(0u, NumberOfDetectedLauncherBrowsers(false));
EXPECT_EQ(++running_browser, chrome::GetTotalBrowserCount()); EXPECT_EQ(++running_browser, chrome::GetTotalBrowserCount());
LoadAndLaunchExtension("app1", extensions::LAUNCH_CONTAINER_TAB, OpenApplication(AppLaunchParams(
WindowOpenDisposition::NEW_WINDOW); profile(), extension, extensions::LAUNCH_CONTAINER_TAB,
WindowOpenDisposition::NEW_WINDOW, extensions::SOURCE_TEST));
// A new browser should get detected and one more should be running. // A new browser should get detected and one more should be running.
EXPECT_EQ(NumberOfDetectedLauncherBrowsers(false), 1u); EXPECT_EQ(NumberOfDetectedLauncherBrowsers(false), 1u);
......
...@@ -1393,10 +1393,10 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, OpenAppWindowLikeNtp) { ...@@ -1393,10 +1393,10 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, OpenAppWindowLikeNtp) {
WindowOpenDisposition::NEW_WINDOW, extensions::SOURCE_TEST)); WindowOpenDisposition::NEW_WINDOW, extensions::SOURCE_TEST));
ASSERT_TRUE(app_window); ASSERT_TRUE(app_window);
// Apps launched in a window from the NTP have an extensions tab helper but // Apps launched in a window from the NTP have an extensions tab helper with
// do not have extension_app set in it. // extension_app set.
ASSERT_TRUE(extensions::TabHelper::FromWebContents(app_window)); ASSERT_TRUE(extensions::TabHelper::FromWebContents(app_window));
EXPECT_FALSE( EXPECT_TRUE(
extensions::TabHelper::FromWebContents(app_window)->extension_app()); extensions::TabHelper::FromWebContents(app_window)->extension_app());
EXPECT_EQ(extensions::AppLaunchInfo::GetFullLaunchURL(extension_app), EXPECT_EQ(extensions::AppLaunchInfo::GetFullLaunchURL(extension_app),
app_window->GetURL()); app_window->GetURL());
......
...@@ -1226,21 +1226,23 @@ void CopyURL(Browser* browser) { ...@@ -1226,21 +1226,23 @@ void CopyURL(Browser* browser) {
.spec())); .spec()));
} }
void OpenInChrome(Browser* browser) { Browser* OpenInChrome(Browser* hosted_app_browser) {
DCHECK(hosted_app_browser->hosted_app_controller());
// Find a non-incognito browser. // Find a non-incognito browser.
Browser* target_browser = Browser* target_browser =
chrome::FindTabbedBrowser(browser->profile(), false); chrome::FindTabbedBrowser(hosted_app_browser->profile(), false);
if (!target_browser) { if (!target_browser) {
target_browser = target_browser =
new Browser(Browser::CreateParams(browser->profile(), true)); new Browser(Browser::CreateParams(hosted_app_browser->profile(), true));
} }
TabStripModel* source_tabstrip = browser->tab_strip_model(); TabStripModel* source_tabstrip = hosted_app_browser->tab_strip_model();
target_browser->tab_strip_model()->AppendWebContents( target_browser->tab_strip_model()->AppendWebContents(
source_tabstrip->DetachWebContentsAt(source_tabstrip->active_index()), source_tabstrip->DetachWebContentsAt(source_tabstrip->active_index()),
true); true);
target_browser->window()->Show(); target_browser->window()->Show();
return target_browser;
} }
bool CanViewSource(const Browser* browser) { bool CanViewSource(const Browser* browser) {
......
...@@ -146,7 +146,9 @@ void ToggleFullscreenMode(Browser* browser); ...@@ -146,7 +146,9 @@ void ToggleFullscreenMode(Browser* browser);
void ClearCache(Browser* browser); void ClearCache(Browser* browser);
bool IsDebuggerAttachedToCurrentTab(Browser* browser); bool IsDebuggerAttachedToCurrentTab(Browser* browser);
void CopyURL(Browser* browser); void CopyURL(Browser* browser);
void OpenInChrome(Browser* browser); // Moves the WebContents of a hosted app Browser to a tabbed Browser. Returns
// the tabbed Browser.
Browser* OpenInChrome(Browser* hosted_app_browser);
bool CanViewSource(const Browser* browser); bool CanViewSource(const Browser* browser);
#if defined(OS_WIN) || (defined(OS_LINUX) && !defined(OS_CHROMEOS)) #if defined(OS_WIN) || (defined(OS_LINUX) && !defined(OS_CHROMEOS))
void ToggleConfirmToQuitOption(Browser* browser); void ToggleConfirmToQuitOption(Browser* browser);
......
...@@ -126,6 +126,9 @@ void HostedAppBrowserController::SetAppPrefsForWebContents( ...@@ -126,6 +126,9 @@ void HostedAppBrowserController::SetAppPrefsForWebContents(
if (!controller) if (!controller)
return; return;
extensions::TabHelper::FromWebContents(web_contents)
->SetExtensionApp(controller->GetExtension());
web_contents->NotifyPreferencesChanged(); web_contents->NotifyPreferencesChanged();
} }
...@@ -300,6 +303,8 @@ void HostedAppBrowserController::TabDetachedAt(content::WebContents* contents, ...@@ -300,6 +303,8 @@ void HostedAppBrowserController::TabDetachedAt(content::WebContents* contents,
contents->GetMutableRendererPrefs()->can_accept_load_drops = true; contents->GetMutableRendererPrefs()->can_accept_load_drops = true;
rvh->SyncRendererPrefs(); rvh->SyncRendererPrefs();
extensions::TabHelper::FromWebContents(contents)->SetExtensionApp(nullptr);
contents->NotifyPreferencesChanged(); contents->NotifyPreferencesChanged();
} }
......
...@@ -207,6 +207,14 @@ bool TryToLoadImage(const content::ToRenderFrameHost& adapter, ...@@ -207,6 +207,14 @@ bool TryToLoadImage(const content::ToRenderFrameHost& adapter,
return image_loaded; return image_loaded;
} }
bool IsBrowserOpen(const Browser* test_browser) {
for (Browser* browser : *BrowserList::GetInstance()) {
if (browser == test_browser)
return true;
}
return false;
}
} // namespace } // namespace
class TestAppBannerManagerDesktop : public banners::AppBannerManagerDesktop { class TestAppBannerManagerDesktop : public banners::AppBannerManagerDesktop {
...@@ -1145,8 +1153,40 @@ IN_PROC_BROWSER_TEST_P(HostedAppPWAOnlyTest, ...@@ -1145,8 +1153,40 @@ IN_PROC_BROWSER_TEST_P(HostedAppPWAOnlyTest,
IN_PROC_BROWSER_TEST_P(HostedAppPWAOnlyTest, UninstallPwaWithWindowOpened) { IN_PROC_BROWSER_TEST_P(HostedAppPWAOnlyTest, UninstallPwaWithWindowOpened) {
ASSERT_TRUE(https_server()->Start()); ASSERT_TRUE(https_server()->Start());
ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(embedded_test_server()->Start());
InstallMixedContentPWA(); InstallSecurePWA();
EXPECT_TRUE(IsBrowserOpen(app_browser_));
UninstallExtension(app_->id());
base::RunLoop().RunUntilIdle();
EXPECT_FALSE(IsBrowserOpen(app_browser_));
}
// PWAs moved to tabbed browsers should not get closed when uninstalled.
IN_PROC_BROWSER_TEST_P(HostedAppPWAOnlyTest, UninstallPwaWithWindowMovedToTab) {
ASSERT_TRUE(https_server()->Start());
ASSERT_TRUE(embedded_test_server()->Start());
InstallSecurePWA();
EXPECT_TRUE(IsBrowserOpen(app_browser_));
Browser* tabbed_browser = chrome::OpenInChrome(app_browser_);
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(IsBrowserOpen(tabbed_browser));
EXPECT_EQ(tabbed_browser, browser());
EXPECT_FALSE(IsBrowserOpen(app_browser_));
UninstallExtension(app_->id()); UninstallExtension(app_->id());
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(IsBrowserOpen(tabbed_browser));
EXPECT_EQ(tabbed_browser->tab_strip_model()
->GetActiveWebContents()
->GetLastCommittedURL(),
GetSecureAppURL());
} }
IN_PROC_BROWSER_TEST_P(HostedAppTest, IN_PROC_BROWSER_TEST_P(HostedAppTest,
......
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