Add an arbitrary delay before unloading lazy background pages.

BUG=119619
TEST=Make sure that lazy page process stays open until 10 seconds after the last of a series of events it's listening for.

Review URL: https://chromiumcodereview.appspot.com/10272019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@134853 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/extension_process_manager.cc

@@ -5,6 +5,9 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
+#include "base/message_loop.h"
+#include "base/string_number_conversions.h"
+#include "base/time.h"
 #include "chrome/browser/extensions/extension_event_router.h"
 #include "chrome/browser/extensions/extension_process_manager.h"
 #include "chrome/browser/extensions/extension_host.h"
@@ -125,7 +128,8 @@ ExtensionProcessManager* ExtensionProcessManager::Create(Profile* profile) {
 }
 
 ExtensionProcessManager::ExtensionProcessManager(Profile* profile)
-    : site_instance_(SiteInstance::Create(profile)) {
+  : site_instance_(SiteInstance::Create(profile)),
+    weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
   Profile* original_profile = profile->GetOriginalProfile();
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSIONS_READY,
                  content::Source<Profile>(original_profile));
@@ -145,6 +149,20 @@ ExtensionProcessManager::ExtensionProcessManager(Profile* profile)
                  content::Source<content::BrowserContext>(profile));
   registrar_.Add(this, content::NOTIFICATION_DEVTOOLS_WINDOW_CLOSING,
                  content::Source<content::BrowserContext>(profile));
+
+  event_page_idle_time_ = base::TimeDelta::FromSeconds(10);
+  unsigned idle_time_sec = 0;
+  if (base::StringToUint(CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kEventPageIdleTime), &idle_time_sec)) {
+    event_page_idle_time_ = base::TimeDelta::FromSeconds(idle_time_sec);
+  }
+  event_page_unloading_time_ = base::TimeDelta::FromSeconds(5);
+  unsigned unloading_time_sec = 0;
+  if (base::StringToUint(CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kEventPageUnloadingTime), &unloading_time_sec)) {
+    event_page_unloading_time_ = base::TimeDelta::FromSeconds(
+        unloading_time_sec);
+  }
 }
 
 ExtensionProcessManager::~ExtensionProcessManager() {
@@ -414,22 +432,31 @@ int ExtensionProcessManager::DecrementLazyKeepaliveCount(
 
   int& count = background_page_data_[extension->id()].lazy_keepalive_count;
   DCHECK_GT(count, 0);
-  if (--count == 0)
-    OnLazyBackgroundPageIdle(extension->id());
+  if (--count == 0) {
+    MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&ExtensionProcessManager::OnLazyBackgroundPageIdle,
+                   weak_ptr_factory_.GetWeakPtr(), extension->id(),
+                   ++background_page_data_[extension->id()].close_sequence_id),
+        event_page_idle_time_);
+  }
 
   return count;
 }
 
 void ExtensionProcessManager::OnLazyBackgroundPageIdle(
-    const std::string& extension_id) {
+    const std::string& extension_id, int sequence_id) {
   ExtensionHost* host = GetBackgroundHostForExtension(extension_id);
-  if (host && !background_page_data_[extension_id].is_closing) {
+  if (host && !background_page_data_[extension_id].is_closing &&
+      sequence_id == background_page_data_[extension_id].close_sequence_id) {
     // Tell the renderer we are about to close. This is a simple ping that the
     // renderer will respond to. The purpose is to control sequencing: if the
     // extension remains idle until the renderer responds with an ACK, then we
-    // know that the extension process is ready to shut down.
+    // know that the extension process is ready to shut down. If our
+    // close_sequence_id has already changed, then we would ignore the
+    // ShouldUnloadAck, so we don't send the ping.
     host->render_view_host()->Send(new ExtensionMsg_ShouldUnload(
-        extension_id, ++background_page_data_[extension_id].close_sequence_id));
+        extension_id, sequence_id));
   }
 }
 
@@ -449,10 +476,21 @@ void ExtensionProcessManager::OnShouldUnloadAck(
   if (host &&
       sequence_id == background_page_data_[extension_id].close_sequence_id) {
     background_page_data_[extension_id].is_closing = true;
-    host->render_view_host()->Send(new ExtensionMsg_Unload(extension_id));
+    MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&ExtensionProcessManager::CloseLazyBackgroundPageNow,
+                   weak_ptr_factory_.GetWeakPtr(), extension_id),
+        event_page_unloading_time_);
   }
 }
 
+void ExtensionProcessManager::CloseLazyBackgroundPageNow(
+    const std::string& extension_id) {
+  ExtensionHost* host = GetBackgroundHostForExtension(extension_id);
+  if (host)
+    host->render_view_host()->Send(new ExtensionMsg_Unload(extension_id));
+}
+
 void ExtensionProcessManager::OnUnloadAck(const std::string& extension_id) {
   ExtensionHost* host = GetBackgroundHostForExtension(extension_id);
   if (host)

chrome/browser/extensions/extension_process_manager.h

@@ -12,6 +12,8 @@
 
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "base/time.h"
 #include "content/public/common/view_type.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
@@ -182,8 +184,10 @@ class ExtensionProcessManager : public content::NotificationObserver {
 
   // These are called when the extension transitions between idle and active.
   // They control the process of closing the background page when idle.
-  void OnLazyBackgroundPageIdle(const std::string& extension_id);
+  void OnLazyBackgroundPageIdle(const std::string& extension_id,
+                                int sequence_id);
   void OnLazyBackgroundPageActive(const std::string& extension_id);
+  void CloseLazyBackgroundPageNow(const std::string& extension_id);
 
   // Updates a potentially-registered RenderViewHost once it has been
   // associated with a WebContents. This allows us to gather information that
@@ -192,6 +196,16 @@ class ExtensionProcessManager : public content::NotificationObserver {
 
   BackgroundPageDataMap background_page_data_;
 
+  // The time to delay between an extension becoming idle and
+  // sending a ShouldUnload message; read from command-line switch.
+  base::TimeDelta event_page_idle_time_;
+
+  // The time to delay between sending a ShouldUnload message and
+  // sending a Unload message; read from command-line switch.
+  base::TimeDelta event_page_unloading_time_;
+
+  base::WeakPtrFactory<ExtensionProcessManager> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionProcessManager);
 };
 

chrome/browser/extensions/lazy_background_page_apitest.cc

@@ -103,6 +103,9 @@ public:
   void SetUpCommandLine(CommandLine* command_line) {
     ExtensionApiTest::SetUpCommandLine(command_line);
     command_line->AppendSwitch(switches::kEnableExperimentalExtensionApis);
+    // Set shorter delays to prevent test timeouts.
+    command_line->AppendSwitchASCII(switches::kEventPageIdleTime, "1");
+    command_line->AppendSwitchASCII(switches::kEventPageUnloadingTime, "1");
   }
 
   // Loads the extension, which temporarily starts the lazy background page

chrome/common/chrome_switches.cc

@@ -645,6 +645,14 @@ const char kExperimentalSpellcheckerFeatures[] =
 // numbers.
 const char kExplicitlyAllowedPorts[]        = "explicitly-allowed-ports";
 
+// The time in seconds that an extension event page can be idle before it
+// is shut down.
+const char kEventPageIdleTime[]             = "event-page-idle-time";
+
+// The time in seconds that an extension event page has between being notified
+// of its impending unload and that unload happening.
+const char kEventPageUnloadingTime[]        = "event-page-unloading-time";
+
 // Marks a renderer as extension process.
 const char kExtensionProcess[]              = "extension-process";
 

chrome/common/chrome_switches.h

@@ -18,7 +18,8 @@ namespace switches {
 // -----------------------------------------------------------------------------
 // Can't find the switch you are looking for? Try looking in
 // ui/gfx/gl/gl_switches.cc or base/base_switches.cc or
-// content/common/content_switches.cc or media/base/media_switches.cc instead.
+// content/public/common/content_switches.cc or media/base/media_switches.cc
+// instead.
 // -----------------------------------------------------------------------------
 
 // All switches in alphabetical order. The switches should be documented
@@ -178,6 +179,8 @@ extern const char kEnableWatchdog[];
 extern const char kEnableWebsiteSettings[];
 extern const char kEnableWebSocketOverSpdy[];
 extern const char kEnableWebStoreLink[];
+extern const char kEventPageIdleTime[];
+extern const char kEventPageUnloadingTime[];
 extern const char kExperimentalSpellcheckerFeatures[];
 extern const char kExplicitlyAllowedPorts[];
 extern const char kExtensionProcess[];