From e80b887059a487f560e4c0408f1cf0c480461d9c Mon Sep 17 00:00:00 2001
From: TheFightingCatfish <seekingblues@gmail.com>
Date: Sat, 7 Aug 2021 04:28:22 +0800
Subject: [PATCH] Browser+LibWeb: Make sure the default favicon is loaded

Previously in Browser, when we navigate back from a page that has an
icon to a page that does not have an icon, the icon does not update and
the old icon is displayed because FrameLoader does not set the default
favicon when the favicon cannot be loaded. This patch ensures that
Browser receives a new icon bitmap every time a load takes place.
---
 .../Applications/Browser/BrowserWindow.cpp    |  4 --
 .../Libraries/LibWeb/Loader/FrameLoader.cpp   | 39 ++++++++++++++-----
 .../Libraries/LibWeb/Loader/FrameLoader.h     |  1 +
 3 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/Userland/Applications/Browser/BrowserWindow.cpp b/Userland/Applications/Browser/BrowserWindow.cpp
index 4f2f3e6d91..4aafd3aea3 100644
--- a/Userland/Applications/Browser/BrowserWindow.cpp
+++ b/Userland/Applications/Browser/BrowserWindow.cpp
@@ -515,10 +515,6 @@ void BrowserWindow::create_new_tab(URL url, bool activate)
 
     m_tab_widget->set_bar_visible(!is_fullscreen() && m_tab_widget->children().size() > 1);
 
-    auto default_favicon = Gfx::Bitmap::try_load_from_file("/res/icons/16x16/filetype-html.png");
-    VERIFY(default_favicon);
-    m_tab_widget->set_tab_icon(new_tab, default_favicon);
-
     new_tab.on_title_change = [this, &new_tab](auto& title) {
         m_tab_widget->set_tab_title(new_tab, title);
         if (m_tab_widget->active_widget() == &new_tab)
diff --git a/Userland/Libraries/LibWeb/Loader/FrameLoader.cpp b/Userland/Libraries/LibWeb/Loader/FrameLoader.cpp
index eedb7e99c7..99650330e6 100644
--- a/Userland/Libraries/LibWeb/Loader/FrameLoader.cpp
+++ b/Userland/Libraries/LibWeb/Loader/FrameLoader.cpp
@@ -22,9 +22,15 @@
 
 namespace Web {
 
+static RefPtr<Gfx::Bitmap> s_default_favicon_bitmap;
+
 FrameLoader::FrameLoader(BrowsingContext& browsing_context)
     : m_browsing_context(browsing_context)
 {
+    if (!s_default_favicon_bitmap) {
+        s_default_favicon_bitmap = Gfx::Bitmap::try_load_from_file("/res/icons/16x16/filetype-html.png");
+        VERIFY(s_default_favicon_bitmap);
+    }
 }
 
 FrameLoader::~FrameLoader()
@@ -166,21 +172,24 @@ bool FrameLoader::load(const LoadRequest& request, Type type)
             favicon_url,
             [this, favicon_url](auto data, auto&, auto) {
                 dbgln("Favicon downloaded, {} bytes from {}", data.size(), favicon_url);
+                RefPtr<Gfx::Bitmap> favicon_bitmap;
                 auto decoder = Gfx::ImageDecoder::try_create(data);
                 if (!decoder) {
                     dbgln("No image decoder plugin for favicon {}", favicon_url);
-                    return;
+                } else {
+                    favicon_bitmap = decoder->frame(0).image;
+                    if (!favicon_bitmap)
+                        dbgln("Could not decode favicon {}", favicon_url);
+                    else
+                        dbgln("Decoded favicon, {}", favicon_bitmap->size());
                 }
-                auto frame = decoder->frame(0);
-                auto bitmap = frame.image;
-                if (!bitmap) {
-                    dbgln("Could not decode favicon {}", favicon_url);
-                    return;
-                }
-                dbgln("Decoded favicon, {}", bitmap->size());
-                if (auto* page = browsing_context().page())
-                    page->client().page_did_change_favicon(*bitmap);
+                load_favicon(favicon_bitmap);
+            },
+            [this](auto&, auto) {
+                load_favicon();
             });
+    } else {
+        load_favicon();
     }
 
     return true;
@@ -232,6 +241,16 @@ void FrameLoader::load_error_page(const URL& failed_url, const String& error)
         });
 }
 
+void FrameLoader::load_favicon(RefPtr<Gfx::Bitmap> bitmap)
+{
+    if (auto* page = browsing_context().page()) {
+        if (bitmap)
+            page->client().page_did_change_favicon(*bitmap);
+        else
+            page->client().page_did_change_favicon(*s_default_favicon_bitmap);
+    }
+}
+
 void FrameLoader::resource_did_load()
 {
     auto url = resource()->url();
diff --git a/Userland/Libraries/LibWeb/Loader/FrameLoader.h b/Userland/Libraries/LibWeb/Loader/FrameLoader.h
index 2cd98a94a7..993025d826 100644
--- a/Userland/Libraries/LibWeb/Loader/FrameLoader.h
+++ b/Userland/Libraries/LibWeb/Loader/FrameLoader.h
@@ -40,6 +40,7 @@ private:
     virtual void resource_did_fail() override;
 
     void load_error_page(const URL& failed_url, const String& error_message);
+    void load_favicon(RefPtr<Gfx::Bitmap> bitmap = nullptr);
     bool parse_document(DOM::Document&, const ByteBuffer& data);
 
     BrowsingContext& m_browsing_context;