[Android] Use same favicon for chrome://history and chrome-native://history

This CL removes the special favicon for chrome://history to match
chrome-native://history. This pattern matches the behaviour of
chrome://bookmarks which also uses a native page.

BUG=1104811
TEST=HistoryTest.*

Change-Id: I152bd7cf75570de914013560845e64ea47b18e18
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2335650
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: dpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799273}

chrome/android/chrome_test_java_sources.gni

@@ -216,6 +216,7 @@ chrome_test_java_sources = [
   "javatests/src/org/chromium/chrome/browser/hardware_acceleration/WebappActivityHWATest.java",
   "javatests/src/org/chromium/chrome/browser/history/HistoryActivityScrollingTest.java",
   "javatests/src/org/chromium/chrome/browser/history/HistoryActivityTest.java",
+  "javatests/src/org/chromium/chrome/browser/history/HistoryTest.java",
   "javatests/src/org/chromium/chrome/browser/homepage/HomepagePolicyIntegrationTest.java",
   "javatests/src/org/chromium/chrome/browser/homepage/HomepageTestRule.java",
   "javatests/src/org/chromium/chrome/browser/homepage/settings/HomepageSettingsFragmentTest.java",

chrome/android/javatests/src/org/chromium/chrome/browser/history/HistoryTest.java

+// Copyright 2020 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.
+
+package org.chromium.chrome.browser.history;
+
+import static org.junit.Assert.assertNull;
+
+import android.graphics.Bitmap;
+import android.support.test.filters.SmallTest;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.chromium.base.test.util.CallbackHelper;
+import org.chromium.base.test.util.CommandLineFlags;
+import org.chromium.chrome.browser.flags.ChromeSwitches;
+import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.ui.favicon.FaviconHelper;
+import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
+import org.chromium.components.embedder_support.util.UrlConstants;
+import org.chromium.content_public.browser.test.util.TestThreadUtils;
+
+import java.util.concurrent.TimeoutException;
+
+/**
+ * Tests for history feature.
+ */
+@RunWith(ChromeJUnit4ClassRunner.class)
+@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
+public class HistoryTest {
+    private static class FaviconWaiter
+            extends CallbackHelper implements FaviconHelper.FaviconImageCallback {
+        private Bitmap mFavicon;
+
+        @Override
+        public void onFaviconAvailable(Bitmap image, String iconUrl) {
+            mFavicon = image;
+            notifyCalled();
+        }
+
+        public Bitmap waitForFavicon() throws TimeoutException {
+            waitForFirst();
+            return mFavicon;
+        }
+    }
+
+    @Rule
+    public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
+
+    /**
+     * Check that the favicons for {@link UrlConstants#HISTORY_URL} and for
+     * {@link UrlConstants#NATIVE_HISTORY_URL} are identical.
+     */
+    @Test
+    @SmallTest
+    public void testFavicon() throws Exception {
+        mActivityTestRule.startMainActivityOnBlankPage();
+
+        FaviconHelper helper = TestThreadUtils.runOnUiThreadBlocking(FaviconHelper::new);
+        // If the returned favicons are non-null Bitmap#sameAs() should be used.
+        assertNull(getFavicon(helper, UrlConstants.HISTORY_URL));
+        assertNull(getFavicon(helper, UrlConstants.NATIVE_HISTORY_URL));
+    }
+
+    public Bitmap getFavicon(FaviconHelper helper, String pageUrl) throws TimeoutException {
+        FaviconWaiter waiter = new FaviconWaiter();
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            helper.getLocalFaviconImageForURL(
+                    Profile.getLastUsedRegularProfile(), pageUrl, 0, waiter);
+        });
+        return waiter.waitForFavicon();
+    }
+}

chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc

@@ -1045,11 +1045,6 @@ base::RefCountedMemory* ChromeWebUIControllerFactory::GetFaviconResourceBytes(
   if (page_url.host_piece() == chrome::kChromeUIFlagsHost)
     return FlagsUI::GetFaviconResourceBytes(scale_factor);
 
-  if (page_url.host_piece() == chrome::kChromeUIHistoryHost) {
-    return ui::ResourceBundle::GetSharedInstance()
-        .LoadDataResourceBytesForScale(IDR_HISTORY_FAVICON, scale_factor);
-  }
-
 #if !defined(OS_ANDROID)
 #if !defined(OS_CHROMEOS)
   // The Apps launcher page is not available on android or ChromeOS.
@@ -1061,6 +1056,9 @@ base::RefCountedMemory* ChromeWebUIControllerFactory::GetFaviconResourceBytes(
   if (page_url.host_piece() == chrome::kChromeUIBookmarksHost)
     return BookmarksUI::GetFaviconResourceBytes(scale_factor);
 
+  if (page_url.host_piece() == chrome::kChromeUIHistoryHost)
+    return HistoryUI::GetFaviconResourceBytes(scale_factor);
+
   // Android uses the native download manager.
   if (page_url.host_piece() == chrome::kChromeUIDownloadsHost)
     return DownloadsUI::GetFaviconResourceBytes(scale_factor);

chrome/browser/ui/webui/history/history_ui.cc

@@ -12,6 +12,7 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
+#include "base/memory/ref_counted_memory.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/browser/profiles/profile.h"
@@ -29,6 +30,7 @@
 #include "chrome/grit/history_resources.h"
 #include "chrome/grit/history_resources_map.h"
 #include "chrome/grit/locale_settings.h"
+#include "components/grit/components_scaled_resources.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
@@ -36,6 +38,7 @@
 #include "content/public/browser/web_ui.h"
 #include "content/public/browser/web_ui_data_source.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/resource/resource_bundle.h"
 #include "ui/base/webui/web_ui_util.h"
 
 namespace {
@@ -179,6 +182,14 @@ void HistoryUI::RegisterProfilePrefs(
       user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
 }
 
+// static
+base::RefCountedMemory* HistoryUI::GetFaviconResourceBytes(
+    ui::ScaleFactor scale_factor) {
+  return static_cast<base::RefCountedMemory*>(
+      ui::ResourceBundle::GetSharedInstance().LoadDataResourceBytesForScale(
+          IDR_HISTORY_FAVICON, scale_factor));
+}
+
 void HistoryUI::UpdateDataSource() {
   CHECK(web_ui());
 

chrome/browser/ui/webui/history/history_ui.h

@@ -8,9 +8,11 @@
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "content/public/browser/web_ui_controller.h"
+#include "ui/base/layout.h"
 
 namespace base {
 class ListValue;
+class RefCountedMemory;
 }
 
 namespace user_prefs {
@@ -24,6 +26,9 @@ class HistoryUI : public content::WebUIController {
 
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 
+  static base::RefCountedMemory* GetFaviconResourceBytes(
+      ui::ScaleFactor scale_factor);
+
  private:
   void UpdateDataSource();
 

components/resources/components_scaled_resources.grd

@@ -20,7 +20,9 @@
       <part file="version_ui_scaled_resources.grdp" />
 
       <!-- Generic resources -->
-      <structure type="chrome_scaled_image" name="IDR_HISTORY_FAVICON" file="favicon_history.png" />
+      <if expr="not is_android">
+        <structure type="chrome_scaled_image" name="IDR_HISTORY_FAVICON" file="favicon_history.png" />
+      </if>
       <structure type="chrome_scaled_image" name="IDR_SAD_WEBVIEW" file="webview-crash.png" />
       <structure type="chrome_scaled_image" name="IDR_SAD_PLUGIN" file="sadplugin.png" />
     </structures>