Refactor ARC OOM handler code

chrome/browser/memory/tab_manager_delegate_chromeos.cc

 // Copyright 2015 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 <math.h>
 #include <stdint.h>
 
 #include <algorithm>
 #include <map>
 #include <vector>
 
 #include "ash/shell.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
@@ skipping 12 matching lines @@
 #include "chrome/browser/memory/memory_kills_monitor.h"
 #include "chrome/browser/memory/tab_stats.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_features.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "components/arc/arc_bridge_service.h"
 #include "components/arc/arc_service_manager.h"
-#include "components/arc/common/process.mojom.h"
 #include "components/device_event_log/device_event_log.h"
 #include "components/exo/shell_surface.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/zygote_host_linux.h"
 #include "ui/aura/window.h"
 #include "ui/wm/public/activation_client.h"
@@ skipping 29 matching lines @@
   return base::StartsWith(application_id, kArcProcessNamePrefix,
                           base::CompareCase::SENSITIVE);
 }
 
 bool IsArcMemoryManagementEnabled() {
   return base::FeatureList::IsEnabled(features::kArcMemoryManagement);
 }
 
 void OnSetOomScoreAdj(bool success, const std::string& output) {
   VLOG(2) << "OnSetOomScoreAdj " << success << " " << output;
-  if (!success || output != "")
-    LOG(WARNING) << "Set OOM score error: " << output;
+  if (!success)
+    LOG(ERROR) << "Set OOM score error: " << output;
+  else if (!output.empty())
+    LOG(WARNING) << "Set OOM score: " << output;
 }
 
 }  // namespace
 
+// static
+const int TabManagerDelegate::kLowestOomScore = -1000;
+
 std::ostream& operator<<(std::ostream& os, const ProcessType& type) {
   switch (type) {
     case ProcessType::FOCUSED_TAB:
       return os << "FOCUSED_TAB";
     case ProcessType::FOCUSED_APP:
       return os << "FOCUSED_APP";
-    case ProcessType::VISIBLE_APP:
-      return os << "VISIBLE_APP";
+    case ProcessType::IMPORTANT_APP:
+      return os << "IMPORTANT_APP";
     case ProcessType::BACKGROUND_TAB:
       return os << "BACKGROUND_TAB";
     case ProcessType::BACKGROUND_APP:
       return os << "BACKGROUND_APP";
     case ProcessType::UNKNOWN_TYPE:
       return os << "UNKNOWN_TYPE";
     default:
       return os << "NOT_IMPLEMENTED_ERROR";
   }
   return os;
@@ skipping 35 matching lines @@
   // apps.
   NOTREACHED() << "Undefined comparison between apps and tabs: process_type="
                << process_type();
   return app();
 }
 
 ProcessType TabManagerDelegate::Candidate::GetProcessTypeInternal() const {
   if (app()) {
     if (app()->is_focused())
       return ProcessType::FOCUSED_APP;
-    if (app()->process_state() <=
-        arc::mojom::ProcessState::IMPORTANT_FOREGROUND) {
-      return ProcessType::VISIBLE_APP;
-    }
+    if (app()->IsImportant())
+      return ProcessType::IMPORTANT_APP;
     return ProcessType::BACKGROUND_APP;
   }
   if (tab()) {
     if (tab()->is_selected)
       return ProcessType::FOCUSED_TAB;
     return ProcessType::BACKGROUND_TAB;
   }
   NOTREACHED() << "Unexpected process type";
   return ProcessType::UNKNOWN_TYPE;
 }
@@ skipping 352 matching lines @@
 TabManagerDelegate::GetSortedCandidates(
     const TabStatsList& tab_list,
     const std::vector<arc::ArcProcess>& arc_processes) {
   std::vector<Candidate> candidates;
   candidates.reserve(tab_list.size() + arc_processes.size());
 
   for (const auto& tab : tab_list) {
     candidates.emplace_back(&tab);
   }
 
-  // A special process on Android side which serves as a dummy "focused" app
-  // when the focused window is a Chrome side window (i.e., all Android
-  // processes are running in the background). We don't want to kill it anyway.
-  static constexpr char kArcInBackgroundDummyprocess[] =
-      "org.chromium.arc.home";
-
   for (const auto& app : arc_processes) {
-    // Skip persistent android processes since they should never be killed here.
-    // Neither do we set their OOM scores so their score remains minimum.
-    if (app.process_state() <= arc::mojom::ProcessState::PERSISTENT_UI ||
-        app.process_name() == kArcInBackgroundDummyprocess) {
-      continue;
-    }
     candidates.emplace_back(&app);
   }
 
   // Sort candidates according to priority.
   std::sort(candidates.begin(), candidates.end());
 
   return candidates;
 }
 
 bool TabManagerDelegate::IsRecentlyKilledArcProcess(
@@ skipping 45 matching lines @@
 
   // 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 descending importance, so we go through the list
   // backwards.
   for (auto it = candidates.rbegin(); it != candidates.rend(); ++it) {
     MEMORY_LOG(ERROR) << "Target memory to free: " << target_memory_to_free_kb
                       << " KB";
     if (target_memory_to_free_kb <= 0)
       break;
-    // Never kill selected tab or Android foreground app, regardless whether
-    // they're in the active window. Since the user experience would be bad.
+    // Never kill selected tab, foreground app, and important apps regardless of
+    // whether they're in the active window. Since the user experience would be
+    // bad.
     ProcessType process_type = it->process_type();
-    if (process_type == ProcessType::VISIBLE_APP ||
-        process_type == ProcessType::FOCUSED_APP ||
-        process_type == ProcessType::FOCUSED_TAB) {
+    if (process_type <= ProcessType::IMPORTANT_APP) {
       MEMORY_LOG(ERROR) << "Skipped killing " << *it;
       continue;
     }
     if (it->app()) {
       if (IsRecentlyKilledArcProcess(it->app()->process_name(), now)) {
         MEMORY_LOG(ERROR) << "Avoided killing " << *it << " too often";
         continue;
       }
       int estimated_memory_freed_kb =
           mem_stat_->EstimatedMemoryFreedKB(it->app()->pid());
@@ skipping 25 matching lines @@
   }
   if (target_memory_to_free_kb > 0) {
     MEMORY_LOG(ERROR)
         << "Unable to kill enough candidates to meet target_memory_to_free_kb ";
   }
 }
 
 void TabManagerDelegate::AdjustOomPrioritiesImpl(
     const TabStatsList& tab_list,
     const std::vector<arc::ArcProcess>& arc_processes) {
+  std::vector<TabManagerDelegate::Candidate> candidates;
+  std::vector<TabManagerDelegate::Candidate> apps_non_killable;
+
   // Least important first.
-  const auto candidates = GetSortedCandidates(tab_list, arc_processes);
+  auto all_candidates = GetSortedCandidates(tab_list, arc_processes);
+  for (auto& candidate : all_candidates) {
+    // TODO(cylee|yusukes): Consider using IsImportant() instead of
+    // IsKernelKillable() for simplicity.
+    // TODO(cylee): Also consider protecting FOCUSED_TAB from the kernel OOM

[Yusuke Sato, 2017/05/11 18:50:16]

Added this to track.

[cylee1, 2017/05/11 19:29:14]

Done.

+    // killer so that Chrome and the kernel do the same regarding OOM handling.
+    if (!candidate.app() || candidate.app()->IsKernelKillable()) {
+      // Add tabs and killable apps to |candidates|.
+      candidates.emplace_back(std::move(candidate));
+    } else {
+      // Add non-killable apps to |apps_non_killable|.
+      apps_non_killable.emplace_back(std::move(candidate));
+    }
+  }
 
   // Now we assign priorities based on the sorted list. We're assigning
   // priorities in the range of kLowestRendererOomScore to
   // kHighestRendererOomScore (defined in chrome_constants.h). oom_score_adj
   // takes values from -1000 to 1000. Negative values are reserved for system
   // processes, and we want to give some room below the range we're using to
   // allow for things that want to be above the renderers in priority, so the
   // defined range gives us some variation in priority without taking up the
   // whole range. In the end, however, it's a pretty arbitrary range to use.
   // Higher values are more likely to be killed by the OOM killer.
 
   // Break the processes into 2 parts. This is to help lower the chance of
   // altering OOM score for many processes on any small change.
   int range_middle =
       (chrome::kLowestRendererOomScore + chrome::kHighestRendererOomScore) / 2;
 
   // 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.end();
   for (auto it = candidates.begin(); it != candidates.end(); ++it) {
     if (it->process_type() >= ProcessType::BACKGROUND_APP) {
       lower_priority_part = it;
       break;
     }
   }
 
   ProcessScoreMap new_map;
 
+  // Make the apps non-killable.
+  if (!apps_non_killable.empty())
+    SetOomScore(apps_non_killable, kLowestOomScore, &new_map);
+
   // Higher priority part.
   DistributeOomScoreInRange(candidates.begin(), lower_priority_part,
                             chrome::kLowestRendererOomScore, range_middle,
                             &new_map);
   // Lower priority part.
   DistributeOomScoreInRange(lower_priority_part, candidates.end(), range_middle,
                             chrome::kHighestRendererOomScore, &new_map);
+
   oom_score_map_.swap(new_map);
 }
 
+void TabManagerDelegate::SetOomScore(
+    const std::vector<TabManagerDelegate::Candidate>& candidates,
+    int score,
+    ProcessScoreMap* new_map) {
+  DistributeOomScoreInRange(candidates.begin(), candidates.end(), score, score,
+                            new_map);
+}
+
 void TabManagerDelegate::DistributeOomScoreInRange(
     std::vector<TabManagerDelegate::Candidate>::const_iterator begin,
     std::vector<TabManagerDelegate::Candidate>::const_iterator end,
     int range_begin,
     int range_end,
     ProcessScoreMap* new_map) {
   // Processes whose OOM scores should be updated. Ignore duplicated pids but
   // the last occurrence.
   std::map<base::ProcessHandle, int32_t> oom_scores_to_change;
 
   // 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 = (end - begin);
   const float priority_increment =
       static_cast<float>(range_end - range_begin) / num;
 
   float priority = range_begin;
   for (auto cur = begin; cur != end; ++cur) {
-    int score = static_cast<int>(priority + 0.5f);
+    const int score = round(priority);
 
     base::ProcessHandle pid = base::kNullProcessHandle;
     if (cur->app()) {
       pid = cur->app()->pid();
     } else {
       pid = 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
       // the same process, the process is set to an OOM score based on its "most
@@ skipping 12 matching lines @@
 
     // Need to update OOM score if the calculated score is different from
     // current cached score.
     if (oom_score_map_[pid] != score) {
       VLOG(3) << "Update OOM score " << score << " for " << *cur;
       oom_scores_to_change[pid] = static_cast<int32_t>(score);
     }
     priority += priority_increment;
   }
 
-  if (oom_scores_to_change.size())
+  if (oom_scores_to_change.size()) {

[Yusuke Sato, 2017/05/11 18:50:16]

I believe multi-line if-body requires {}.

[cylee1, 2017/05/11 19:29:14]

Acknowledged.

     GetDebugDaemonClient()->SetOomScoreAdj(
         oom_scores_to_change, base::Bind(&OnSetOomScoreAdj));
+  }
 }
 
 }  // namespace memory