Improve the about/discards UI, allowing each tab to be individually closed.

chrome/browser/ui/webui/about_ui.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/ui/webui/about_ui.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 11 matching lines @@
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread.h"
 #include "base/values.h"
 #include "chrome/browser/about_flags.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/memory/oom_priority_manager.h"
+#include "chrome/browser/memory/tab_stats.h"
 #include "chrome/browser/memory_details.h"
 #include "chrome/browser/net/predictor.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
@@ skipping 440 matching lines @@
   return WrapWithTR(row);
 }
 
 void AddContentSecurityPolicy(std::string* output) {
   output->append("<meta http-equiv='Content-Security-Policy' "
       "content='default-src 'none';'>");
 }
 
 // TODO(stevenjb): L10N AboutDiscards.
 
-std::string AboutDiscardsRun() {
+std::string BuildAboutDiscardsRunPage() {
   std::string output;
   AppendHeader(&output, 0, "About discards");
   output.append(
       base::StringPrintf("<meta http-equiv='refresh' content='2;%s'>",
-      chrome::kChromeUIDiscardsURL));
+                         chrome::kChromeUIDiscardsURL));
   AddContentSecurityPolicy(&output);
   output.append(WrapWithTag("p", "Discarding a tab..."));
-  g_browser_process->GetOomPriorityManager()->LogMemoryAndDiscardTab();
   AppendFooter(&output);
   return output;
 }
 
+std::string AboutDiscardsRun(int64 web_content_id) {

[Georges Khalil, 2015/07/21 18:13:37]

Let's give this a different name. How about AboutDiscardsRunSingleTab?

[proberge, 2015/07/21 18:29:07]

I don't think that's a good name for it either. Do we typically avoid function
overloading?

[Georges Khalil, 2015/07/21 18:38:29]

Yes, if possible. In that case, there's no reason to overload the name.

I'm thinking let's eliminate those two functions, they're only called from a
single place each. Inline them instead.

[proberge, 2015/07/21 18:55:09]

Done.

+  g_browser_process->GetOomPriorityManager()->DiscardTabById(web_content_id);
+  return BuildAboutDiscardsRunPage();
+}
+
+std::string AboutDiscardsRun() {
+  g_browser_process->GetOomPriorityManager()->DiscardTab();
+  return BuildAboutDiscardsRunPage();
+}
+
+std::vector<std::string> GetTabLines() {
+  const char kRunCommand[] = "run";
+  memory::OomPriorityManager* oom = g_browser_process->GetOomPriorityManager();
+  memory::TabStatsList stats = oom->GetTabStats();
+  std::vector<std::string> titles;
+  titles.reserve(stats.size());
+  memory::TabStatsList::iterator it = stats.begin();
+  for (; it != stats.end(); ++it) {
+    base::string16 str;
+    str.reserve(4096);
+    str += base::ASCIIToUTF16("<b>");
+    str += base::ASCIIToUTF16(it->is_app ? "[App] " : "");
+    str += base::ASCIIToUTF16(it->is_internal_page ? "[Internal] " : "");
+    str += base::ASCIIToUTF16(it->is_playing_audio ? "[Audio] " : "");
+    str += base::ASCIIToUTF16(it->is_pinned ? "[Pinned] " : "");
+    str += base::ASCIIToUTF16(it->is_discarded ? "[Discarded] " : "");
+    str += base::ASCIIToUTF16("</b>");
+    str += net::EscapeForHTML(it->title);
+#if defined(OS_CHROMEOS)
+    str += base::ASCIIToUTF16(base::StringPrintf(" %d ", it->oom_score));

[Georges Khalil, 2015/07/21 18:13:37]

Let's wrap this in parenthesis for better clarity.

[proberge, 2015/07/21 18:29:07]

I don't understand what to wrap - this is an #if, not an if() {}.

[Georges Khalil, 2015/07/21 18:38:29]

Should have been more specific. Let's wrap the score in parenthesis. So that
when it's displayed, it looks less part of the title.

[proberge, 2015/07/21 18:55:09]

Done.

+#endif
+    if (!it->is_discarded) {
+      str += base::ASCIIToUTF16(
+          base::StringPrintf(" <a href='%s%s/%ld'>Discard</a>",
+                             chrome::kChromeUIDiscardsURL,
+                             kRunCommand,
+                             it->tab_contents_id));
+    }
+    titles.push_back(base::UTF16ToUTF8((str)));
+  }
+  return titles;
+}
+
 std::string AboutDiscards(const std::string& path) {
   std::string output;
+  std::vector<std::string> path_split;
+  int64 web_content_id;
   const char kRunCommand[] = "run";

[Georges Khalil, 2015/07/21 18:13:37]

Let's dedupe this and put it higher up and give it a more significant name.
kAboutDiscardsRunCommand?

[proberge, 2015/07/21 18:29:07]

Done.

-  if (path == kRunCommand)
+
+  base::SplitString(path, '/', &path_split);
+  if (path_split.size() == 2 && path_split[0] == kRunCommand &&
+      base::StringToInt64(path_split[1], &web_content_id)) {
+    return AboutDiscardsRun(web_content_id);
+  } else if (path_split.size() == 1 && path_split[0] == kRunCommand) {
     return AboutDiscardsRun();
+  }
+
   AppendHeader(&output, 0, "About discards");
   AddContentSecurityPolicy(&output);
   AppendBody(&output);
-  output.append("<h3>About discards</h3>");
+  output.append("<h3>Discarded Tabs</h3>");
   output.append(
       "<p>Tabs sorted from most interesting to least interesting. The least "
       "interesting tab may be discarded if we run out of physical memory.</p>");
 
   memory::OomPriorityManager* oom = g_browser_process->GetOomPriorityManager();
-  std::vector<base::string16> titles = oom->GetTabTitles();
+  std::vector<std::string> titles = GetTabLines();
   if (!titles.empty()) {
     output.append("<ul>");
-    std::vector<base::string16>::iterator it = titles.begin();
+    std::vector<std::string>::iterator it = titles.begin();
     for ( ; it != titles.end(); ++it) {
-      std::string title = base::UTF16ToUTF8(*it);
-      title = net::EscapeForHTML(title);
-      output.append(WrapWithTag("li", title));
+      output.append(WrapWithTag("li", *it));
     }
     output.append("</ul>");
   } else {
     output.append("<p>None found.  Wait 10 seconds, then refresh.</p>");
   }
-  output.append(base::StringPrintf("%d discards this session. ",
-                             oom->discard_count()));
+  output.append(
+      base::StringPrintf("%d discards this session. ", oom->discard_count()));
   output.append(base::StringPrintf("<a href='%s%s'>Discard tab now</a>",
-                                   chrome::kChromeUIDiscardsURL,
-                                   kRunCommand));
+                                   chrome::kChromeUIDiscardsURL, kRunCommand));
+
 #if defined(OS_CHROMEOS)
   base::SystemMemoryInfoKB meminfo;
   base::GetSystemMemoryInfo(&meminfo);
   output.append("<h3>System memory information in MB</h3>");
   output.append("<table>");
   // Start with summary statistics.
   output.append(AddStringRow(
       "Total", base::IntToString(meminfo.total / 1024)));
   output.append(AddStringRow(
       "Free", base::IntToString(meminfo.free / 1024)));
@@ skipping 439 matching lines @@
   Profile* profile = Profile::FromWebUI(web_ui);
 
 #if defined(ENABLE_THEMES)
   // Set up the chrome://theme/ source.
   ThemeSource* theme = new ThemeSource(profile);
   content::URLDataSource::Add(profile, theme);
 #endif
 
   content::URLDataSource::Add(profile, new AboutUIHTMLSource(name, profile));
 }