Fix race condition in UnlockManagedGuestSession test

The |ResultCatcher| and |ExtensionTestMessageListener| are possibly not
ready before the login screen extension is re-enabled when the screen
locks. This results in a race condition.

This CL fixes the race by using 2 separate listeners which are
initialised before the call to the lock.

Bug: 1044280
Change-Id: I221f3f70e4eef02df3f13bd5c9c8cda24f132a2d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2012952Reviewed-by: Alexander Hendrich <hendrich@chromium.org>
Commit-Queue: Jit Yao Yap <jityao@google.com>
Cr-Commit-Position: refs/heads/master@{#733989}

chrome/browser/chromeos/extensions/login_screen/login/login_apitest.cc

@@ -40,6 +40,8 @@
 #include "extensions/browser/api/test/test_api.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/manifest.h"
+#include "extensions/test/extension_test_message_listener.h"
+#include "extensions/test/result_catcher.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -290,15 +292,28 @@ IN_PROC_BROWSER_TEST_F(LoginApitest, UnlockManagedGuestSessionLockedWithApi) {
   SetUpDeviceLocalAccountPolicy();
   LogInWithPassword();
 
-  SetUpTestListeners();
+  // |RunTest()| has to be handled by the test as it requires multiple
+  // listeners. Using one listener at a time would result in race conditions.
+  ClearTestListeners();
+  extensions::ResultCatcher catcher;
+  ExtensionTestMessageListener login_screen_listener(listener_message(),
+                                                     /*will_reply=*/true);
+  login_screen_listener.set_extension_id(extension_id());
+  ExtensionTestMessageListener in_session_listener(listener_message(),
+                                                   /*will_reply=*/true);
+  in_session_listener.set_extension_id(kInSessionExtensionId);
+
   SetUpInSessionExtension();
   SessionStateWaiter locked_waiter(session_manager::SessionState::LOCKED);
-  RunTest(kInSessionLoginLockManagedGuestSession);
+  ASSERT_TRUE(in_session_listener.WaitUntilSatisfied());
+  in_session_listener.Reply(kInSessionLoginLockManagedGuestSession);
+  ASSERT_TRUE(catcher.GetNextResult());
   locked_waiter.Wait();
 
-  SetUpTestListeners();
   SessionStateWaiter active_waiter(session_manager::SessionState::ACTIVE);
-  RunTest(kUnlockManagedGuestSession);
+  ASSERT_TRUE(login_screen_listener.WaitUntilSatisfied());
+  login_screen_listener.Reply(kUnlockManagedGuestSession);
+  ASSERT_TRUE(catcher.GetNextResult());
   active_waiter.Wait();
 }
 

chrome/browser/chromeos/extensions/login_screen/login_screen_apitest_base.cc

@@ -26,20 +26,25 @@ namespace chromeos {
 LoginScreenApitestBase::LoginScreenApitestBase(version_info::Channel channel)
     : SigninProfileExtensionsPolicyTestBase(channel),
       extension_id_(kExtensionId),
-      extension_update_manifest_path_(kExtensionUpdateManifestPath) {}
+      extension_update_manifest_path_(kExtensionUpdateManifestPath),
+      listener_message_(kWaitingForTestName) {}
 
 LoginScreenApitestBase::~LoginScreenApitestBase() {
-  catcher_.reset();
-  listener_.reset();
+  ClearTestListeners();
 }
 
 void LoginScreenApitestBase::SetUpTestListeners() {
   catcher_ = std::make_unique<extensions::ResultCatcher>();
   listener_ =
-      std::make_unique<ExtensionTestMessageListener>(kWaitingForTestName,
+      std::make_unique<ExtensionTestMessageListener>(listener_message_,
                                                      /*will_reply=*/true);
 }
 
+void LoginScreenApitestBase::ClearTestListeners() {
+  catcher_.reset();
+  listener_.reset();
+}
+
 void LoginScreenApitestBase::RunTest(const std::string& test_name) {
   RunTest(test_name, /*assert_test_succeed=*/true);
 }

chrome/browser/chromeos/extensions/login_screen/login_screen_apitest_base.h

@@ -41,6 +41,8 @@ class LoginScreenApitestBase
 
   void SetUpTestListeners();
 
+  void ClearTestListeners();
+
   void RunTest(const std::string& test_name);
   void RunTest(const std::string& test_name, bool assert_test_succeed);
 
@@ -48,9 +50,15 @@ class LoginScreenApitestBase
   void SetUpLoginScreenExtensionAndRunTest(const std::string& test_name,
                                            bool assert_test_succeed);
 
+  std::string extension_id() { return extension_id_; }
+
+  std::string listener_message() { return listener_message_; }
+
  protected:
   const std::string extension_id_;
   const std::string extension_update_manifest_path_;
+  // The message |listener_| is listening for.
+  const std::string listener_message_;
   std::unique_ptr<extensions::ResultCatcher> catcher_;
   std::unique_ptr<ExtensionTestMessageListener> listener_;
 };