Do not kill recently killed ARC processes again

chrome/browser/memory/tab_manager_delegate_chromeos_unittest.cc

 // Copyright (c) 2012 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/memory/tab_manager_delegate_chromeos.h"
 
 #include <algorithm>
 #include <map>
 #include <string>
 #include <vector>
@@ skipping 84 matching lines @@
   // chrome app.
   EXPECT_EQ(500, candidates[6].tab()->tab_contents_id);
   // internal page.
   EXPECT_EQ(200, candidates[7].tab()->tab_contents_id);
   // background service.
   EXPECT_EQ("service", candidates[8].app()->process_name());
 }
 
 class MockTabManagerDelegate : public TabManagerDelegate {
  public:
-  MockTabManagerDelegate(): TabManagerDelegate(nullptr) {
-  }
+  MockTabManagerDelegate()
+      : TabManagerDelegate(nullptr),
+        always_return_true_from_is_recently_killed_(false) {}
 
   explicit MockTabManagerDelegate(TabManagerDelegate::MemoryStat* mem_stat)
-      : TabManagerDelegate(nullptr, mem_stat) {
-  }
+      : TabManagerDelegate(nullptr, mem_stat),
+        always_return_true_from_is_recently_killed_(false) {}
 
   // unit test.
   std::vector<int> GetKilledArcProcesses() {
     return killed_arc_processes_;
   }
 
   // unit test.
   std::vector<int64_t> GetKilledTabs() {
     return killed_tabs_;
   }
 
   // unit test.
   void Clear() {
     killed_arc_processes_.clear();
     killed_tabs_.clear();
   }
 
+  // unit test.
+  void set_always_return_true_from_is_recently_killed(
+      bool always_return_true_from_is_recently_killed) {
+    always_return_true_from_is_recently_killed_ =
+        always_return_true_from_is_recently_killed;
+  }
+
+  bool IsRecentlyKilledArcProcess(const std::string& process_name,
+                                  const base::TimeTicks& now) override {
+    if (always_return_true_from_is_recently_killed_)
+      return true;
+    return TabManagerDelegate::IsRecentlyKilledArcProcess(process_name, now);
+  }
+
  protected:
   bool KillArcProcess(const int nspid) override {
     killed_arc_processes_.push_back(nspid);
     return true;
   }
 
   bool KillTab(int64_t tab_id) override {
     killed_tabs_.push_back(tab_id);
     return true;
   }
 
   chromeos::DebugDaemonClient* GetDebugDaemonClient() override {
     return &debugd_client_;
   }
 
  private:
   chromeos::FakeDebugDaemonClient debugd_client_;
   std::vector<int> killed_arc_processes_;
   std::vector<int64_t> killed_tabs_;
+  bool always_return_true_from_is_recently_killed_;
 };
 
 class MockMemoryStat : public TabManagerDelegate::MemoryStat {
  public:
   MockMemoryStat() {}
   ~MockMemoryStat() override {}
 
   int TargetMemoryToFreeKB() override {
     return target_memory_to_free_kb_;
   }
@@ skipping 67 matching lines @@
   EXPECT_EQ(300, oom_score_map[10]);
   EXPECT_EQ(344, oom_score_map[20]);
   EXPECT_EQ(388, oom_score_map[40]);
   EXPECT_EQ(431, oom_score_map[12]);
   EXPECT_EQ(475, oom_score_map[11]);
 
   // Lower priority part.
   EXPECT_EQ(650, oom_score_map[30]);
 }
 
+TEST_F(TabManagerDelegateTest, IsRecentlyKilledArcProcess) {
+  constexpr char kProcessName1[] = "org.chromium.arc.test1";
+  constexpr char kProcessName2[] = "org.chromium.arc.test2";
+
+  // Not owned.
+  MockMemoryStat* memory_stat = new MockMemoryStat();
+  // Instantiate the mock instance.
+  MockTabManagerDelegate tab_manager_delegate(memory_stat);
+
+  // When the process name is not in the map, sRecentlyKilledArcProcess should

[Georges Khalil, 2017/05/02 21:36:30]

nit: s/sRecentlyKilledArcProcess/IsRecentlyKilledArcProcess

[Yusuke Sato, 2017/05/02 21:40:11]

Done.

+  // return false.
+  const base::TimeTicks now = base::TimeTicks::Now();
+  EXPECT_FALSE(
+      tab_manager_delegate.IsRecentlyKilledArcProcess(kProcessName1, now));
+  EXPECT_FALSE(
+      tab_manager_delegate.IsRecentlyKilledArcProcess(kProcessName2, now));
+
+  // Update the map to tell the manager that the process was killed very
+  // recently.
+  tab_manager_delegate.recently_killed_arc_processes_[kProcessName1] = now;
+  EXPECT_TRUE(
+      tab_manager_delegate.IsRecentlyKilledArcProcess(kProcessName1, now));
+  EXPECT_FALSE(
+      tab_manager_delegate.IsRecentlyKilledArcProcess(kProcessName2, now));
+  tab_manager_delegate.recently_killed_arc_processes_[kProcessName1] =
+      now - base::TimeDelta::FromMicroseconds(1);
+  EXPECT_TRUE(
+      tab_manager_delegate.IsRecentlyKilledArcProcess(kProcessName1, now));
+  EXPECT_FALSE(
+      tab_manager_delegate.IsRecentlyKilledArcProcess(kProcessName2, now));
+  tab_manager_delegate.recently_killed_arc_processes_[kProcessName1] =
+      now - TabManagerDelegate::GetArcRespawnKillDelay();
+  EXPECT_TRUE(
+      tab_manager_delegate.IsRecentlyKilledArcProcess(kProcessName1, now));
+  EXPECT_FALSE(
+      tab_manager_delegate.IsRecentlyKilledArcProcess(kProcessName2, now));
+
+  // Update the map to tell the manager that the process was killed
+  // (kArcSkipKillingTimeInSeconds + 1) seconds ago. In this case,

[cylee1, 2017/05/02 20:59:53]

nit: kArcSkipKillingTimeInSeconds no longer exists

[Yusuke Sato, 2017/05/02 21:40:11]

Done.

+  // IsRecentlyKilledArcProcess(kProcessName1) should return false.
+  tab_manager_delegate.recently_killed_arc_processes_[kProcessName1] =
+      now - TabManagerDelegate::GetArcRespawnKillDelay() -
+      base::TimeDelta::FromSeconds(1);
+  EXPECT_FALSE(
+      tab_manager_delegate.IsRecentlyKilledArcProcess(kProcessName1, now));
+  EXPECT_FALSE(
+      tab_manager_delegate.IsRecentlyKilledArcProcess(kProcessName2, now));
+}
+
+TEST_F(TabManagerDelegateTest, DoNotKillRecentlyKilledArcProcesses) {
+  // Not owned.
+  MockMemoryStat* memory_stat = new MockMemoryStat();
+
+  // Instantiate the mock instance.
+  MockTabManagerDelegate tab_manager_delegate(memory_stat);
+  tab_manager_delegate.set_always_return_true_from_is_recently_killed(true);
+
+  std::vector<arc::ArcProcess> arc_processes;
+  arc_processes.emplace_back(
+      1, 10, "service", arc::mojom::ProcessState::SERVICE, kNotFocused, 500);
+
+  memory_stat->SetTargetMemoryToFreeKB(250000);
+  memory_stat->SetProcessPss(30, 10000);
+  tab_manager_delegate.LowMemoryKillImpl({} /* tab_list */, arc_processes);
+
+  auto killed_arc_processes = tab_manager_delegate.GetKilledArcProcesses();
+  EXPECT_EQ(0U, killed_arc_processes.size());
+}
+
 TEST_F(TabManagerDelegateTest, KillMultipleProcesses) {
   // Not owned.
   MockMemoryStat* memory_stat = new MockMemoryStat();
 
   // Instantiate the mock instance.
   MockTabManagerDelegate tab_manager_delegate(memory_stat);
 
   std::vector<arc::ArcProcess> arc_processes;
   arc_processes.emplace_back(1, 10, "focused", arc::mojom::ProcessState::TOP,
                              kIsFocused, 100);
@@ skipping 55 matching lines @@
   // Killed apps and their nspid.
   EXPECT_EQ(1U, killed_arc_processes.size());
   EXPECT_EQ(3, killed_arc_processes[0]);
   // Killed tabs and their content id.
   // Note that process with pid 11 is counted twice. But so far I don't have a
   // good way to estimate the memory freed if multiple tabs share one process.
   EXPECT_EQ(3U, killed_tabs.size());
   EXPECT_EQ(2, killed_tabs[0]);
   EXPECT_EQ(5, killed_tabs[1]);
   EXPECT_EQ(1, killed_tabs[2]);
+
+  // Check that killed apps are in the map.
+  const TabManagerDelegate::KilledArcProcessesMap& processes_map =
+      tab_manager_delegate.recently_killed_arc_processes_;
+  EXPECT_EQ(1U, processes_map.size());
+  EXPECT_EQ(1U, processes_map.count("service"));
 }
 
 }  // namespace memory