Remove direct favicon loading in chrome://discards

Load the favicons shown in chrome://discards via cr.icon instead of
doing a direct network request from WebUI.

Remove LifecycleUnit::GetIconURL() which is now unused.

Bug: 837420
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I0ce123cdf944ace2e03b2b56feb18402848f875d
Reviewed-on: https://chromium-review.googlesource.com/1031783
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555291}

chrome/browser/resource_coordinator/discard_metrics_lifecycle_unit_observer_unittest.cc

@@ -45,7 +45,6 @@ class DummyLifecycleUnit : public LifecycleUnitBase {
     return nullptr;
   }
   base::string16 GetTitle() const override { return base::string16(); }
-  std::string GetIconURL() const override { return std::string(); }
   base::ProcessHandle GetProcessHandle() const override {
     return base::ProcessHandle();
   }

chrome/browser/resource_coordinator/lifecycle_unit.h

@@ -6,7 +6,6 @@
 #define CHROME_BROWSER_RESOURCE_COORDINATOR_LIFECYCLE_UNIT_H_
 
 #include <stdint.h>
-#include <string>
 #include <vector>
 
 #include "base/containers/flat_set.h"
@@ -61,10 +60,6 @@ class LifecycleUnit {
   // title is available.
   virtual base::string16 GetTitle() const = 0;
 
-  // Returns the URL of an icon for this LifecycleUnit, or an empty string if no
-  // icon is available.
-  virtual std::string GetIconURL() const = 0;
-
   // Returns the process hosting this LifecycleUnit. Used to distribute OOM
   // scores.
   //

chrome/browser/resource_coordinator/lifecycle_unit_base_unittest.cc

@@ -42,7 +42,6 @@ class DummyLifecycleUnit : public LifecycleUnitBase {
     return nullptr;
   }
   base::string16 GetTitle() const override { return base::string16(); }
-  std::string GetIconURL() const override { return std::string(); }
   base::ProcessHandle GetProcessHandle() const override {
     return base::ProcessHandle();
   }

chrome/browser/resource_coordinator/tab_lifecycle_unit.cc

@@ -13,7 +13,6 @@
 #include "chrome/browser/resource_coordinator/tab_lifecycle_observer.h"
 #include "chrome/browser/resource_coordinator/time.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
-#include "content/public/browser/favicon_status.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/render_frame_host.h"
@@ -86,15 +85,6 @@ base::string16 TabLifecycleUnitSource::TabLifecycleUnit::GetTitle() const {
   return GetWebContents()->GetTitle();
 }
 
-std::string TabLifecycleUnitSource::TabLifecycleUnit::GetIconURL() const {
-  auto* last_committed_entry =
-      GetWebContents()->GetController().GetLastCommittedEntry();
-  if (!last_committed_entry)
-    return std::string();
-  const auto& favicon = last_committed_entry->GetFavicon();
-  return favicon.valid ? favicon.url.spec() : std::string();
-}
-
 base::ProcessHandle TabLifecycleUnitSource::TabLifecycleUnit::GetProcessHandle()
     const {
   content::RenderFrameHost* main_frame = GetWebContents()->GetMainFrame();

chrome/browser/resource_coordinator/tab_lifecycle_unit.h

@@ -69,7 +69,6 @@ class TabLifecycleUnitSource::TabLifecycleUnit
   // LifecycleUnit:
   TabLifecycleUnitExternal* AsTabLifecycleUnitExternal() override;
   base::string16 GetTitle() const override;
-  std::string GetIconURL() const override;
   base::ProcessHandle GetProcessHandle() const override;
   SortKey GetSortKey() const override;
   content::Visibility GetVisibility() const override;

chrome/browser/resource_coordinator/tab_manager_delegate_chromeos_unittest.cc

@@ -41,7 +41,6 @@ class DummyLifecycleUnit : public LifecycleUnitBase {
 
   // LifecycleUnit:
   base::string16 GetTitle() const override { return base::string16(); }
-  std::string GetIconURL() const override { return std::string(); }
   base::ProcessHandle GetProcessHandle() const override {
     return process_handle_;
   }

chrome/browser/resources/discards/discards.css

@@ -37,19 +37,11 @@ table td div.title-cell-container {
 table td div.favicon-div {
   height: 16px;
   margin: 3px;
-  padding: 0;
-  width: 16px;
-}
-
-table td div.favicon-div img {
-  height: 16px;
-  width: 16px;
+  min-width: 16px;
 }
 
 table td div.title-div {
-  margin: 0;
   overflow: hidden;
-  padding: 0;
   white-space: nowrap;
 }
 

chrome/browser/resources/discards/discards.html

@@ -14,6 +14,7 @@ general use and is not localized.
     <link rel="stylesheet" href="chrome://resources/css/action_link.css">
     <link rel="stylesheet" href="chrome://resources/css/text_defaults.css">
     <script src="chrome://resources/js/cr.js"></script>
+    <script src="chrome://resources/js/icon.js"></script>
     <script src="chrome://resources/js/mojo_bindings.js"></script>
     <script src="chrome://resources/js/util.js"></script>
     <script src="chrome/browser/ui/webui/discards/discards.mojom.js"></script>
@@ -50,9 +51,7 @@ general use and is not localized.
         <td class="utility-rank-cell"></td>
         <td class="title-cell">
           <div class="title-cell-container">
-            <div class="favicon-div">
-              <img class="favicon" alt="FavIcon">
-            </div>
+            <div class="favicon-div"></div>
             <div class="title-div"></div>
           </div>
         </td>

chrome/browser/resources/discards/discards.js

@@ -274,8 +274,8 @@ cr.define('discards', function() {
     // Update the content.
     row.querySelector('.utility-rank-cell').textContent =
         info.utilityRank.toString();
-    row.querySelector('.favicon').src =
-        info.faviconUrl ? info.faviconUrl : 'chrome://favicon';
+    row.querySelector('.favicon-div').style.backgroundImage =
+        cr.icon.getFavicon(info.tabUrl);
     row.querySelector('.title-div').textContent = info.title;
     row.querySelector('.tab-url-cell').textContent = info.tabUrl;
     row.querySelector('.visibility-cell').textContent =

chrome/browser/ui/webui/discards/discards.mojom

@@ -16,8 +16,6 @@ struct TabDiscardsInfo {
   // The URL associated with the tab. This corresponds to GetLastCommittedURL,
   // and is also what is visible in the Omnibox for a given tab.
   string tab_url;
-  // The URL of the favicon being displayed for a tab.
-  string favicon_url;
   // The title of the tab, as displayed on the tab itself.
   string title;
   // The visibility of the LifecycleUnit.

chrome/browser/ui/webui/discards/discards_ui.cc

@@ -83,9 +83,6 @@ class DiscardsDetailsProviderImpl : public mojom::DiscardsDetailsProvider {
           tab_lifecycle_unit_external->GetWebContents();
 
       info->tab_url = contents->GetLastCommittedURL().spec();
-      // This can be empty for pages without a favicon. The WebUI takes care of
-      // showing the chrome://favicon default in that case.
-      info->favicon_url = lifecycle_unit->GetIconURL();
       info->title = base::UTF16ToUTF8(lifecycle_unit->GetTitle());
       info->visibility =
           GetLifecycleUnitVisibility(lifecycle_unit->GetVisibility());

extensions/browser/api/web_request/web_request_permissions.cc

@@ -90,9 +90,7 @@ bool IsWebUIAllowedToMakeNetworkRequests(const url::Origin& origin) {
       // https://crbug.com/831812
       origin.host() == "sync-confirmation" ||
       // https://crbug.com/831813
-      origin.host() == "inspect" ||
-      // https://crbug.com/837420
-      origin.host() == "discards";
+      origin.host() == "inspect";
 }
 
 }  // namespace