LibWebView: Persist page metadata in history

Record visits as soon as a page produces useful metadata such as a
title or favicon so pages that never finish loading still become
autocomplete candidates.

Store favicons in the history schema from the start instead of
introducing an upgrade path inside this series, and cover persisted
metadata behavior in TestHistoryStore.

Libraries/LibWebView/HistoryStore.cpp

@@ -125,6 +125,7 @@ ErrorOr<NonnullOwnPtr<HistoryStore>> HistoryStore::create(Database::Database& da
         CREATE TABLE IF NOT EXISTS History (
             url TEXT PRIMARY KEY,
             title TEXT NOT NULL,
+            favicon TEXT,
             visit_count INTEGER NOT NULL,
             last_visited_time INTEGER NOT NULL
         );
@@ -153,8 +154,13 @@ ErrorOr<NonnullOwnPtr<HistoryStore>> HistoryStore::create(Database::Database& da
         SET title = ?
         WHERE url = ?;
     )#"sv));
+    statements.update_favicon = TRY(database.prepare_statement(R"#(
+        UPDATE History
+        SET favicon = ?
+        WHERE url = ?;
+    )#"sv));
     statements.get_entry = TRY(database.prepare_statement(R"#(
-        SELECT title, visit_count, last_visited_time
+        SELECT title, visit_count, last_visited_time, COALESCE(favicon, '')
         FROM History
         WHERE url = ?;
     )#"sv));
@@ -292,6 +298,28 @@ void HistoryStore::update_title(URL::URL const& url, String const& title)
         m_transient_storage.update_title(*normalized_url, title);
 }
 
+void HistoryStore::update_favicon(URL::URL const& url, String const& favicon_base64_png)
+{
+    if (favicon_base64_png.is_empty()) {
+        dbgln_if(WEBVIEW_HISTORY_DEBUG, "[History] Ignoring empty history favicon update for {}", url);
+        return;
+    }
+
+    auto normalized_url = normalize_url(url);
+    if (!normalized_url.has_value())
+        return;
+
+    dbgln_if(WEBVIEW_HISTORY_DEBUG, "[History] Updating history favicon in {} store: url='{}' bytes={}",
+        m_persisted_storage.has_value() ? "SQL"sv : "transient"sv,
+        *normalized_url,
+        favicon_base64_png.bytes().size());
+
+    if (m_persisted_storage.has_value())
+        m_persisted_storage->update_favicon(*normalized_url, favicon_base64_png);
+    else
+        m_transient_storage.update_favicon(*normalized_url, favicon_base64_png);
+}
+
 Optional<HistoryEntry> HistoryStore::entry_for_url(URL::URL const& url)
 {
     if (m_is_disabled)
@@ -306,11 +334,12 @@ Optional<HistoryEntry> HistoryStore::entry_for_url(URL::URL const& url)
         : m_transient_storage.entry_for_url(*normalized_url);
 
     if (entry.has_value()) {
-        dbgln_if(WEBVIEW_HISTORY_DEBUG, "[History] Found history entry for '{}': title='{}' visits={} last_visited={}",
+        dbgln_if(WEBVIEW_HISTORY_DEBUG, "[History] Found history entry for '{}': title='{}' visits={} last_visited={} has_favicon={}",
             entry->url,
             entry->title.has_value() ? entry->title->bytes_as_string_view() : "<none>"sv,
             entry->visit_count,
-            entry->last_visited_time.seconds_since_epoch());
+            entry->last_visited_time.seconds_since_epoch(),
+            entry->favicon_base64_png.has_value());
     } else {
         dbgln_if(WEBVIEW_HISTORY_DEBUG, "[History] No history entry found for '{}'", *normalized_url);
     }
@@ -380,6 +409,7 @@ void HistoryStore::TransientStorage::record_visit(String url, Optional<String> t
         auto new_entry = HistoryEntry {
             .url = url,
             .title = move(title),
+            .favicon_base64_png = {},
             .visit_count = 1,
             .last_visited_time = visited_at,
         };
@@ -404,6 +434,15 @@ void HistoryStore::TransientStorage::update_title(String const& url, String titl
     entry->value.title = move(title);
 }
 
+void HistoryStore::TransientStorage::update_favicon(String const& url, String favicon_base64_png)
+{
+    auto entry = m_entries.find(url);
+    if (entry == m_entries.end())
+        return;
+
+    entry->value.favicon_base64_png = move(favicon_base64_png);
+}
+
 Optional<HistoryEntry> HistoryStore::TransientStorage::entry_for_url(String const& url)
 {
     auto entry = m_entries.get(url);
@@ -464,6 +503,15 @@ void HistoryStore::PersistedStorage::update_title(String const& url, String cons
         url);
 }
 
+void HistoryStore::PersistedStorage::update_favicon(String const& url, String const& favicon_base64_png)
+{
+    database.execute_statement(
+        statements.update_favicon,
+        {},
+        favicon_base64_png,
+        url);
+}
+
 Optional<HistoryEntry> HistoryStore::PersistedStorage::entry_for_url(String const& url)
 {
     Optional<HistoryEntry> entry;
@@ -472,10 +520,12 @@ Optional<HistoryEntry> HistoryStore::PersistedStorage::entry_for_url(String cons
         statements.get_entry,
         [&](auto statement_id) {
             auto title = database.result_column<String>(statement_id, 0);
+            auto favicon = database.result_column<String>(statement_id, 3);
 
             entry = HistoryEntry {
                 .url = url,
                 .title = title.is_empty() ? Optional<String> {} : Optional<String> { move(title) },
+                .favicon_base64_png = favicon.is_empty() ? Optional<String> {} : Optional<String> { move(favicon) },
                 .visit_count = database.result_column<u64>(statement_id, 1),
                 .last_visited_time = database.result_column<UnixDateTime>(statement_id, 2),
             };

Libraries/LibWebView/HistoryStore.h

@@ -20,6 +20,7 @@ namespace WebView {
 struct WEBVIEW_API HistoryEntry {
     String url;
     Optional<String> title;
+    Optional<String> favicon_base64_png;
     u64 visit_count { 0 };
     UnixDateTime last_visited_time;
 };
@@ -37,6 +38,7 @@ public:
 
     void record_visit(URL::URL const&, Optional<String> title = {}, UnixDateTime visited_at = UnixDateTime::now());
     void update_title(URL::URL const&, String const& title);
+    void update_favicon(URL::URL const&, String const& favicon_base64_png);
 
     Optional<HistoryEntry> entry_for_url(URL::URL const&);
     Vector<String> autocomplete_suggestions(StringView query, size_t limit = 8);
@@ -48,6 +50,7 @@ private:
     struct Statements {
         Database::StatementID upsert_entry { 0 };
         Database::StatementID update_title { 0 };
+        Database::StatementID update_favicon { 0 };
         Database::StatementID get_entry { 0 };
         Database::StatementID search_entries { 0 };
         Database::StatementID clear_entries { 0 };
@@ -58,6 +61,7 @@ private:
     public:
         void record_visit(String url, Optional<String> title, UnixDateTime visited_at);
         void update_title(String const& url, String title);
+        void update_favicon(String const& url, String favicon_base64_png);
 
         Optional<HistoryEntry> entry_for_url(String const& url);
         Vector<String> autocomplete_suggestions(StringView title_query, StringView url_query, size_t limit);
@@ -72,6 +76,7 @@ private:
     struct PersistedStorage {
         void record_visit(String const& url, Optional<String> const& title, UnixDateTime visited_at);
         void update_title(String const& url, String const& title);
+        void update_favicon(String const& url, String const& favicon_base64_png);
 
         Optional<HistoryEntry> entry_for_url(String const& url);
         Vector<String> autocomplete_suggestions(StringView title_query, StringView url_query, size_t limit);

Libraries/LibWebView/ViewImplementation.cpp

@@ -18,6 +18,7 @@
 #include <LibWebView/Application.h>
 #include <LibWebView/BookmarkStore.h>
 #include <LibWebView/HelperProcess.h>
+#include <LibWebView/HistoryStore.h>
 #include <LibWebView/Menu.h>
 #include <LibWebView/URL.h>
 #include <LibWebView/UserAgent.h>
@@ -111,8 +112,10 @@ void ViewImplementation::set_favicon(Badge<WebContentClient>, Gfx::Bitmap const&
             m_favicon_base64_png = favicon_base64_png.release_value();
     }
 
-    if (m_favicon_base64_png.has_value())
+    if (m_favicon_base64_png.has_value()) {
         Application::bookmark_store().update_favicon(m_url, *m_favicon_base64_png);
+        Application::history_store().update_favicon(m_url, *m_favicon_base64_png);
+    }
 
     if (on_favicon_change)
         on_favicon_change(favicon);

Libraries/LibWebView/WebContentClient.cpp

@@ -65,11 +65,13 @@ void WebContentClient::register_view(u64 page_id, ViewImplementation& view)
 {
     VERIFY(page_id > 0);
     m_views.set(page_id, view);
+    m_history_recorded_urls_for_current_load.remove(page_id);
 }
 
 void WebContentClient::unregister_view(u64 page_id)
 {
     m_views.remove(page_id);
+    m_history_recorded_urls_for_current_load.remove(page_id);
     if (m_views.is_empty())
         async_close_server();
 }
@@ -113,11 +115,32 @@ void WebContentClient::did_request_new_process_for_navigation(u64 page_id, URL::
         view->create_new_process_for_cross_site_navigation(url);
 }
 
+void WebContentClient::maybe_record_history_visit_for_current_load(u64 page_id, URL::URL const& url, Optional<String> title, StringView reason)
+{
+    auto normalized_url = HistoryStore::normalize_url(url);
+    if (!normalized_url.has_value())
+        return;
+
+    if (auto recorded_url = m_history_recorded_urls_for_current_load.get(page_id); recorded_url.has_value() && *recorded_url == *normalized_url) {
+        dbgln_if(WEBVIEW_HISTORY_DEBUG, "[History] Visit for page {} at '{}' was already recorded during this load before {}", page_id, *normalized_url, reason);
+        return;
+    }
+
+    dbgln_if(WEBVIEW_HISTORY_DEBUG, "[History] Recording history visit for page {} at '{}' after {}", page_id, *normalized_url, reason);
+
+    // Title and favicon updates already give us a useful history entry, so
+    // do not wait for did_finish_loading() on pages that never reach it.
+    Application::history_store().record_visit(url, move(title));
+    m_history_recorded_urls_for_current_load.set(page_id, normalized_url.release_value());
+}
+
 void WebContentClient::did_start_loading(u64 page_id, URL::URL url, bool is_redirect)
 {
     if (auto process = WebView::Application::the().find_process(m_process_handle.pid); process.has_value())
         process->set_title(OptionalNone {});
 
+    m_history_recorded_urls_for_current_load.remove(page_id);
+
     if (auto view = view_for_page_id(page_id); view.has_value()) {
         view->set_url({}, url);
 
@@ -149,7 +172,11 @@ void WebContentClient::did_finish_loading(u64 page_id, URL::URL url)
             url,
             title.has_value() ? title->bytes_as_string_view() : "<none>"sv);
 
-        Application::history_store().record_visit(url, move(title));
+        maybe_record_history_visit_for_current_load(page_id, url, title, "load finish"sv);
+        if (title.has_value())
+            Application::history_store().update_title(url, *title);
+        if (view->favicon_base64_png().has_value())
+            Application::history_store().update_favicon(url, *view->favicon_base64_png());
 
         if (view->on_load_finish)
             view->on_load_finish(url);
@@ -230,6 +257,7 @@ void WebContentClient::did_change_title(u64 page_id, Utf16String title)
         if (!title.is_empty()) {
             auto title_utf8 = title.to_utf8();
 
+            maybe_record_history_visit_for_current_load(page_id, view->url(), title_utf8, "title change"sv);
             dbgln_if(WEBVIEW_HISTORY_DEBUG, "[History] Title changed for page {} at '{}' to '{}'",
                 page_id,
                 view->url(),
@@ -556,8 +584,10 @@ void WebContentClient::did_change_favicon(u64 page_id, Gfx::ShareableBitmap favi
         return;
     }
 
-    if (auto view = view_for_page_id(page_id); view.has_value())
+    if (auto view = view_for_page_id(page_id); view.has_value()) {
+        maybe_record_history_visit_for_current_load(page_id, view->url(), history_title(view->title(), view->url()), "favicon change"sv);
         view->set_favicon({}, *favicon.bitmap());
+    }
 }
 
 void WebContentClient::did_request_document_cookie_version_index(u64 page_id, i64 document_id, String domain)

Libraries/LibWebView/WebContentClient.h

@@ -8,7 +8,10 @@
 
 #include <AK/HashMap.h>
 #include <AK/NonnullRawPtr.h>
+#include <AK/Optional.h>
 #include <AK/SourceLocation.h>
+#include <AK/String.h>
+#include <AK/StringView.h>
 #include <LibGfx/SharedImage.h>
 #include <LibHTTP/Header.h>
 #include <LibIPC/ConnectionToServer.h>
@@ -60,6 +63,8 @@ public:
     void set_pid(pid_t pid) { m_process_handle.pid = pid; }
 
 private:
+    void maybe_record_history_visit_for_current_load(u64 page_id, URL::URL const&, Optional<String> title, StringView reason);
+
     virtual void die() override;
 
     virtual void did_paint(u64 page_id, Gfx::IntRect, i32) override;
@@ -154,6 +159,7 @@ private:
     Optional<ViewImplementation&> view_for_page_id(u64, SourceLocation = SourceLocation::current());
 
     HashMap<u64, NonnullRawPtr<ViewImplementation>> m_views;
+    HashMap<u64, String> m_history_recorded_urls_for_current_load;
 
     ProcessHandle m_process_handle;
 

Tests/LibWebView/TestHistoryStore.cpp

@@ -145,6 +145,19 @@ TEST_CASE(history_autocomplete_requires_three_characters_for_non_prefix_url_matc
     auto store = WebView::HistoryStore::create();
     expect_history_autocomplete_requires_three_characters_for_non_prefix_url_matches(*store);
 }
+
+TEST_CASE(history_favicon_updates_entry)
+{
+    auto store = WebView::HistoryStore::create();
+    auto url = parse_url("https://ladybird.dev/"sv);
+
+    store->record_visit(url, "Ladybird"_string, UnixDateTime::from_seconds_since_epoch(10));
+    store->update_favicon(url, "Zm9v"_string);
+
+    auto entry = store->entry_for_url(url);
+    VERIFY(entry.has_value());
+    EXPECT_EQ(entry->favicon_base64_png, Optional<String> { "Zm9v"_string });
+}
 TEST_CASE(non_browsable_urls_are_not_recorded)
 {
     auto store = WebView::HistoryStore::create();
@@ -169,6 +182,7 @@ TEST_CASE(disabled_history_store_ignores_updates)
     EXPECT(!store->entry_for_url(url).has_value());
     EXPECT(store->autocomplete_suggestions("example"sv, 8).is_empty());
 }
+
 TEST_CASE(persisted_history_survives_reopen)
 {
     auto database_directory = ByteString::formatted(
@@ -185,6 +199,7 @@ TEST_CASE(persisted_history_survives_reopen)
         auto database = TRY_OR_FAIL(Database::Database::create(database_directory, "HistoryStore"sv));
         auto store = TRY_OR_FAIL(WebView::HistoryStore::create(*database));
         store->record_visit(parse_url("https://persist.example.com/"sv), "Persisted title"_string, UnixDateTime::from_seconds_since_epoch(77));
+        store->update_favicon(parse_url("https://persist.example.com/"sv), "Zm9v"_string);
     }
 
     {
@@ -197,6 +212,7 @@ TEST_CASE(persisted_history_survives_reopen)
         EXPECT_EQ(entry->title, Optional<String> { "Persisted title"_string });
         EXPECT_EQ(entry->visit_count, 1u);
         EXPECT_EQ(entry->last_visited_time, UnixDateTime::from_seconds_since_epoch(77));
+        EXPECT_EQ(entry->favicon_base64_png, Optional<String> { "Zm9v"_string });
     }
 }
 
@@ -253,4 +269,3 @@ TEST_CASE(persisted_history_autocomplete_requires_three_characters_for_non_prefi
 
     expect_history_autocomplete_requires_three_characters_for_non_prefix_url_matches(*store);
 }
-}