Fix not deleting a lockfile or not killing a frozen browser on hostname change

chrome/browser/process_singleton_posix_unittest.cc

 // 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.
 
 #include "chrome/browser/process_singleton.h"
 
 #include <fcntl.h>
 #include <limits.h>
 #include <signal.h>
 #include <stddef.h>
@@ skipping 65 matching lines @@
                     base::WaitableEvent::InitialState::NOT_SIGNALED),
         signal_event_(base::WaitableEvent::ResetPolicy::MANUAL,
                       base::WaitableEvent::InitialState::NOT_SIGNALED),
         process_singleton_on_thread_(NULL) {}
 
   void SetUp() override {
     testing::Test::SetUp();
 
     ProcessSingleton::DisablePromptForTesting();
     ProcessSingleton::SkipIsChromeProcessCheckForTesting(false);
+    ProcessSingleton::SetUserOptedUnlockInUseProfileForTesting(false);
     // Put the lock in a temporary directory.  Doesn't need to be a
     // full profile to test this code.
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
     // Use a long directory name to ensure that the socket isn't opened through
     // the symlink.
     user_data_path_ = temp_dir_.GetPath().Append(
         std::string(sizeof(sockaddr_un::sun_path), 'a'));
     ASSERT_TRUE(CreateDirectory(user_data_path_));
 
     lock_path_ = user_data_path_.Append(chrome::kSingletonLockFilename);
@@ skipping 235 matching lines @@
 // hostname changed.
 TEST_F(ProcessSingletonPosixTest, NotifyOtherProcessHostChanged) {
   CreateProcessSingletonOnThread();
   EXPECT_EQ(0, unlink(lock_path_.value().c_str()));
   EXPECT_EQ(0, symlink("FAKEFOOHOST-1234", lock_path_.value().c_str()));
 
   EXPECT_EQ(ProcessSingleton::PROCESS_NOTIFIED, NotifyOtherProcess(false));
   CheckNotified();
 }
 
-// Test that we fail when lock says process is on another host and we can't
-// notify it over the socket.
+// Test that we kill hung browser when lock says process is on another host and
+// we can't notify it over the socket.
 TEST_F(ProcessSingletonPosixTest, NotifyOtherProcessDifferingHost) {
+  base::HistogramTester histogram_tester;
   CreateProcessSingletonOnThread();
 
   BlockWorkerThread();
 
   EXPECT_EQ(0, unlink(lock_path_.value().c_str()));
   EXPECT_EQ(0, symlink("FAKEFOOHOST-1234", lock_path_.value().c_str()));
 
-  EXPECT_EQ(ProcessSingleton::PROFILE_IN_USE, NotifyOtherProcess(false));
+  EXPECT_EQ(ProcessSingleton::PROCESS_NONE, NotifyOtherProcess(true));
+  ASSERT_EQ(1, kill_callbacks_);
 
-  ASSERT_EQ(0, unlink(lock_path_.value().c_str()));
+  // lock_path_ should be unlinked in NotifyOtherProcess().
+  base::FilePath target_path;
+  EXPECT_FALSE(base::ReadSymbolicLink(lock_path_, &target_path));
 
   UnblockWorkerThread();
+
+  histogram_tester.ExpectUniqueSample(
+      "Chrome.ProcessSingleton.RemoteHungProcessTerminateReason",
+      ProcessSingleton::SOCKET_READ_FAILED, 1u);
 }
 
-// Test that we fail when lock says process is on another host and we can't
-// notify it over the socket.
-TEST_F(ProcessSingletonPosixTest, NotifyOtherProcessOrCreate_DifferingHost) {
+// Test that we'll start creating ProcessSingleton when we have old lock file
+// that says process is on another host and there is browser with the same pid
+// but with another user data dir. Also suppose that user opted to unlock
+// profile.
+TEST_F(ProcessSingletonPosixTest,
+       NotifyOtherProcessDifferingHost_UnlockedProfileBeforeKill) {
+  base::HistogramTester histogram_tester;
   CreateProcessSingletonOnThread();
 
   BlockWorkerThread();
+
+  EXPECT_EQ(0, unlink(lock_path_.value().c_str()));
+  EXPECT_EQ(0, symlink("FAKEFOOHOST-1234", lock_path_.value().c_str()));
+
+  // Remove socket so that we will not be able to notify the existing browser.
+  EXPECT_EQ(0, unlink(socket_path_.value().c_str()));
+
+  // Unlock profile that was locked by process on another host.
+  ProcessSingleton::SetUserOptedUnlockInUseProfileForTesting(true);
+  // Treat process with pid 1234 as browser with different user data dir.
+  ProcessSingleton::SkipIsChromeProcessCheckForTesting(true);
+
+  EXPECT_EQ(ProcessSingleton::PROCESS_NONE, NotifyOtherProcess(false));
+
+  // lock_path_ should be unlinked in NotifyOtherProcess().
+  base::FilePath target_path;
+  EXPECT_FALSE(base::ReadSymbolicLink(lock_path_, &target_path));
+
+  UnblockWorkerThread();
+
+  histogram_tester.ExpectUniqueSample(
+      "Chrome.ProcessSingleton.RemoteHungProcessTerminateReason",
+      ProcessSingleton::NOTIFY_ATTEMPTS_EXCEEDED, 1u);
+  histogram_tester.ExpectUniqueSample(
+      "Chrome.ProcessSingleton.RemoteProcessInteractionResult",
+      ProcessSingleton::PROFILE_UNLOCKED_BEFORE_KILL, 1u);
+}
+
+// Test that we unlock profile when lock says process is on another host and we
+// can't notify it over the socket.
+TEST_F(ProcessSingletonPosixTest, NotifyOtherProcessOrCreate_DifferingHost) {
+  base::HistogramTester histogram_tester;
+  CreateProcessSingletonOnThread();
+
+  BlockWorkerThread();
 
   EXPECT_EQ(0, unlink(lock_path_.value().c_str()));
   EXPECT_EQ(0, symlink("FAKEFOOHOST-1234", lock_path_.value().c_str()));
 
+  // Remove socket so that we will not be able to notify the existing browser.
+  EXPECT_EQ(0, unlink(socket_path_.value().c_str()));
+  // Unlock profile that was locked by process on another host.
+  ProcessSingleton::SetUserOptedUnlockInUseProfileForTesting(true);
+
   std::string url("about:blank");
-  EXPECT_EQ(ProcessSingleton::PROFILE_IN_USE, NotifyOtherProcessOrCreate(url));
+  EXPECT_EQ(ProcessSingleton::PROCESS_NONE, NotifyOtherProcessOrCreate(url));
 
   ASSERT_EQ(0, unlink(lock_path_.value().c_str()));
 
   UnblockWorkerThread();
+
+  histogram_tester.ExpectUniqueSample(
+      "Chrome.ProcessSingleton.RemoteProcessInteractionResult",
+      ProcessSingleton::PROFILE_UNLOCKED, 1u);
 }
 
 // Test that Create fails when another browser is using the profile directory.
 TEST_F(ProcessSingletonPosixTest, CreateFailsWithExistingBrowser) {
   CreateProcessSingletonOnThread();
 
   std::unique_ptr<TestableProcessSingleton> process_singleton(
       CreateProcessSingleton());
   process_singleton->OverrideCurrentPidForTesting(base::GetCurrentProcId() + 1);
   EXPECT_FALSE(process_singleton->Create());
@@ skipping 80 matching lines @@
 // Test that if there is an existing lock file, and it's not locked, we replace
 // it.
 TEST_F(ProcessSingletonPosixTest, CreateReplacesOldMacLock) {
   std::unique_ptr<TestableProcessSingleton> process_singleton(
       CreateProcessSingleton());
   EXPECT_EQ(0, base::WriteFile(lock_path_, "", 0));
   EXPECT_TRUE(process_singleton->Create());
   VerifyFiles();
 }
 #endif  // defined(OS_MACOSX)