diff --git a/Libraries/LibWebView/HistoryStore.cpp b/Libraries/LibWebView/HistoryStore.cpp index cc824267086..9e75f61fe48 100644 --- a/Libraries/LibWebView/HistoryStore.cpp +++ b/Libraries/LibWebView/HistoryStore.cpp @@ -125,6 +125,7 @@ ErrorOr> 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> 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 HistoryStore::entry_for_url(URL::URL const& url) { if (m_is_disabled) @@ -306,11 +334,12 @@ Optional 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() : ""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 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 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 HistoryStore::PersistedStorage::entry_for_url(String const& url) { Optional entry; @@ -472,10 +520,12 @@ Optional HistoryStore::PersistedStorage::entry_for_url(String cons statements.get_entry, [&](auto statement_id) { auto title = database.result_column(statement_id, 0); + auto favicon = database.result_column(statement_id, 3); entry = HistoryEntry { .url = url, .title = title.is_empty() ? Optional {} : Optional { move(title) }, + .favicon_base64_png = favicon.is_empty() ? Optional {} : Optional { move(favicon) }, .visit_count = database.result_column(statement_id, 1), .last_visited_time = database.result_column(statement_id, 2), }; diff --git a/Libraries/LibWebView/HistoryStore.h b/Libraries/LibWebView/HistoryStore.h index 61ffc75c5d6..2f2464209d5 100644 --- a/Libraries/LibWebView/HistoryStore.h +++ b/Libraries/LibWebView/HistoryStore.h @@ -20,6 +20,7 @@ namespace WebView { struct WEBVIEW_API HistoryEntry { String url; Optional title; + Optional favicon_base64_png; u64 visit_count { 0 }; UnixDateTime last_visited_time; }; @@ -37,6 +38,7 @@ public: void record_visit(URL::URL const&, Optional 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 entry_for_url(URL::URL const&); Vector 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 title, UnixDateTime visited_at); void update_title(String const& url, String title); + void update_favicon(String const& url, String favicon_base64_png); Optional entry_for_url(String const& url); Vector 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 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 entry_for_url(String const& url); Vector autocomplete_suggestions(StringView title_query, StringView url_query, size_t limit); diff --git a/Libraries/LibWebView/ViewImplementation.cpp b/Libraries/LibWebView/ViewImplementation.cpp index 0bd3bc39668..e07b18bcd3e 100644 --- a/Libraries/LibWebView/ViewImplementation.cpp +++ b/Libraries/LibWebView/ViewImplementation.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -111,8 +112,10 @@ void ViewImplementation::set_favicon(Badge, 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); diff --git a/Libraries/LibWebView/WebContentClient.cpp b/Libraries/LibWebView/WebContentClient.cpp index 206d62e5ee6..e2787d9560a 100644 --- a/Libraries/LibWebView/WebContentClient.cpp +++ b/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 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() : ""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) diff --git a/Libraries/LibWebView/WebContentClient.h b/Libraries/LibWebView/WebContentClient.h index e92241a703e..3dc230ae683 100644 --- a/Libraries/LibWebView/WebContentClient.h +++ b/Libraries/LibWebView/WebContentClient.h @@ -8,7 +8,10 @@ #include #include +#include #include +#include +#include #include #include #include @@ -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 title, StringView reason); + virtual void die() override; virtual void did_paint(u64 page_id, Gfx::IntRect, i32) override; @@ -154,6 +159,7 @@ private: Optional view_for_page_id(u64, SourceLocation = SourceLocation::current()); HashMap> m_views; + HashMap m_history_recorded_urls_for_current_load; ProcessHandle m_process_handle; diff --git a/Tests/LibWebView/TestHistoryStore.cpp b/Tests/LibWebView/TestHistoryStore.cpp index d0df6fb5389..a1daf87f7b2 100644 --- a/Tests/LibWebView/TestHistoryStore.cpp +++ b/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 { "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 { "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 { "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); } -}