Revert from M41 branch 2272: "Hidden windows should not keep Chrome alive."
This reverts commit c307178c2f20729fbdc6cadaf91501963cddb9f9.
Broke beta builder: http://master.chrome.corp.google.com:8011/builders/cros%20beta/builds/7554.
BUG=
Review URL: https://codereview.chromium.org/936533002
Cr-Commit-Position: refs/branch-heads/2272@{#306}
Cr-Branched-From: 827a380cfdb31aa54c8d56e63ce2c3fd8c3ba4d4-refs/heads/master@{#310958}
diff --git a/chrome/browser/apps/app_window_interactive_uitest.cc b/chrome/browser/apps/app_window_interactive_uitest.cc
index 30247d7..1212c206 100644
--- a/chrome/browser/apps/app_window_interactive_uitest.cc
+++ b/chrome/browser/apps/app_window_interactive_uitest.cc
@@ -3,9 +3,6 @@
// found in the LICENSE file.
#include "chrome/browser/apps/app_browsertest_util.h"
-#include "chrome/browser/lifetime/application_lifetime.h"
-#include "chrome/browser/ui/browser_iterator.h"
-#include "chrome/browser/ui/browser_window.h"
#include "chrome/test/base/interactive_test_utils.h"
#include "extensions/browser/app_window/native_app_window.h"
#include "extensions/test/extension_test_message_listener.h"
@@ -460,78 +457,3 @@
IN_PROC_BROWSER_TEST_F(AppWindowInteractiveTest, TestDrawAttention) {
ASSERT_TRUE(RunAppWindowInteractiveTest("testDrawAttention")) << message_;
}
-
-// Only Linux and Windows use keep-alive to determine when to shut down.
-#if defined(OS_LINUX) || defined(OS_WIN)
-
-// In general, hidden windows should not keep Chrome alive. The exception is
-// when windows are created hidden, we allow the app some time to show the
-// the window.
-class AppWindowHiddenKeepAliveTest : public extensions::PlatformAppBrowserTest {
- protected:
- AppWindowHiddenKeepAliveTest() {}
-
- private:
- DISALLOW_COPY_AND_ASSIGN(AppWindowHiddenKeepAliveTest);
-};
-
-// A window that becomes hidden should not keep Chrome alive.
-IN_PROC_BROWSER_TEST_F(AppWindowHiddenKeepAliveTest, ShownThenHidden) {
- LoadAndLaunchPlatformApp("minimal", "Launched");
- for (chrome::BrowserIterator it; !it.done(); it.Next())
- it->window()->Close();
-
- EXPECT_TRUE(chrome::WillKeepAlive());
- GetFirstAppWindow()->Hide();
- EXPECT_FALSE(chrome::WillKeepAlive());
-}
-
-// A window that is hidden but re-shown should still keep Chrome alive.
-IN_PROC_BROWSER_TEST_F(AppWindowHiddenKeepAliveTest, ShownThenHiddenThenShown) {
- LoadAndLaunchPlatformApp("minimal", "Launched");
- AppWindow* app_window = GetFirstAppWindow();
- app_window->Hide();
- app_window->Show(AppWindow::SHOW_ACTIVE);
-
- EXPECT_TRUE(chrome::WillKeepAlive());
- for (chrome::BrowserIterator it; !it.done(); it.Next())
- it->window()->Close();
- EXPECT_TRUE(chrome::WillKeepAlive());
- app_window->GetBaseWindow()->Close();
-}
-
-// A window that is created hidden and stays hidden should not keep Chrome
-// alive.
-IN_PROC_BROWSER_TEST_F(AppWindowHiddenKeepAliveTest, StaysHidden) {
- LoadAndLaunchPlatformApp("hidden", "Launched");
- AppWindow* app_window = GetFirstAppWindow();
- EXPECT_TRUE(app_window->is_hidden());
-
- for (chrome::BrowserIterator it; !it.done(); it.Next())
- it->window()->Close();
- // This will time out if the command above does not terminate Chrome.
- content::RunMessageLoop();
-}
-
-// A window that is created hidden but shown soon after should keep Chrome
-// alive.
-IN_PROC_BROWSER_TEST_F(AppWindowHiddenKeepAliveTest, HiddenThenShown) {
- ExtensionTestMessageListener launched_listener("Launched", true);
- LoadAndLaunchPlatformApp("hidden_then_shown", &launched_listener);
- AppWindow* app_window = GetFirstAppWindow();
- EXPECT_TRUE(app_window->is_hidden());
-
- // Close all browser windows.
- for (chrome::BrowserIterator it; !it.done(); it.Next())
- it->window()->Close();
-
- // The app window will show after 3 seconds.
- ExtensionTestMessageListener shown_listener("Shown", false);
- launched_listener.Reply("");
- EXPECT_TRUE(shown_listener.WaitUntilSatisfied());
- EXPECT_FALSE(app_window->is_hidden());
- EXPECT_TRUE(chrome::WillKeepAlive());
- app_window->GetBaseWindow()->Close();
-}
-
-#endif
diff --git a/chrome/browser/ui/apps/chrome_app_delegate.cc b/chrome/browser/ui/apps/chrome_app_delegate.cc
index 96ac0bf..4fbff22b 100644
--- a/chrome/browser/ui/apps/chrome_app_delegate.cc
+++ b/chrome/browser/ui/apps/chrome_app_delegate.cc
@@ -25,7 +25,6 @@
#include "chrome/common/extensions/chrome_extension_messages.h"
#include "components/ui/zoom/zoom_controller.h"
#include "content/public/browser/browser_context.h"
-#include "content/public/browser/browser_thread.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/render_view_host.h"
#include "content/public/browser/web_contents.h"
@@ -47,9 +46,6 @@
namespace {
-// Time to wait for an app window to show before allowing Chrome to quit.
-int kAppWindowFirstShowTimeoutSeconds = 10;
-
bool disable_external_open_for_testing_ = false;
// Opens a URL with Chromium (not external browser) with the right profile.
@@ -114,16 +110,6 @@
} // namespace
-// static
-void ChromeAppDelegate::RelinquishKeepAliveAfterTimeout(
- const base::WeakPtr<ChromeAppDelegate>& chrome_app_delegate) {
- // Resetting the ScopedKeepAlive may cause nested destruction of the
- // ChromeAppDelegate which also resets the ScopedKeepAlive. To avoid this,
- // move the ScopedKeepAlive out to here and let it fall out of scope.
- if (chrome_app_delegate.get() && chrome_app_delegate->is_hidden_)
- scoped_ptr<ScopedKeepAlive>(chrome_app_delegate->keep_alive_.Pass());
-}
-
class ChromeAppDelegate::NewWindowContentsDelegate
: public content::WebContentsDelegate {
public:
@@ -164,11 +150,8 @@
}
ChromeAppDelegate::ChromeAppDelegate(scoped_ptr<ScopedKeepAlive> keep_alive)
- : has_been_shown_(false),
- is_hidden_(true),
- keep_alive_(keep_alive.Pass()),
- new_window_contents_delegate_(new NewWindowContentsDelegate()),
- weak_factory_(this) {
+ : keep_alive_(keep_alive.Pass()),
+ new_window_contents_delegate_(new NewWindowContentsDelegate()) {
registrar_.Add(this,
chrome::NOTIFICATION_APP_TERMINATING,
content::NotificationService::AllSources());
@@ -302,28 +285,6 @@
terminating_callback_ = callback;
}
-void ChromeAppDelegate::OnHide() {
- is_hidden_ = true;
- if (has_been_shown_) {
- keep_alive_.reset();
- return;
- }
-
- // Hold on to the keep alive for some time to give the app a chance to show
- // the window.
- content::BrowserThread::PostDelayedTask(
- content::BrowserThread::UI, FROM_HERE,
- base::Bind(&ChromeAppDelegate::RelinquishKeepAliveAfterTimeout,
- weak_factory_.GetWeakPtr()),
- base::TimeDelta::FromSeconds(kAppWindowFirstShowTimeoutSeconds));
-}
-
-void ChromeAppDelegate::OnShow() {
- has_been_shown_ = true;
- is_hidden_ = false;
- keep_alive_.reset(new ScopedKeepAlive);
-}
-
void ChromeAppDelegate::Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) {
diff --git a/chrome/browser/ui/apps/chrome_app_delegate.h b/chrome/browser/ui/apps/chrome_app_delegate.h
index 8501e64..e08e613 100644
--- a/chrome/browser/ui/apps/chrome_app_delegate.h
+++ b/chrome/browser/ui/apps/chrome_app_delegate.h
@@ -7,7 +7,6 @@
#include "base/callback.h"
#include "base/memory/scoped_ptr.h"
-#include "base/memory/weak_ptr.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
#include "extensions/browser/app_window/app_delegate.h"
@@ -32,9 +31,6 @@
static void DisableExternalOpenForTesting();
private:
- static void RelinquishKeepAliveAfterTimeout(
- const base::WeakPtr<ChromeAppDelegate>& chrome_app_delegate);
-
class NewWindowContentsDelegate;
// extensions::AppDelegate:
@@ -70,21 +66,16 @@
bool blocked) override;
bool IsWebContentsVisible(content::WebContents* web_contents) override;
void SetTerminatingCallback(const base::Closure& callback) override;
- void OnHide() override;
- void OnShow() override;
// content::NotificationObserver:
void Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) override;
- bool has_been_shown_;
- bool is_hidden_;
scoped_ptr<ScopedKeepAlive> keep_alive_;
scoped_ptr<NewWindowContentsDelegate> new_window_contents_delegate_;
base::Closure terminating_callback_;
content::NotificationRegistrar registrar_;
- base::WeakPtrFactory<ChromeAppDelegate> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(ChromeAppDelegate);
};
diff --git a/chrome/test/data/extensions/platform_apps/hidden/test.js b/chrome/test/data/extensions/platform_apps/hidden/test.js
index 15a1b5e7..c110227 100644
--- a/chrome/test/data/extensions/platform_apps/hidden/test.js
+++ b/chrome/test/data/extensions/platform_apps/hidden/test.js
@@ -3,7 +3,7 @@
// found in the LICENSE file.
chrome.app.runtime.onLaunched.addListener(function() {
- chrome.app.window.create('empty.html', {
+ chrome.app.window.create('main.html', {
hidden: true,
}, function () {
chrome.test.sendMessage('Launched');
diff --git a/chrome/test/data/extensions/platform_apps/hidden_then_shown/empty.html b/chrome/test/data/extensions/platform_apps/hidden_then_shown/empty.html
deleted file mode 100644
index aabcd1b..0000000
--- a/chrome/test/data/extensions/platform_apps/hidden_then_shown/empty.html
+++ /dev/null
@@ -1 +0,0 @@
-<!-- This file intentionally left blank. -->
diff --git a/chrome/test/data/extensions/platform_apps/hidden_then_shown/manifest.json b/chrome/test/data/extensions/platform_apps/hidden_then_shown/manifest.json
deleted file mode 100644
index 59a8a94..0000000
--- a/chrome/test/data/extensions/platform_apps/hidden_then_shown/manifest.json
+++ /dev/null
@@ -1,10 +0,0 @@
-{
- "name": "Platform App Hidden Then Shown",
- "version": "1",
- "manifest_version": 2,
- "app": {
- "background": {
- "scripts": ["test.js"]
- }
- }
-}
diff --git a/chrome/test/data/extensions/platform_apps/hidden_then_shown/test.js b/chrome/test/data/extensions/platform_apps/hidden_then_shown/test.js
deleted file mode 100644
index d1a7ff13..0000000
--- a/chrome/test/data/extensions/platform_apps/hidden_then_shown/test.js
+++ /dev/null
@@ -1,16 +0,0 @@
-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-chrome.app.runtime.onLaunched.addListener(function() {
- chrome.app.window.create('empty.html', {
- hidden: true,
- }, function (win) {
- chrome.test.sendMessage('Launched', function () {
- setTimeout(function () {
- win.show();
- chrome.test.sendMessage('Shown');
- }, 3000);
- });
- });
-});
diff --git a/extensions/browser/app_window/app_delegate.h b/extensions/browser/app_window/app_delegate.h
index fd78d106..0e4477e 100644
--- a/extensions/browser/app_window/app_delegate.h
+++ b/extensions/browser/app_window/app_delegate.h
@@ -76,10 +76,6 @@
// |callback| will be called when the process is about to terminate.
virtual void SetTerminatingCallback(const base::Closure& callback) = 0;
-
- // Called when the app is hidden or shown.
- virtual void OnHide() = 0;
- virtual void OnShow() = 0;
};
} // namespace extensions
diff --git a/extensions/browser/app_window/app_window.cc b/extensions/browser/app_window/app_window.cc
index 83bd201..7fdaeffb 100644
--- a/extensions/browser/app_window/app_window.cc
+++ b/extensions/browser/app_window/app_window.cc
@@ -658,7 +658,6 @@
}
void AppWindow::Show(ShowType show_type) {
- app_delegate_->OnShow();
bool was_hidden = is_hidden_ || !has_been_shown_;
is_hidden_ = false;
@@ -695,7 +694,6 @@
show_on_first_paint_ = false;
GetBaseWindow()->Hide();
AppWindowRegistry::Get(browser_context_)->AppWindowHidden(this);
- app_delegate_->OnHide();
}
void AppWindow::SetAlwaysOnTop(bool always_on_top) {
diff --git a/extensions/shell/browser/shell_app_delegate.h b/extensions/shell/browser/shell_app_delegate.h
index b458155..8c9be2e 100644
--- a/extensions/shell/browser/shell_app_delegate.h
+++ b/extensions/shell/browser/shell_app_delegate.h
@@ -50,8 +50,6 @@
bool blocked) override;
bool IsWebContentsVisible(content::WebContents* web_contents) override;
void SetTerminatingCallback(const base::Closure& callback) override;
- void OnHide() override {}
- void OnShow() override {}
// content::WebContentsObserver:
void RenderViewCreated(content::RenderViewHost* render_view_host) override;