diff --git a/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/DeletedPageInfo.java b/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/DeletedPageInfo.java new file mode 100644 index 00000000000000..5bc2ea405e52ef --- /dev/null +++ b/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/DeletedPageInfo.java @@ -0,0 +1,40 @@ +// Copyright 2017 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.offlinepages; + +/** + * Simple object representing important information of a deleted offline page. + */ +public class DeletedPageInfo { + private final long mOfflineId; + private final ClientId mClientId; + private final String mRequestOrigin; + + public DeletedPageInfo( + long offlineId, String clientNamespace, String clientId, String requestOrigin) { + this(offlineId, new ClientId(clientNamespace, clientId), requestOrigin); + } + + public DeletedPageInfo(long offlineId, ClientId clientId, String requestOrigin) { + mOfflineId = offlineId; + mClientId = clientId; + mRequestOrigin = requestOrigin; + } + + /** @return offline id for the deleted page */ + public long getOfflineId() { + return mOfflineId; + } + + /** @return Client Id for the deleted page */ + public ClientId getClientId() { + return mClientId; + } + + /** @return request origin of the deleted page */ + public String getRequestOrigin() { + return mRequestOrigin; + } +} diff --git a/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java index e1c8b93487840b..c2f25be3ed07be 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java @@ -94,7 +94,7 @@ public void offlinePageAdded(OfflinePageItem addedPage) {} * @param offlineId The offline ID of the deleted offline page. * @param clientId The client supplied ID of the deleted offline page. */ - public void offlinePageDeleted(long offlineId, ClientId clientId) {} + public void offlinePageDeleted(DeletedPageInfo deletedPage) {} } /** @@ -568,9 +568,9 @@ protected void offlinePageBridgeDestroyed() { } @CalledByNative - void offlinePageDeleted(long offlineId, ClientId clientId) { + void offlinePageDeleted(DeletedPageInfo deletedPage) { for (OfflinePageModelObserver observer : mObservers) { - observer.offlinePageDeleted(offlineId, clientId); + observer.offlinePageDeleted(deletedPage); } } @@ -595,6 +595,12 @@ private static ClientId createClientId(String clientNamespace, String id) { return new ClientId(clientNamespace, id); } + @CalledByNative + private static DeletedPageInfo createDeletedPageInfo( + long offlineId, String clientNamespace, String clientId, String requestOrigin) { + return new DeletedPageInfo(offlineId, clientNamespace, clientId, requestOrigin); + } + private static native boolean nativeIsOfflineBookmarksEnabled(); private static native boolean nativeIsPageSharingEnabled(); private static native boolean nativeCanSavePage(String url); diff --git a/chrome/android/java/src/org/chromium/chrome/browser/suggestions/SuggestionsOfflineModelObserver.java b/chrome/android/java/src/org/chromium/chrome/browser/suggestions/SuggestionsOfflineModelObserver.java index 9cfb5bf789afef..6d8a102c6b46c7 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/suggestions/SuggestionsOfflineModelObserver.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/suggestions/SuggestionsOfflineModelObserver.java @@ -7,7 +7,7 @@ import android.support.annotation.Nullable; import org.chromium.base.Callback; -import org.chromium.chrome.browser.offlinepages.ClientId; +import org.chromium.chrome.browser.offlinepages.DeletedPageInfo; import org.chromium.chrome.browser.offlinepages.OfflinePageBridge; import org.chromium.chrome.browser.offlinepages.OfflinePageItem; @@ -46,13 +46,13 @@ public void offlinePageAdded(OfflinePageItem addedPage) { } @Override - public void offlinePageDeleted(long offlineId, ClientId clientId) { + public void offlinePageDeleted(DeletedPageInfo deletedPage) { for (T suggestion : getOfflinableSuggestions()) { if (suggestion.requiresExactOfflinePage()) continue; Long suggestionOfflineId = suggestion.getOfflinePageOfflineId(); if (suggestionOfflineId == null) continue; - if (suggestionOfflineId != offlineId) continue; + if (suggestionOfflineId != deletedPage.getOfflineId()) continue; // The old value cannot be simply removed without a request to the // model, because there may be an older offline page for the same diff --git a/chrome/android/java_sources.gni b/chrome/android/java_sources.gni index d90f33edff2618..20b7cec2b71a13 100644 --- a/chrome/android/java_sources.gni +++ b/chrome/android/java_sources.gni @@ -682,6 +682,7 @@ chrome_java_sources = [ "java/src/org/chromium/chrome/browser/offlinepages/BackgroundSchedulerBridge.java", "java/src/org/chromium/chrome/browser/offlinepages/BackgroundSchedulerProcessor.java", "java/src/org/chromium/chrome/browser/offlinepages/ClientId.java", + "java/src/org/chromium/chrome/browser/offlinepages/DeletedPageInfo.java", "java/src/org/chromium/chrome/browser/offlinepages/OfflineBackgroundTask.java", "java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java", "java/src/org/chromium/chrome/browser/offlinepages/OfflinePageItem.java", diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridgeUnitTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridgeUnitTest.java index 3fede53823b874..fe5a1d26031503 100644 --- a/chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridgeUnitTest.java +++ b/chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridgeUnitTest.java @@ -82,9 +82,9 @@ public class MockOfflinePageModelObserver extends OfflinePageModelObserver { public long lastDeletedOfflineId; public ClientId lastDeletedClientId; - public void offlinePageDeleted(long offlineId, ClientId clientId) { - lastDeletedOfflineId = offlineId; - lastDeletedClientId = clientId; + public void offlinePageDeleted(DeletedPageInfo deletedPage) { + lastDeletedOfflineId = deletedPage.getOfflineId(); + lastDeletedClientId = deletedPage.getClientId(); } } @@ -110,7 +110,7 @@ public void testRemovePageByClientId() { ClientId testClientId = new ClientId(TEST_NAMESPACE, TEST_ID); long testOfflineId = 123; - mBridge.offlinePageDeleted(testOfflineId, testClientId); + mBridge.offlinePageDeleted(new DeletedPageInfo(testOfflineId, testClientId, "")); assertEquals(testOfflineId, observer1.lastDeletedOfflineId); assertEquals(testClientId, observer1.lastDeletedClientId); assertEquals(testOfflineId, observer2.lastDeletedOfflineId); diff --git a/chrome/browser/android/offline_pages/evaluation/offline_page_evaluation_bridge.cc b/chrome/browser/android/offline_pages/evaluation/offline_page_evaluation_bridge.cc index 6fc30a64db5062..5755f67770befd 100644 --- a/chrome/browser/android/offline_pages/evaluation/offline_page_evaluation_bridge.cc +++ b/chrome/browser/android/offline_pages/evaluation/offline_page_evaluation_bridge.cc @@ -267,8 +267,7 @@ void OfflinePageEvaluationBridge::OfflinePageAdded( const OfflinePageItem& added_page) {} void OfflinePageEvaluationBridge::OfflinePageDeleted( - int64_t offline_id, - const ClientId& client_id) {} + const DeletedPageInfo& page_info) {} // Implement RequestCoordinator::Observer void OfflinePageEvaluationBridge::OnAdded(const SavePageRequest& request) { diff --git a/chrome/browser/android/offline_pages/evaluation/offline_page_evaluation_bridge.h b/chrome/browser/android/offline_pages/evaluation/offline_page_evaluation_bridge.h index 398196ff4e4fe9..17df3b383cbbd5 100644 --- a/chrome/browser/android/offline_pages/evaluation/offline_page_evaluation_bridge.h +++ b/chrome/browser/android/offline_pages/evaluation/offline_page_evaluation_bridge.h @@ -44,8 +44,7 @@ class OfflinePageEvaluationBridge : public OfflinePageModel::Observer, void OfflinePageModelLoaded(OfflinePageModel* model) override; void OfflinePageAdded(OfflinePageModel* model, const OfflinePageItem& added_page) override; - void OfflinePageDeleted(int64_t offline_id, - const ClientId& client_id) override; + void OfflinePageDeleted(const DeletedPageInfo& page_info) override; // RequestCoordinator::Observer implementation. void OnAdded(const SavePageRequest& request) override; diff --git a/chrome/browser/android/offline_pages/offline_page_bridge.cc b/chrome/browser/android/offline_pages/offline_page_bridge.cc index 3c9e4ab3d83244..104e5f7224afb7 100644 --- a/chrome/browser/android/offline_pages/offline_page_bridge.cc +++ b/chrome/browser/android/offline_pages/offline_page_bridge.cc @@ -80,6 +80,16 @@ ScopedJavaLocalRef ToJavaOfflinePageItem( offline_page.access_count, offline_page.last_access_time.ToJavaTime()); } +ScopedJavaLocalRef ToJavaDeletedPageInfo( + JNIEnv* env, + const OfflinePageModel::DeletedPageInfo& deleted_page) { + return Java_OfflinePageBridge_createDeletedPageInfo( + env, deleted_page.offline_id, + ConvertUTF8ToJavaString(env, deleted_page.client_id.name_space), + ConvertUTF8ToJavaString(env, deleted_page.client_id.id), + ConvertUTF8ToJavaString(env, deleted_page.request_origin)); +} + void CheckPagesExistOfflineCallback( const ScopedJavaGlobalRef& j_callback_obj, const OfflinePageModel::CheckPagesExistOfflineResult& offline_pages) { @@ -297,11 +307,11 @@ void OfflinePageBridge::OfflinePageAdded(OfflinePageModel* model, env, java_ref_, ToJavaOfflinePageItem(env, added_page)); } -void OfflinePageBridge::OfflinePageDeleted(int64_t offline_id, - const ClientId& client_id) { +void OfflinePageBridge::OfflinePageDeleted( + const OfflinePageModel::DeletedPageInfo& page_info) { JNIEnv* env = base::android::AttachCurrentThread(); - Java_OfflinePageBridge_offlinePageDeleted(env, java_ref_, offline_id, - CreateClientId(env, client_id)); + Java_OfflinePageBridge_offlinePageDeleted( + env, java_ref_, ToJavaDeletedPageInfo(env, page_info)); } void OfflinePageBridge::CheckPagesExistOffline( diff --git a/chrome/browser/android/offline_pages/offline_page_bridge.h b/chrome/browser/android/offline_pages/offline_page_bridge.h index 49b8d5a99968da..21283c61a7c9a7 100644 --- a/chrome/browser/android/offline_pages/offline_page_bridge.h +++ b/chrome/browser/android/offline_pages/offline_page_bridge.h @@ -42,8 +42,8 @@ class OfflinePageBridge : public OfflinePageModel::Observer, void OfflinePageModelLoaded(OfflinePageModel* model) override; void OfflinePageAdded(OfflinePageModel* model, const OfflinePageItem& added_page) override; - void OfflinePageDeleted(int64_t offline_id, - const ClientId& client_id) override; + void OfflinePageDeleted( + const OfflinePageModel::DeletedPageInfo& page_info) override; void CheckPagesExistOffline( JNIEnv* env, diff --git a/chrome/browser/android/offline_pages/recent_tab_helper_unittest.cc b/chrome/browser/android/offline_pages/recent_tab_helper_unittest.cc index 9e46292f42a387..1ed59913bf733e 100644 --- a/chrome/browser/android/offline_pages/recent_tab_helper_unittest.cc +++ b/chrome/browser/android/offline_pages/recent_tab_helper_unittest.cc @@ -138,7 +138,8 @@ class RecentTabHelperTest page_added_count_++; all_pages_needs_updating_ = true; } - void OfflinePageDeleted(int64_t, const offline_pages::ClientId&) override { + void OfflinePageDeleted( + const OfflinePageModel::DeletedPageInfo& pageInfo) override { model_removed_count_++; all_pages_needs_updating_ = true; } diff --git a/chrome/browser/ntp_snippets/download_suggestions_provider.cc b/chrome/browser/ntp_snippets/download_suggestions_provider.cc index c3bdf6737adca2..8d0e677f9d367c 100644 --- a/chrome/browser/ntp_snippets/download_suggestions_provider.cc +++ b/chrome/browser/ntp_snippets/download_suggestions_provider.cc @@ -383,12 +383,11 @@ void DownloadSuggestionsProvider::OfflinePageAdded( } void DownloadSuggestionsProvider::OfflinePageDeleted( - int64_t offline_id, - const offline_pages::ClientId& client_id) { + const offline_pages::OfflinePageModel::DeletedPageInfo& page_info) { DCHECK(offline_page_model_); if (IsClientIdForOfflinePageDownload( - offline_page_model_->GetPolicyController(), client_id)) { - InvalidateSuggestion(GetOfflinePagePerCategoryID(offline_id)); + offline_page_model_->GetPolicyController(), page_info.client_id)) { + InvalidateSuggestion(GetOfflinePagePerCategoryID(page_info.offline_id)); } } diff --git a/chrome/browser/ntp_snippets/download_suggestions_provider.h b/chrome/browser/ntp_snippets/download_suggestions_provider.h index 951be129e046cc..c62f8e47f6b985 100644 --- a/chrome/browser/ntp_snippets/download_suggestions_provider.h +++ b/chrome/browser/ntp_snippets/download_suggestions_provider.h @@ -92,8 +92,9 @@ class DownloadSuggestionsProvider void OfflinePageAdded( offline_pages::OfflinePageModel* model, const offline_pages::OfflinePageItem& added_page) override; - void OfflinePageDeleted(int64_t offline_id, - const offline_pages::ClientId& client_id) override; + void OfflinePageDeleted( + const offline_pages::OfflinePageModel::DeletedPageInfo& page_info) + override; // content::DownloadManager::Observer implementation. void OnDownloadCreated(content::DownloadManager* manager, diff --git a/chrome/browser/ntp_snippets/download_suggestions_provider_unittest.cc b/chrome/browser/ntp_snippets/download_suggestions_provider_unittest.cc index aa5f26eee165b0..ba5aa5fb824fb5 100644 --- a/chrome/browser/ntp_snippets/download_suggestions_provider_unittest.cc +++ b/chrome/browser/ntp_snippets/download_suggestions_provider_unittest.cc @@ -324,7 +324,9 @@ class DownloadSuggestionsProviderTest : public testing::Test { void FireOfflinePageDeleted(const OfflinePageItem& item) { DCHECK(provider_); - provider_->OfflinePageDeleted(item.offline_id, item.client_id); + offline_pages::OfflinePageModel::DeletedPageInfo info( + item.offline_id, item.client_id, item.request_origin); + provider_->OfflinePageDeleted(info); } void FireDownloadCreated(DownloadItem* item) { diff --git a/components/ntp_snippets/offline_pages/recent_tab_suggestions_provider_unittest.cc b/components/ntp_snippets/offline_pages/recent_tab_suggestions_provider_unittest.cc index 7d8489ca0e022a..c43ad141d2bdff 100644 --- a/components/ntp_snippets/offline_pages/recent_tab_suggestions_provider_unittest.cc +++ b/components/ntp_snippets/offline_pages/recent_tab_suggestions_provider_unittest.cc @@ -116,7 +116,9 @@ class RecentTabSuggestionsProviderTestNoLoad : public testing::Test { int tab_id = offline_pages::RecentTabsUIAdapterDelegate::TabIdFromClientId( item.client_id); RemoveTab(tab_id); - ui_adapter_->OfflinePageDeleted(item.offline_id, item.client_id); + ui_adapter_->OfflinePageDeleted( + offline_pages::OfflinePageModel::DeletedPageInfo( + item.offline_id, item.client_id, "" /* request_origin */)); } std::set ReadDismissedIDsFromPrefs() { diff --git a/components/ntp_snippets/remote/prefetched_pages_tracker_impl.cc b/components/ntp_snippets/remote/prefetched_pages_tracker_impl.cc index df17693868b4b6..a595715a321b57 100644 --- a/components/ntp_snippets/remote/prefetched_pages_tracker_impl.cc +++ b/components/ntp_snippets/remote/prefetched_pages_tracker_impl.cc @@ -86,10 +86,9 @@ void PrefetchedPagesTrackerImpl::OfflinePageAdded( } void PrefetchedPagesTrackerImpl::OfflinePageDeleted( - int64_t offline_id, - const offline_pages::ClientId& client_id) { + const offline_pages::OfflinePageModel::DeletedPageInfo& page_info) { std::map::iterator it = - offline_id_to_url_mapping_.find(offline_id); + offline_id_to_url_mapping_.find(page_info.offline_id); if (it != offline_id_to_url_mapping_.end()) { DCHECK(prefetched_urls_.count(it->second)); prefetched_urls_.erase(it->second); diff --git a/components/ntp_snippets/remote/prefetched_pages_tracker_impl.h b/components/ntp_snippets/remote/prefetched_pages_tracker_impl.h index 045c98e389365f..32be1db182e552 100644 --- a/components/ntp_snippets/remote/prefetched_pages_tracker_impl.h +++ b/components/ntp_snippets/remote/prefetched_pages_tracker_impl.h @@ -41,8 +41,9 @@ class PrefetchedPagesTrackerImpl void OfflinePageAdded( offline_pages::OfflinePageModel* model, const offline_pages::OfflinePageItem& added_page) override; - void OfflinePageDeleted(int64_t offline_id, - const offline_pages::ClientId& client_id) override; + void OfflinePageDeleted( + const offline_pages::OfflinePageModel::DeletedPageInfo& page_info) + override; private: void Initialize(const std::vector& diff --git a/components/ntp_snippets/remote/prefetched_pages_tracker_impl_unittest.cc b/components/ntp_snippets/remote/prefetched_pages_tracker_impl_unittest.cc index 15d22c013e0c2f..ddbb295cb14f06 100644 --- a/components/ntp_snippets/remote/prefetched_pages_tracker_impl_unittest.cc +++ b/components/ntp_snippets/remote/prefetched_pages_tracker_impl_unittest.cc @@ -141,7 +141,8 @@ TEST_F(PrefetchedPagesTrackerImplTest, ShouldDeletePrefetchedURLWhenNotified) { ASSERT_TRUE( tracker.PrefetchedOfflinePageExists(GURL("http://prefetched.com"))); - tracker.OfflinePageDeleted(item.offline_id, item.client_id); + tracker.OfflinePageDeleted(offline_pages::OfflinePageModel::DeletedPageInfo( + item.offline_id, item.client_id, "" /* request_origin */)); EXPECT_FALSE( tracker.PrefetchedOfflinePageExists(GURL("http://prefetched.com"))); } @@ -160,8 +161,9 @@ TEST_F(PrefetchedPagesTrackerImplTest, ASSERT_TRUE( tracker.PrefetchedOfflinePageExists(GURL("http://prefetched.com"))); - tracker.OfflinePageDeleted(manually_downloaded_item.offline_id, - manually_downloaded_item.client_id); + tracker.OfflinePageDeleted(offline_pages::OfflinePageModel::DeletedPageInfo( + manually_downloaded_item.offline_id, manually_downloaded_item.client_id, + "" /* request_origin */)); EXPECT_TRUE( tracker.PrefetchedOfflinePageExists(GURL("http://prefetched.com"))); } diff --git a/components/offline_pages/core/downloads/download_ui_adapter.cc b/components/offline_pages/core/downloads/download_ui_adapter.cc index 7e1368ec7ba81b..310531ad57733f 100644 --- a/components/offline_pages/core/downloads/download_ui_adapter.cc +++ b/components/offline_pages/core/downloads/download_ui_adapter.cc @@ -116,11 +116,11 @@ void DownloadUIAdapter::OfflinePageAdded(OfflinePageModel* model, AddItemHelper(base::MakeUnique(added_page, temporarily_hidden)); } -void DownloadUIAdapter::OfflinePageDeleted(int64_t offline_id, - const ClientId& client_id) { - if (!delegate_->IsVisibleInUI(client_id)) +void DownloadUIAdapter::OfflinePageDeleted( + const OfflinePageModel::DeletedPageInfo& page_info) { + if (!delegate_->IsVisibleInUI(page_info.client_id)) return; - DeleteItemHelper(client_id.id); + DeleteItemHelper(page_info.client_id.id); } // RequestCoordinator::Observer diff --git a/components/offline_pages/core/downloads/download_ui_adapter.h b/components/offline_pages/core/downloads/download_ui_adapter.h index 9868fb009ca85b..ec1f8d56219dda 100644 --- a/components/offline_pages/core/downloads/download_ui_adapter.h +++ b/components/offline_pages/core/downloads/download_ui_adapter.h @@ -109,8 +109,8 @@ class DownloadUIAdapter : public OfflinePageModel::Observer, void OfflinePageModelLoaded(OfflinePageModel* model) override; void OfflinePageAdded(OfflinePageModel* model, const OfflinePageItem& added_page) override; - void OfflinePageDeleted(int64_t offline_id, - const ClientId& client_id) override; + void OfflinePageDeleted( + const OfflinePageModel::DeletedPageInfo& page_info) override; // RequestCoordinator::Observer void OnAdded(const SavePageRequest& request) override; diff --git a/components/offline_pages/core/downloads/download_ui_adapter_unittest.cc b/components/offline_pages/core/downloads/download_ui_adapter_unittest.cc index 60cafb477680f2..b5c2b32a83bbcb 100644 --- a/components/offline_pages/core/downloads/download_ui_adapter_unittest.cc +++ b/components/offline_pages/core/downloads/download_ui_adapter_unittest.cc @@ -112,8 +112,9 @@ class MockOfflinePageModel : public StubOfflinePageModel { void DeletePageAndNotifyAdapter(const std::string& guid) { for (const auto& page : pages) { if (page.second.client_id.id == guid) { - observer_->OfflinePageDeleted(page.second.offline_id, - page.second.client_id); + DeletedPageInfo info(page.second.offline_id, page.second.client_id, + page.second.request_origin); + observer_->OfflinePageDeleted(info); pages.erase(page.first); return; } diff --git a/components/offline_pages/core/offline_page_model.cc b/components/offline_pages/core/offline_page_model.cc index e0d7cbff4ae709..109cec6fd16528 100644 --- a/components/offline_pages/core/offline_page_model.cc +++ b/components/offline_pages/core/offline_page_model.cc @@ -25,6 +25,14 @@ OfflinePageModel::SavePageParams::SavePageParams(const SavePageParams& other) { OfflinePageModel::SavePageParams::~SavePageParams() {} +OfflinePageModel::DeletedPageInfo::DeletedPageInfo( + int64_t offline_id, + const ClientId& client_id, + const std::string& request_origin) + : offline_id(offline_id), + client_id(client_id), + request_origin(request_origin) {} + // static bool OfflinePageModel::CanSaveURL(const GURL& url) { return url.is_valid() && url.SchemeIsHTTPOrHTTPS(); diff --git a/components/offline_pages/core/offline_page_model.h b/components/offline_pages/core/offline_page_model.h index a155222fb59f8e..3f1061852f6611 100644 --- a/components/offline_pages/core/offline_page_model.h +++ b/components/offline_pages/core/offline_page_model.h @@ -82,6 +82,19 @@ class OfflinePageModel : public base::SupportsUserData { std::string request_origin; }; + // Information about a deleted page. + struct DeletedPageInfo { + DeletedPageInfo(int64_t offline_id, + const ClientId& client_id, + const std::string& request_origin); + // The ID of the deleted page. + int64_t offline_id; + // Client ID of the deleted page. + ClientId client_id; + // The origin that the page was saved on behalf of. + std::string request_origin; + }; + // Observer of the OfflinePageModel. class Observer { public: @@ -93,8 +106,7 @@ class OfflinePageModel : public base::SupportsUserData { const OfflinePageItem& added_page) = 0; // Invoked when an offline copy related to |offline_id| was deleted. - virtual void OfflinePageDeleted(int64_t offline_id, - const ClientId& client_id) = 0; + virtual void OfflinePageDeleted(const DeletedPageInfo& page_info) = 0; protected: virtual ~Observer() = default; diff --git a/components/offline_pages/core/offline_page_model_impl.cc b/components/offline_pages/core/offline_page_model_impl.cc index a850fe34644a9e..4909a12cf403e5 100644 --- a/components/offline_pages/core/offline_page_model_impl.cc +++ b/components/offline_pages/core/offline_page_model_impl.cc @@ -982,8 +982,9 @@ void OfflinePageModelImpl::OnRemoveOfflinePagesDone( } for (const auto& page : result->updated_items) { + DeletedPageInfo info(page.offline_id, page.client_id, page.request_origin); for (Observer& observer : observers_) - observer.OfflinePageDeleted(page.offline_id, page.client_id); + observer.OfflinePageDeleted(info); } DeletePageResult delete_result; diff --git a/components/offline_pages/core/offline_page_model_impl_unittest.cc b/components/offline_pages/core/offline_page_model_impl_unittest.cc index 609bb33d1bcb19..235dbe5faca3c8 100644 --- a/components/offline_pages/core/offline_page_model_impl_unittest.cc +++ b/components/offline_pages/core/offline_page_model_impl_unittest.cc @@ -56,6 +56,7 @@ const ClientId kTestClientId3(kTestClientNamespace, "42"); const ClientId kTestUserRequestedClientId(kUserRequestedNamespace, "714"); const int64_t kTestFileSize = 876543LL; const base::string16 kTestTitle = base::UTF8ToUTF16("a title"); +const std::string kRequestOrigin("abc.xyz"); bool URLSpecContains(std::string contains_value, const GURL& url) { std::string spec = url.spec(); @@ -80,8 +81,8 @@ class OfflinePageModelImplTest void OfflinePageModelLoaded(OfflinePageModel* model) override; void OfflinePageAdded(OfflinePageModel* model, const OfflinePageItem& added_page) override; - void OfflinePageDeleted(int64_t offline_id, - const ClientId& client_id) override; + void OfflinePageDeleted( + const OfflinePageModel::DeletedPageInfo& pageInfo) override; // OfflinePageTestArchiver::Observer implementation. void SetLastPathCreatedByArchiver(const base::FilePath& file_path) override; @@ -136,11 +137,11 @@ class OfflinePageModelImplTest std::unique_ptr archiver); // Saves the page without waiting for it to finish. - void SavePageWithArchiverAsync( - const GURL& url, - const ClientId& client_id, - const GURL& original_url, - std::unique_ptr archiver); + void SavePageWithArchiverAsync(const GURL& url, + const ClientId& client_id, + const GURL& original_url, + const std::string& request_origin, + std::unique_ptr archiver); // All 3 methods below will wait for the save to finish. void SavePageWithArchiver( @@ -153,6 +154,10 @@ class OfflinePageModelImplTest // Returns the offline ID of the saved page. std::pair SavePage(const GURL& url, const ClientId& client_id); + std::pair SavePage( + const GURL& url, + const ClientId& client_id, + const std::string& request_origin); void DeletePage(int64_t offline_id, const DeletePageCallback& callback) { std::vector offline_ids; @@ -183,6 +188,10 @@ class OfflinePageModelImplTest ClientId last_deleted_client_id() const { return last_deleted_client_id_; } + std::string last_deleted_request_origin() const { + return last_deleted_request_origin_; + } + const base::FilePath& last_archiver_path() { return last_archiver_path_; } int last_cleared_pages_count() const { return last_cleared_pages_count_; } @@ -207,6 +216,7 @@ class OfflinePageModelImplTest base::FilePath last_archiver_path_; int64_t last_deleted_offline_id_; ClientId last_deleted_client_id_; + std::string last_deleted_request_origin_; CheckPagesExistOfflineResult last_pages_exist_result_; int last_cleared_pages_count_; DeletePageResult last_clear_page_result_; @@ -250,10 +260,11 @@ void OfflinePageModelImplTest::OfflinePageModelLoaded(OfflinePageModel* model) { ASSERT_EQ(model_.get(), model); } -void OfflinePageModelImplTest::OfflinePageDeleted(int64_t offline_id, - const ClientId& client_id) { - last_deleted_offline_id_ = offline_id; - last_deleted_client_id_ = client_id; +void OfflinePageModelImplTest::OfflinePageDeleted( + const OfflinePageModel::DeletedPageInfo& info) { + last_deleted_offline_id_ = info.offline_id; + last_deleted_client_id_ = info.client_id; + last_deleted_request_origin_ = info.request_origin; } void OfflinePageModelImplTest::OfflinePageAdded( @@ -354,12 +365,14 @@ void OfflinePageModelImplTest::SavePageWithArchiverAsync( const GURL& url, const ClientId& client_id, const GURL& original_url, + const std::string& request_origin, std::unique_ptr archiver) { OfflinePageModel::SavePageParams save_page_params; save_page_params.url = url; save_page_params.client_id = client_id; save_page_params.original_url = original_url; save_page_params.is_background = false; + save_page_params.request_origin = request_origin; SavePageWithParamsAsync(save_page_params, std::move(archiver)); } @@ -367,7 +380,7 @@ void OfflinePageModelImplTest::SavePageWithArchiver( const GURL& url, const ClientId& client_id, std::unique_ptr archiver) { - SavePageWithArchiverAsync(url, client_id, GURL(), std::move(archiver)); + SavePageWithArchiverAsync(url, client_id, GURL(), "", std::move(archiver)); PumpLoop(); } @@ -376,15 +389,23 @@ void OfflinePageModelImplTest::SavePageWithArchiverResult( const ClientId& client_id, OfflinePageArchiver::ArchiverResult result) { std::unique_ptr archiver(BuildArchiver(url, result)); - SavePageWithArchiverAsync(url, client_id, GURL(), std::move(archiver)); + SavePageWithArchiverAsync(url, client_id, GURL(), "", std::move(archiver)); PumpLoop(); } std::pair OfflinePageModelImplTest::SavePage(const GURL& url, const ClientId& client_id) { + return SavePage(url, client_id, ""); +} + +std::pair OfflinePageModelImplTest::SavePage( + const GURL& url, + const ClientId& client_id, + const std::string& request_origin) { std::unique_ptr archiver(BuildArchiver( url, OfflinePageArchiver::ArchiverResult::SUCCESSFULLY_CREATED)); - SavePageWithArchiverAsync(url, client_id, GURL(), std::move(archiver)); + SavePageWithArchiverAsync(url, client_id, GURL(), request_origin, + std::move(archiver)); PumpLoop(); return std::make_pair(last_save_result_, last_save_offline_id_); } @@ -514,8 +535,8 @@ TEST_F(OfflinePageModelImplTest, SavePageSuccessful) { std::unique_ptr archiver(BuildArchiver( kTestUrl, OfflinePageArchiver::ArchiverResult::SUCCESSFULLY_CREATED)); - SavePageWithArchiverAsync( - kTestUrl, kTestClientId1, kTestUrl2, std::move(archiver)); + SavePageWithArchiverAsync(kTestUrl, kTestClientId1, kTestUrl2, "", + std::move(archiver)); PumpLoop(); EXPECT_TRUE(HasPages(kTestClientNamespace)); @@ -543,6 +564,44 @@ TEST_F(OfflinePageModelImplTest, SavePageSuccessful) { EXPECT_EQ(0, offline_pages[0].flags); EXPECT_EQ(kTestTitle, offline_pages[0].title); EXPECT_EQ(kTestUrl2, offline_pages[0].original_url); + EXPECT_EQ("", offline_pages[0].request_origin); +} + +TEST_F(OfflinePageModelImplTest, SavePageSuccessfulWithRequestOrigin) { + EXPECT_FALSE(HasPages(kTestClientNamespace)); + + std::unique_ptr archiver(BuildArchiver( + kTestUrl, OfflinePageArchiver::ArchiverResult::SUCCESSFULLY_CREATED)); + SavePageWithArchiverAsync(kTestUrl, kTestClientId1, kTestUrl2, kRequestOrigin, + std::move(archiver)); + PumpLoop(); + EXPECT_TRUE(HasPages(kTestClientNamespace)); + + OfflinePageTestStore* store = GetStore(); + EXPECT_EQ(kTestUrl, store->last_saved_page().url); + EXPECT_EQ(kTestClientId1.id, store->last_saved_page().client_id.id); + EXPECT_EQ(kTestClientId1.name_space, + store->last_saved_page().client_id.name_space); + // Save last_archiver_path since it will be referred to later. + base::FilePath archiver_path = last_archiver_path(); + EXPECT_EQ(archiver_path, store->last_saved_page().file_path); + EXPECT_EQ(kTestFileSize, store->last_saved_page().file_size); + EXPECT_EQ(SavePageResult::SUCCESS, last_save_result()); + ResetResults(); + + const std::vector& offline_pages = GetAllPages(); + + ASSERT_EQ(1UL, offline_pages.size()); + EXPECT_EQ(kTestUrl, offline_pages[0].url); + EXPECT_EQ(kTestClientId1.id, offline_pages[0].client_id.id); + EXPECT_EQ(kTestClientId1.name_space, offline_pages[0].client_id.name_space); + EXPECT_EQ(archiver_path, offline_pages[0].file_path); + EXPECT_EQ(kTestFileSize, offline_pages[0].file_size); + EXPECT_EQ(0, offline_pages[0].access_count); + EXPECT_EQ(0, offline_pages[0].flags); + EXPECT_EQ(kTestTitle, offline_pages[0].title); + EXPECT_EQ(kTestUrl2, offline_pages[0].original_url); + EXPECT_EQ(kRequestOrigin, offline_pages[0].request_origin); } TEST_F(OfflinePageModelImplTest, SavePageOfflineArchiverCancelled) { @@ -605,14 +664,15 @@ TEST_F(OfflinePageModelImplTest, SavePageOfflineArchiverTwoPages) { // CompleteCreateArchive() is called. OfflinePageTestArchiver* archiver_ptr = archiver.get(); archiver_ptr->set_delayed(true); - SavePageWithArchiverAsync( - kTestUrl, kTestClientId1, GURL(), std::move(archiver)); + // First page has no request origin. + SavePageWithArchiverAsync(kTestUrl, kTestClientId1, GURL(), "", + std::move(archiver)); EXPECT_TRUE(archiver_ptr->create_archive_called()); // |remove_popup_overlay| should not be turned on on foreground mode. EXPECT_FALSE(archiver_ptr->create_archive_params().remove_popup_overlay); - // Request to save another page. - SavePage(kTestUrl2, kTestClientId2); + // Request to save another page, with request origin. + SavePage(kTestUrl2, kTestClientId2, kRequestOrigin); OfflinePageTestStore* store = GetStore(); @@ -621,6 +681,7 @@ TEST_F(OfflinePageModelImplTest, SavePageOfflineArchiverTwoPages) { base::FilePath archiver_path2 = last_archiver_path(); EXPECT_EQ(archiver_path2, store->last_saved_page().file_path); EXPECT_EQ(kTestFileSize, store->last_saved_page().file_size); + EXPECT_EQ(kRequestOrigin, store->last_saved_page().request_origin); EXPECT_EQ(SavePageResult::SUCCESS, last_save_result()); ResetResults(); @@ -634,6 +695,7 @@ TEST_F(OfflinePageModelImplTest, SavePageOfflineArchiverTwoPages) { base::FilePath archiver_path = last_archiver_path(); EXPECT_EQ(archiver_path, store->last_saved_page().file_path); EXPECT_EQ(kTestFileSize, store->last_saved_page().file_size); + EXPECT_EQ("", store->last_saved_page().request_origin); EXPECT_EQ(SavePageResult::SUCCESS, last_save_result()); ResetResults(); @@ -659,12 +721,14 @@ TEST_F(OfflinePageModelImplTest, SavePageOfflineArchiverTwoPages) { EXPECT_EQ(kTestFileSize, page1->file_size); EXPECT_EQ(0, page1->access_count); EXPECT_EQ(0, page1->flags); + EXPECT_EQ("", page1->request_origin); EXPECT_EQ(kTestUrl2, page2->url); EXPECT_EQ(kTestClientId2, page2->client_id); EXPECT_EQ(archiver_path2, page2->file_path); EXPECT_EQ(kTestFileSize, page2->file_size); EXPECT_EQ(0, page2->access_count); EXPECT_EQ(0, page2->flags); + EXPECT_EQ(kRequestOrigin, page2->request_origin); } TEST_F(OfflinePageModelImplTest, SavePageOnBackground) { @@ -711,15 +775,15 @@ TEST_F(OfflinePageModelImplTest, GetAllPagesStoreEmpty) { TEST_F(OfflinePageModelImplTest, DeletePageSuccessful) { OfflinePageTestStore* store = GetStore(); - // Save one page. - SavePage(kTestUrl, kTestClientId1); + // Save one page with request origin. + SavePage(kTestUrl, kTestClientId1, kRequestOrigin); int64_t offline1 = last_save_offline_id(); EXPECT_EQ(SavePageResult::SUCCESS, last_save_result()); EXPECT_EQ(1u, store->GetAllPages().size()); ResetResults(); - // Save another page. + // Save another page, no request origin. SavePage(kTestUrl2, kTestClientId2); int64_t offline2 = last_save_offline_id(); EXPECT_EQ(SavePageResult::SUCCESS, last_save_result()); @@ -735,6 +799,7 @@ TEST_F(OfflinePageModelImplTest, DeletePageSuccessful) { EXPECT_EQ(last_deleted_offline_id(), offline1); EXPECT_EQ(last_deleted_client_id(), kTestClientId1); + EXPECT_EQ(last_deleted_request_origin(), kRequestOrigin); EXPECT_EQ(DeletePageResult::SUCCESS, last_delete_result()); ASSERT_EQ(1u, store->GetAllPages().size()); EXPECT_EQ(kTestUrl2, store->GetAllPages()[0].url); @@ -749,6 +814,7 @@ TEST_F(OfflinePageModelImplTest, DeletePageSuccessful) { EXPECT_EQ(last_deleted_offline_id(), offline2); EXPECT_EQ(last_deleted_client_id(), kTestClientId2); + EXPECT_EQ(last_deleted_request_origin(), ""); EXPECT_EQ(DeletePageResult::SUCCESS, last_delete_result()); EXPECT_EQ(0u, store->GetAllPages().size()); } @@ -1023,8 +1089,8 @@ TEST_F(OfflinePageModelImplTest, GetPagesByFinalURLWithFragmentStripped) { TEST_F(OfflinePageModelImplTest, GetPagesByAllURLS) { std::unique_ptr archiver(BuildArchiver( kTestUrl, OfflinePageArchiver::ArchiverResult::SUCCESSFULLY_CREATED)); - SavePageWithArchiverAsync( - kTestUrl, kTestClientId1, kTestUrl2, std::move(archiver)); + SavePageWithArchiverAsync(kTestUrl, kTestClientId1, kTestUrl2, "", + std::move(archiver)); PumpLoop(); SavePage(kTestUrl2, kTestClientId2); @@ -1277,7 +1343,7 @@ TEST_F(OfflinePageModelImplTest, StoreLoadFailurePersists) { TEST_F(OfflinePageModelImplTest, GetPagesMatchingQuery) { std::unique_ptr archiver(BuildArchiver( kTestUrl, OfflinePageArchiver::ArchiverResult::SUCCESSFULLY_CREATED)); - SavePageWithArchiverAsync(kTestUrl, kTestClientId1, kTestUrl2, + SavePageWithArchiverAsync(kTestUrl, kTestClientId1, kTestUrl2, "", std::move(archiver)); PumpLoop();