Add arc_process_unittest.cc

chrome/browser/chromeos/arc/process/arc_process_unittest.cc

Index: chrome/browser/chromeos/arc/process/arc_process_unittest.cc
diff --git a/chrome/browser/chromeos/arc/process/arc_process_unittest.cc b/chrome/browser/chromeos/arc/process/arc_process_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..546d061deffbf57604505f87f0c2709aa4bb6827
--- /dev/null
+++ b/chrome/browser/chromeos/arc/process/arc_process_unittest.cc
@@ -0,0 +1,55 @@
+// Copyright 2017 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/chromeos/arc/process/arc_process.h"
+
+#include <list>
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace arc {

[Luis Héctor Chávez, 2017/05/01 15:19:51]

What was the final decision as to having the test methods in the anonymous vs.
the arc namespace? I can't find it anywhere :(

[Yusuke Sato, 2017/05/01 17:59:52]

Good catch. The last conversation I remember was
https://codereview.chromium.org/2648213004/diff/40001/components/arc/arc_util...
. Let me use arc+anonymous which is the safest anyways and follows the coding
guide.

+
+// Tests that ArcProcess objects can be sorted by their priority (higher to
+// lower). This is critical for the OOM handler to work correctly.
+TEST(ArcProcess, TestSorting) {
+  const int64_t kNow = 1234567890;

[Luis Héctor Chávez, 2017/05/01 15:19:51]

nit: constexpr

[Yusuke Sato, 2017/05/01 17:59:52]

Done.

+
+  std::list<ArcProcess> processes;

[hidehiko, 2017/05/01 05:24:48]

IIUC, list is used to avoid copying ArcProcess on sorting, right?
If so how about add a brief comment?

[Yusuke Sato, 2017/05/01 17:59:52]

No strong reason actually, used it just for emplace_front. Added a comment.

[hidehiko, 2017/05/02 05:43:08]

In future, if emplace_front is just a reason, probably I'd recommend deque,
instead, which is randomaccessible so no need to use iterator.

+  processes.emplace_back(0, 0, "process 0", mojom::ProcessState::PERSISTENT,
+                         false /* is_foreground */, kNow + 1);
+  processes.emplace_front(1, 1, "process 1", mojom::ProcessState::PERSISTENT,
+                          false, kNow);
+  processes.emplace_back(2, 2, "process 2", mojom::ProcessState::LAST_ACTIVITY,
+                         false, kNow);
+  processes.emplace_front(3, 3, "process 3", mojom::ProcessState::LAST_ACTIVITY,
+                          false, kNow + 1);
+  processes.emplace_back(4, 4, "process 4", mojom::ProcessState::CACHED_EMPTY,
+                         false, kNow + 1);
+  processes.emplace_front(5, 5, "process 5", mojom::ProcessState::CACHED_EMPTY,
+                          false, kNow);
+  processes.sort();
+
+  ASSERT_LT(mojom::ProcessState::PERSISTENT,

[Luis Héctor Chávez, 2017/05/01 15:19:51]

This can be done as a static assert (in arc_process.cc), so it breaks on
compile-time rather than during tests :)

[Yusuke Sato, 2017/05/01 17:59:52]

Done.

+            mojom::ProcessState::LAST_ACTIVITY);
+  ASSERT_LT(mojom::ProcessState::LAST_ACTIVITY,
+            mojom::ProcessState::CACHED_EMPTY);
+
+  std::list<ArcProcess>::const_iterator it = processes.begin();
+  // 0 should have higher priority since its last_activity_time is more recent.
+  EXPECT_EQ(0, it->pid());
+  ++it;
+  EXPECT_EQ(1, it->pid());
+  ++it;
+  // Same, 3 should have higher priority.
+  EXPECT_EQ(3, it->pid());
+  ++it;
+  EXPECT_EQ(2, it->pid());
+  ++it;
+  // Same, 4 should have higher priority.
+  EXPECT_EQ(4, it->pid());
+  ++it;
+  EXPECT_EQ(5, it->pid());
+}
+
+}  // namespace arc