Clear testing callbacks in AppBackgrounPageNaclTest reentrantly.

Addresses a flaky ASAN failure where I believe callbacks from ImpulseCallbackCounter with an unretained reference were being held too long by ProcessManager.

This clears the callbacks immediately when the impulse count is reached, and does not call the quit closure repeatedly.

BUG=332440

Review URL: https://codereview.chromium.org/129243003

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

chrome/browser/extensions/app_background_page_apitest.cc

@@ -156,9 +156,11 @@ class AppBackgroundPageNaClTest : public AppBackgroundPageApiTest {
 // that will match a specified goal and can be waited on.
 class ImpulseCallbackCounter {
  public:
-  explicit ImpulseCallbackCounter(const std::string& extension_id)
+  explicit ImpulseCallbackCounter(extensions::ProcessManager* manager,
+                                  const std::string& extension_id)
       : observed_(0),
         goal_(0),
+        manager_(manager),
         extension_id_(extension_id) {
   }
 
@@ -183,6 +185,11 @@ class ImpulseCallbackCounter {
       const std::string& extension_id_from_manager) {
     if (extension_id_from_test == extension_id_from_manager) {
       if (++observed_ >= goal_) {
+        // Clear callback to free reference to message loop.
+        manager_->SetKeepaliveImpulseCallbackForTesting(
+            extensions::ProcessManager::ImpulseCallbackForTesting());
+        manager_->SetKeepaliveImpulseDecrementCallbackForTesting(
+            extensions::ProcessManager::ImpulseCallbackForTesting());
         quit_callback.Run();
       }
     }
@@ -190,6 +197,7 @@ class ImpulseCallbackCounter {
 
   int observed_;
   int goal_;
+  extensions::ProcessManager* manager_;
   const std::string extension_id_;
   scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
 };
@@ -597,17 +605,13 @@ IN_PROC_BROWSER_TEST_F(AppBackgroundPageNaClTest, BackgroundKeepaliveActive) {
   LaunchTestingApp();
   extensions::ProcessManager* manager =
     extensions::ExtensionSystem::Get(browser()->profile())->process_manager();
-  ImpulseCallbackCounter active_impulse_counter(extension()->id());
+  ImpulseCallbackCounter active_impulse_counter(manager, extension()->id());
   EXPECT_TRUE(nacl_modules_loaded.WaitUntilSatisfied());
 
   // Target .5 seconds: .5 seconds / 50ms throttle * 2 embeds == 20 impulses.
   manager->SetKeepaliveImpulseCallbackForTesting(
       active_impulse_counter.SetGoalAndGetCallback(20));
   active_impulse_counter.Wait();
-
-  // Clear callback to free reference to message loop in ImpulseCallbackCounter.
-  manager->SetKeepaliveImpulseCallbackForTesting(
-      extensions::ProcessManager::ImpulseCallbackForTesting());
 }
 
 // Verify that nacl modules that go idle will not send keepalive impulses.
@@ -625,16 +629,12 @@ IN_PROC_BROWSER_TEST_F(AppBackgroundPageNaClTest,
   LaunchTestingApp();
   extensions::ProcessManager* manager =
     extensions::ExtensionSystem::Get(browser()->profile())->process_manager();
-  ImpulseCallbackCounter idle_impulse_counter(extension()->id());
+  ImpulseCallbackCounter idle_impulse_counter(manager, extension()->id());
   EXPECT_TRUE(nacl_modules_loaded.WaitUntilSatisfied());
 
   manager->SetKeepaliveImpulseDecrementCallbackForTesting(
       idle_impulse_counter.SetGoalAndGetCallback(1));
   nacl_modules_loaded.Reply("be idle");
   idle_impulse_counter.Wait();
-
-  // Clear callback to free reference to message loop in ImpulseCallbackCounter.
-  manager->SetKeepaliveImpulseDecrementCallbackForTesting(
-      extensions::ProcessManager::ImpulseCallbackForTesting());
 }
 

extensions/browser/process_manager.cc

@@ -443,8 +443,11 @@ void ProcessManager::KeepaliveImpulse(const Extension* extension) {
     }
   }
 
-  if (!keepalive_impulse_callback_for_testing_.is_null())
-    keepalive_impulse_callback_for_testing_.Run(extension->id());
+  if (!keepalive_impulse_callback_for_testing_.is_null()) {
+    ImpulseCallbackForTesting callback_may_clear_callbacks_reentrantly =
+      keepalive_impulse_callback_for_testing_;
+    callback_may_clear_callbacks_reentrantly.Run(extension->id());
+  }
 }
 
 // DecrementLazyKeepaliveCount is called when no calls to KeepaliveImpulse
@@ -459,8 +462,11 @@ void ProcessManager::OnKeepaliveImpulseCheck() {
        ++i) {
     if (i->second.previous_keepalive_impulse && !i->second.keepalive_impulse) {
       DecrementLazyKeepaliveCount(i->first);
-      if (!keepalive_impulse_decrement_callback_for_testing_.is_null())
-        keepalive_impulse_decrement_callback_for_testing_.Run(i->first);
+      if (!keepalive_impulse_decrement_callback_for_testing_.is_null()) {
+        ImpulseCallbackForTesting callback_may_clear_callbacks_reentrantly =
+          keepalive_impulse_decrement_callback_for_testing_;
+        callback_may_clear_callbacks_reentrantly.Run(i->first);
+      }
     }
 
     i->second.previous_keepalive_impulse = i->second.keepalive_impulse;