[go: up one dir, main page]

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;