TabManagerDelegate: Better prioritize ARC processes.

chrome/browser/memory/tab_manager_delegate_chromeos.cc

Index: chrome/browser/memory/tab_manager_delegate_chromeos.cc
diff --git a/chrome/browser/memory/tab_manager_delegate_chromeos.cc b/chrome/browser/memory/tab_manager_delegate_chromeos.cc
index 5ffbd0b6d4f490d7f156a4f2726f80c3e3adbe2d..473747349918346249f6b9f38fa0ab7d8e75f6a7 100644
--- a/chrome/browser/memory/tab_manager_delegate_chromeos.cc
+++ b/chrome/browser/memory/tab_manager_delegate_chromeos.cc
@@ -76,65 +76,6 @@ bool IsArcWindow(aura::Window* window) {
                           base::CompareCase::SENSITIVE);
 }
 
-bool IsArcWindowInForeground() {
-  auto activation_client = GetActivationClient();
-  return activation_client && IsArcWindow(activation_client->GetActiveWindow());
-}
-
-int AppStateToPriority(
-    const arc::mojom::ProcessState& process_state) {
-  // Logic copied from Android:
-  // frameworks/base/core/java/android/app/ActivityManager.java
-  // Note that ProcessState enumerates from most important (lower value) to
-  // least important (higher value), while ProcessPriority enumerates the
-  // opposite.
-  if (process_state >= arc::mojom::ProcessState::HOME) {
-    return ProcessPriority::ANDROID_BACKGROUND;
-  } else if (process_state >= arc::mojom::ProcessState::SERVICE) {
-    return ProcessPriority::ANDROID_SERVICE;
-  } else if (process_state >= arc::mojom::ProcessState::HEAVY_WEIGHT) {
-    return ProcessPriority::ANDROID_CANT_SAVE_STATE;
-  } else if (process_state >= arc::mojom::ProcessState::IMPORTANT_BACKGROUND) {
-    return ProcessPriority::ANDROID_PERCEPTIBLE;
-  } else if (process_state >= arc::mojom::ProcessState::IMPORTANT_FOREGROUND) {
-    return ProcessPriority::ANDROID_VISIBLE;
-  } else if (process_state >= arc::mojom::ProcessState::TOP_SLEEPING) {
-    return ProcessPriority::ANDROID_TOP_SLEEPING;
-  } else if (process_state >= arc::mojom::ProcessState::FOREGROUND_SERVICE) {
-    return ProcessPriority::ANDROID_FOREGROUND_SERVICE;
-  } else if (process_state >= arc::mojom::ProcessState::TOP) {
-    return IsArcWindowInForeground() ?
-        ProcessPriority::ANDROID_TOP :
-        ProcessPriority::ANDROID_TOP_INACTIVE;
-  } else if (process_state >= arc::mojom::ProcessState::PERSISTENT) {
-    return ProcessPriority::ANDROID_PERSISTENT;
-  }
-  return ProcessPriority::ANDROID_NON_EXISTS;
-}
-
-int TabStatsToPriority(const TabStats& tab) {
-  if (tab.is_selected)
-    return ProcessPriority::CHROME_SELECTED;
-
-  int priority = 0;
-
-  if (tab.is_app) {
-    priority = ProcessPriority::CHROME_APP;
-  } else if (tab.is_internal_page) {
-    priority = ProcessPriority::CHROME_INTERNAL;
-  } else {
-    priority = ProcessPriority::CHROME_NORMAL;
-  }
-  if (tab.is_pinned)
-    priority |= ProcessPriority::CHROME_PINNED;
-  if (tab.is_media)
-    priority |= ProcessPriority::CHROME_MEDIA;
-  if (tab.has_form_entry)
-    priority |= ProcessPriority::CHROME_CANT_SAVE_STATE;
-
-  return priority;
-}
-
 bool IsArcMemoryManagementEnabled() {
   return base::FeatureList::IsEnabled(features::kArcMemoryManagement);
 }
@@ -143,13 +84,16 @@ bool IsArcMemoryManagementEnabled() {
 
 std::ostream& operator<<(
     std::ostream& out, const TabManagerDelegate::Candidate& candidate) {
-  if (candidate.is_arc_app) {
-    out << "app " << candidate.app->pid()
-        << " (" << candidate.app->process_name() << ")";
-  } else {
-    out << "tab " << candidate.tab->renderer_handle;
+  if (candidate.app()) {
+    out << "app " << candidate.app()->pid()
+        << " (" << candidate.app()->process_name() << ")"
+        << ", process_state " << candidate.app()->process_state()
+        << ", is_focused " << candidate.app()->is_focused()
+        << ", lastActivityTime " << candidate.app()->last_activity_time();
+  } else if (candidate.tab()) {
+    out << "tab " << candidate.tab()->renderer_handle;
   }
-  out << " with priority " << candidate.priority;
+  out << ", process_type " << candidate.process_type();
   return out;
 }
 
@@ -405,6 +349,10 @@ void TabManagerDelegate::OnInstanceClosed() {
   arc_process_instance_version_ = 0;
 }
 
+// TODO(cylee): Remove this function if Android process OOM score settings
+// is moved back to Android.
+// For example, negotiate non-overlapping OOM score ranges so Chrome and Android
+// can set OOM score for processes in their own world.
 void TabManagerDelegate::OnWindowActivated(
     aura::client::ActivationChangeObserver::ActivationReason reason,
     aura::Window* gained_active,
@@ -598,32 +546,37 @@ void TabManagerDelegate::AdjustOomPriorities(const TabStatsList& tab_list) {
 }
 
 // Excludes persistent ARC apps, but still preserves active chrome tabs and
-// top ARC apps. The latter ones should not be killed by TabManager since
-// we still want to adjust their oom_score_adj.
+// focused ARC apps. The latter ones should not be killed by TabManager here,
+// but we want to adjust their oom_score_adj.
 // static
 std::vector<TabManagerDelegate::Candidate>
 TabManagerDelegate::GetSortedCandidates(
     const TabStatsList& tab_list,
     const std::vector<arc::ArcProcess>& arc_processes) {
 
+  static const char kAppLauncherProcessName[] = "org.chromium.arc.applauncher";

[Yusuke Sato, 2016/07/15 00:55:05]

constexpr

[cylee1, 2016/07/15 17:31:13]

Done.

+
   std::vector<Candidate> candidates;
   candidates.reserve(tab_list.size() + arc_processes.size());
 
   for (const auto& tab : tab_list) {
-    candidates.push_back(Candidate(&tab, TabStatsToPriority(tab)));
+    candidates.push_back(Candidate(&tab));
   }
 
   for (const auto& app : arc_processes) {
-    Candidate candidate(&app, AppStateToPriority(app.process_state()));
-    // Skip persistent android processes since we should never kill them.
-    // Also don't ajust OOM score so their score remains min oom_score_adj.
-    if (candidate.priority >= ProcessPriority::ANDROID_PERSISTENT)
+    // Skip persistent android processes since they should never be killed here.
+    // Neither do we set their OOM scores so their score remains minimum.
+    //
+    // AppLauncher is treated specially in ARC++. For example it is taken
+    // as the dummy foreground app from Android's point of view when the focused
+    // window is not an Android app. We prefer never kill it.
+    if (app.process_state() <= arc::mojom::ProcessState::PERSISTENT_UI ||
+        app.process_name() == kAppLauncherProcessName)
       continue;
-    candidates.push_back(candidate);
+    candidates.push_back(Candidate(&app));
   }
 
   // Sort candidates according to priority.
-  // TODO(cylee): Missing LRU property. Fix it when apps has the information.
   std::sort(candidates.begin(), candidates.end());
 
   return candidates;
@@ -648,37 +601,41 @@ void TabManagerDelegate::LowMemoryKillImpl(
     const std::vector<arc::ArcProcess>& arc_processes) {
 
   VLOG(2) << "LowMemoryKilleImpl";
-  std::vector<TabManagerDelegate::Candidate> candidates =
+  const std::vector<TabManagerDelegate::Candidate> candidates =
       GetSortedCandidates(tab_list, arc_processes);
 
   int target_memory_to_free_kb = mem_stat_->TargetMemoryToFreeKB();
-  for (const auto& entry : candidates) {
+  // Kill processes until the estimated amount of freed memory is sufficient to
+  // bring the system memory back to a normal level.
+  // The list is sorted by importance descendingly, so we go through the list
+  // backwards.
+  for (auto it = candidates.rbegin(); it != candidates.rend(); ++it) {

[Yusuke Sato, 2016/07/15 00:55:05]

const auto& it

probably?

[cylee1, 2016/07/15 17:31:13]

No it will make "it" const so I cant ++it
candidates is of type const vector<> so is should returns const_iterator() by
default. I think it means auto derives to const_iterator already.

http://www.cplusplus.com/reference/vector/vector/begin/
"If the vector object is const-qualified, the function returns a const_iterator.
Otherwise, it returns an iterator."

     VLOG(3) << "Target memory to free: " << target_memory_to_free_kb << " KB";
     // Never kill selected tab or Android foreground app, regardless whether
     // they're in the active window. Since the user experience would be bad.
-    if ((!entry.is_arc_app &&
-         entry.priority >= ProcessPriority::CHROME_SELECTED) ||
-        (entry.is_arc_app &&
-         entry.priority >= ProcessPriority::ANDROID_TOP_INACTIVE)) {
-      VLOG(2) << "Skipped killing " << entry;
+    ProcessType process_type = it->process_type();
+    if (process_type == ProcessType::VISIBLE_APP ||
+        process_type == ProcessType::FOCUSED_APP ||
+        process_type == ProcessType::FOCUSED_TAB) {
+      VLOG(2) << "Skipped killing " << *it;
       continue;
     }
-    if (entry.is_arc_app) {
+    if (it->app()) {
         int estimated_memory_freed_kb =
-            mem_stat_->EstimatedMemoryFreedKB(entry.app->pid());
-        if (KillArcProcess(entry.app->nspid())) {
+            mem_stat_->EstimatedMemoryFreedKB(it->app()->pid());
+        if (KillArcProcess(it->app()->nspid())) {
           target_memory_to_free_kb -= estimated_memory_freed_kb;
           uma_->ReportKill(estimated_memory_freed_kb);
-          VLOG(2) << "Killed " << entry;
+          VLOG(2) << "Killed " << *it;
         }
     } else {
-      int64_t tab_id = entry.tab->tab_contents_id;
+      int64_t tab_id = it->tab()->tab_contents_id;
       int estimated_memory_freed_kb =
-          mem_stat_->EstimatedMemoryFreedKB(entry.tab->renderer_handle);
+          mem_stat_->EstimatedMemoryFreedKB(it->tab()->renderer_handle);
       if (KillTab(tab_id)) {
         target_memory_to_free_kb -= estimated_memory_freed_kb;
         uma_->ReportKill(estimated_memory_freed_kb);
-        VLOG(2) << "Killed " << entry;
+        VLOG(2) << "Killed " << *it;
       }
     }
     if (target_memory_to_free_kb < 0)
@@ -690,7 +647,7 @@ void TabManagerDelegate::AdjustOomPrioritiesImpl(
     const TabStatsList& tab_list,
     const std::vector<arc::ArcProcess>& arc_processes) {
   // Least important first.
-  auto candidates = GetSortedCandidates(tab_list, arc_processes);
+  const auto candidates = GetSortedCandidates(tab_list, arc_processes);
 
   // Now we assign priorities based on the sorted list. We're assigning
   // priorities in the range of kLowestRendererOomScore to
@@ -710,10 +667,9 @@ void TabManagerDelegate::AdjustOomPrioritiesImpl(
   // Find some pivot point. For now processes with priority >= CHROME_INTERNAL
   // are prone to be affected by LRU change. Taking them as "high priority"
   // processes.
-  auto lower_priority_part = candidates.rend();
-  // Iterate in reverse order since the list is sorted by least importance.
-  for (auto it = candidates.rbegin(); it != candidates.rend(); ++it) {
-    if (it->priority < ProcessPriority::CHROME_INTERNAL) {
+  auto lower_priority_part = candidates.end();
+  for (auto it = candidates.begin(); it != candidates.end(); ++it) {

[Yusuke Sato, 2016/07/15 00:55:05]

same

[cylee1, 2016/07/15 17:31:13]

same.

+    if (it->process_type() >= ProcessType::BACKGROUND_APP) {
       lower_priority_part = it;
       break;
     }
@@ -722,11 +678,11 @@ void TabManagerDelegate::AdjustOomPrioritiesImpl(
   ProcessScoreMap new_map;
 
   // Higher priority part.
-  DistributeOomScoreInRange(candidates.rbegin(), lower_priority_part,
+  DistributeOomScoreInRange(candidates.begin(), lower_priority_part,
                             chrome::kLowestRendererOomScore, range_middle,
                             &new_map);
   // Lower priority part.
-  DistributeOomScoreInRange(lower_priority_part, candidates.rend(),
+  DistributeOomScoreInRange(lower_priority_part, candidates.end(),
                             range_middle, chrome::kHighestRendererOomScore,
                             &new_map);
   base::AutoLock oom_score_autolock(oom_score_lock_);
@@ -762,8 +718,8 @@ void TabManagerDelegate::SetOomScoreAdjForTabsOnFileThread(
 }
 
 void TabManagerDelegate::DistributeOomScoreInRange(
-    std::vector<TabManagerDelegate::Candidate>::reverse_iterator rbegin,
-    std::vector<TabManagerDelegate::Candidate>::reverse_iterator rend,
+    std::vector<TabManagerDelegate::Candidate>::const_iterator begin,
+    std::vector<TabManagerDelegate::Candidate>::const_iterator end,
     int range_begin,
     int range_end,
     ProcessScoreMap* new_map) {
@@ -774,27 +730,27 @@ void TabManagerDelegate::DistributeOomScoreInRange(
   // Though there might be duplicate process handles, it doesn't matter to
   // overestimate the number of processes here since the we don't need to
   // use up the full range.
-  int num = (rend - rbegin);
+  int num = (end - begin);
   const float priority_increment =
       static_cast<float>(range_end - range_begin) / num;
 
   float priority = range_begin;
-  for (auto cur = rbegin; cur != rend; ++cur) {
+  for (auto cur = begin; cur != end; ++cur) {
     int score = static_cast<int>(priority + 0.5f);
-    if (cur->is_arc_app) {
+    if (cur->app()) {
       // Use pid as map keys so it's globally unique.
-      (*new_map)[cur->app->pid()] = score;
+      (*new_map)[cur->app()->pid()] = score;
       int cur_app_pid_score = 0;
       {
         base::AutoLock oom_score_autolock(oom_score_lock_);
-        cur_app_pid_score = oom_score_map_[cur->app->pid()];
+        cur_app_pid_score = oom_score_map_[cur->app()->pid()];
       }
       if (cur_app_pid_score != score) {
         VLOG(3) << "Set OOM score " << score << " for " << *cur;
-        SetOomScoreAdjForApp(cur->app->nspid(), score);
+        SetOomScoreAdjForApp(cur->app()->nspid(), score);
       }
     } else {
-      base::ProcessHandle process_handle = cur->tab->renderer_handle;
+      base::ProcessHandle process_handle = cur->tab()->renderer_handle;
       // 1. tab_list contains entries for already-discarded tabs. If the PID
       // (renderer_handle) is zero, we don't need to adjust the oom_score.
       // 2. Only add unseen process handle so if there's multiple tab maps to