Skip to content

Commit

Permalink
Revert "Extensions: Navigate to default chrome page on extension unload"
Browse files Browse the repository at this point in the history
This reverts commit 4d546d6.

Reason for revert: this patch caused a bug where we leave extraneous
windows around when coming out of sleep.

TBR=sky@chromium.org (c/b/ui/browser - clean revert)

Bug: 818351
Change-Id: I3cf6d60a4588b36619dc4e461c3b8cf29722d6cd
Reviewed-on: https://chromium-review.googlesource.com/947473
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540720}
  • Loading branch information
rdcronin authored and Commit Bot committed Mar 3, 2018
1 parent 6c63256 commit f16ef54
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 128 deletions.
70 changes: 0 additions & 70 deletions chrome/browser/extensions/extension_web_ui_browsertest.cc

This file was deleted.

16 changes: 1 addition & 15 deletions chrome/browser/ui/browser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2123,21 +2123,7 @@ void Browser::OnExtensionUnloaded(content::BrowserContext* browser_context,
web_contents->GetURL().host_piece() == extension->id()) ||
(extensions::TabHelper::FromWebContents(web_contents)
->extension_app() == extension)) {
if (tab_strip_model_->count() > 1) {
tab_strip_model_->CloseWebContentsAt(i, TabStripModel::CLOSE_NONE);
} else {
// If there is only 1 tab remaining, do not close it and instead
// navigate to the default NTP. Note that if there is an installed
// extension that overrides the NTP page, that extension's content
// will override the NTP contents.
GURL url(chrome::kChromeUINewTabURL);
web_contents->GetController().LoadURL(
url,
content::Referrer::SanitizeForRequest(
url,
content::Referrer(url, blink::kWebReferrerPolicyDefault)),
ui::PAGE_TRANSITION_RELOAD, std::string());
}
tab_strip_model_->CloseWebContentsAt(i, TabStripModel::CLOSE_NONE);
} else {
chrome::UnmuteIfMutedByExtension(web_contents, extension->id());
}
Expand Down
45 changes: 3 additions & 42 deletions chrome/browser/ui/browser_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
#include "chrome/browser/devtools/devtools_window_testing.h"
#include "chrome/browser/extensions/extension_browsertest.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_tab_util.h"
#include "chrome/browser/extensions/extension_util.h"
#include "chrome/browser/extensions/tab_helper.h"
#include "chrome/browser/first_run/first_run.h"
Expand Down Expand Up @@ -106,7 +105,6 @@
#include "content/public/test/test_navigation_observer.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/extension_system.h"
#include "extensions/browser/test_extension_registry_observer.h"
#include "extensions/browser/uninstall_reason.h"
#include "extensions/common/constants.h"
#include "extensions/common/extension.h"
Expand Down Expand Up @@ -1109,43 +1107,6 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, MAYBE_TabClosingWhenRemovingExtension) {
ASSERT_EQ(1, browser()->tab_strip_model()->count());
}

// Tests that when an extension is unloaded, if only one tab is opened
// containing extenions-related content, then the tab is kept open and is
// directed to the default NTP.
IN_PROC_BROWSER_TEST_F(BrowserTest, NavigateToDefaultNTPPageOnExtensionUnload) {
ASSERT_TRUE(embedded_test_server()->Start());
TabStripModel* tab_strip_model = browser()->tab_strip_model();

const Extension* extension =
LoadExtension(test_data_dir_.AppendASCII("options_page/"));
ASSERT_TRUE(extension);

GURL extension_url = extension->GetResourceURL("options.html");
ui_test_utils::NavigateToURL(browser(), extension_url);
content::WaitForLoadStop(tab_strip_model->GetActiveWebContents());

ASSERT_EQ(1, browser()->tab_strip_model()->count());
EXPECT_EQ(
extension_url.spec(),
tab_strip_model->GetActiveWebContents()->GetLastCommittedURL().spec());

// Uninstall the extension.
ExtensionService* service =
extensions::ExtensionSystem::Get(browser()->profile())
->extension_service();
extensions::ExtensionRegistry* registry =
extensions::ExtensionRegistry::Get(browser()->profile());
extensions::TestExtensionRegistryObserver registry_observer(registry);
service->UnloadExtension(extension->id(),
extensions::UnloadedExtensionReason::UNINSTALL);
registry_observer.WaitForExtensionUnloaded();
content::WaitForLoadStop(tab_strip_model->GetActiveWebContents());

// There should only be one tab now, with the NTP loaded.
ASSERT_EQ(1, tab_strip_model->count());
EXPECT_TRUE(search::IsInstantNTP(tab_strip_model->GetActiveWebContents()));
}

// Open with --app-id=<id>, and see that an application tab opens by default.
IN_PROC_BROWSER_TEST_F(BrowserTest, AppIdSwitch) {
ASSERT_TRUE(embedded_test_server()->Start());
Expand Down Expand Up @@ -2407,7 +2368,7 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, GetSizeForNewRenderView) {
web_contents->GetContainerBounds().size();
RenderViewSizeObserver observer(web_contents, browser()->window());

// Navigate to a non-NTP, without resizing WebContentsView.
// Navigate to a non-NTP page, without resizing WebContentsView.
ui_test_utils::NavigateToURL(browser(),
embedded_test_server()->GetURL("/title1.html"));
ASSERT_EQ(BookmarkBar::HIDDEN, browser()->bookmark_bar_state());
Expand Down Expand Up @@ -2445,7 +2406,7 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, GetSizeForNewRenderView) {
EXPECT_EQ(wcv_commit_size0, web_contents->GetContainerBounds().size());
#endif

// Navigate to another non-NTP, without resizing WebContentsView.
// Navigate to another non-NTP page, without resizing WebContentsView.
ui_test_utils::NavigateToURL(browser(),
https_test_server.GetURL("/title2.html"));
ASSERT_EQ(BookmarkBar::HIDDEN, browser()->bookmark_bar_state());
Expand All @@ -2461,7 +2422,7 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, GetSizeForNewRenderView) {
web_contents->GetRenderWidgetHostView()->GetViewBounds().size());
EXPECT_EQ(wcv_commit_size1, web_contents->GetContainerBounds().size());

// Navigate from NTP to a non-NTP, resizing WebContentsView while
// Navigate from NTP to a non-NTP page, resizing WebContentsView while
// navigation entry is pending.
ui_test_utils::NavigateToURL(browser(), GURL("chrome://newtab"));
gfx::Size wcv_resize_insets(1, 1);
Expand Down
1 change: 0 additions & 1 deletion chrome/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1264,7 +1264,6 @@ test("browser_tests") {
"../browser/extensions/extension_unload_browsertest.cc",
"../browser/extensions/extension_url_rewrite_browsertest.cc",
"../browser/extensions/extension_view_host_factory_browsertest.cc",
"../browser/extensions/extension_web_ui_browsertest.cc",
"../browser/extensions/extension_websocket_apitest.cc",
"../browser/extensions/extension_webui_apitest.cc",
"../browser/extensions/extension_with_management_policy_apitest.cc",
Expand Down

0 comments on commit f16ef54

Please sign in to comment.