Update recently visited page when navigation happens
ShoppingService::OnWebWrapperSwitched() is only called on focus
change. But if a navigation happens in the tab, this is not get called.
This CL calls OnWebWrapperSwitched() when a navigation happens to keep
recent viewed tab list up to date.
Bug: 355487445
Change-Id: I7f1939f1657a6d4649105ac11f95c484c156ef13
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5739997
Reviewed-by: Adam Rice <[email protected]>
Reviewed-by: Matthew Jones <[email protected]>
Commit-Queue: Min Qin <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1339757}
diff --git a/components/commerce/content/browser/BUILD.gn b/components/commerce/content/browser/BUILD.gn
index 4a27670..795fd8b 100644
--- a/components/commerce/content/browser/BUILD.gn
+++ b/components/commerce/content/browser/BUILD.gn
@@ -29,11 +29,15 @@
source_set("unit_tests") {
testonly = true
- sources = [ "web_extractor_impl_unittest.cc" ]
+ sources = [
+ "commerce_tab_helper_unittest.cc",
+ "web_extractor_impl_unittest.cc",
+ ]
deps = [
":browser",
"//base",
"//base/test:test_support",
+ "//components/commerce/core:shopping_service_test_support",
"//components/commerce/core/mojom:mojo_bindings",
"//content/public/browser",
"//content/test:test_support",
diff --git a/components/commerce/content/browser/commerce_tab_helper.cc b/components/commerce/content/browser/commerce_tab_helper.cc
index cc58ac77..f6fc513 100644
--- a/components/commerce/content/browser/commerce_tab_helper.cc
+++ b/components/commerce/content/browser/commerce_tab_helper.cc
@@ -50,6 +50,12 @@
if (web_contents()->IsDocumentOnLoadCompletedInPrimaryMainFrame()) {
shopping_service_->DidFinishLoad(web_wrapper_.get());
}
+
+ if (navigation_handle->HasCommitted() &&
+ navigation_handle->ShouldUpdateHistory() &&
+ web_contents()->GetFocusedFrame()) {
+ shopping_service_->OnWebWrapperViewed(web_wrapper_.get());
+ }
}
void CommerceTabHelper::DidStopLoading() {
diff --git a/components/commerce/content/browser/commerce_tab_helper.h b/components/commerce/content/browser/commerce_tab_helper.h
index 54804ab8..84ab474 100644
--- a/components/commerce/content/browser/commerce_tab_helper.h
+++ b/components/commerce/content/browser/commerce_tab_helper.h
@@ -49,6 +49,7 @@
void SetShoppingServiceForTesting(KeyedService* service);
private:
+ friend class CommerceTabHelperTest;
friend class content::WebContentsUserData<CommerceTabHelper>;
CommerceTabHelper(content::WebContents* contents,
diff --git a/components/commerce/content/browser/commerce_tab_helper_unittest.cc b/components/commerce/content/browser/commerce_tab_helper_unittest.cc
new file mode 100644
index 0000000..a585b4d
--- /dev/null
+++ b/components/commerce/content/browser/commerce_tab_helper_unittest.cc
@@ -0,0 +1,89 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/commerce/content/browser/commerce_tab_helper.h"
+
+#include "base/memory/ptr_util.h"
+#include "components/commerce/content/browser/web_contents_wrapper.h"
+#include "components/commerce/core/mock_shopping_service.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/mock_navigation_handle.h"
+#include "content/public/test/test_renderer_host.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace commerce {
+
+const char kLastMainFrameUrl[] = "https://foo.com";
+const char kNewMainFrameUrl[] = "https://foobar.com";
+
+class CommerceTabHelperTest : public content::RenderViewHostTestHarness {
+ public:
+ CommerceTabHelperTest() = default;
+ CommerceTabHelperTest(const CommerceTabHelperTest&) = delete;
+ CommerceTabHelperTest& operator=(const CommerceTabHelperTest&) = delete;
+ ~CommerceTabHelperTest() override = default;
+
+ void SetUp() override {
+ content::RenderViewHostTestHarness::SetUp();
+ tab_helper_ = base::WrapUnique(new CommerceTabHelper(
+ web_contents(), browser_context()->IsOffTheRecord(), &shopping_service_,
+ 0));
+ NavigateAndCommit(GURL(kLastMainFrameUrl));
+ }
+
+ void TearDown() override { content::RenderViewHostTestHarness::TearDown(); }
+
+ protected:
+ std::unique_ptr<CommerceTabHelper> tab_helper_;
+ MockShoppingService shopping_service_;
+};
+
+TEST_F(CommerceTabHelperTest, FocusedMainFrameNavigation) {
+ FocusWebContentsOnMainFrame();
+ NavigateAndCommit(GURL(kNewMainFrameUrl));
+
+ EXPECT_CALL(shopping_service_, GetUrlInfosForRecentlyViewedWebWrappers)
+ .WillOnce([this]() {
+ return shopping_service_
+ .ShoppingService::GetUrlInfosForRecentlyViewedWebWrappers();
+ });
+ std::vector<UrlInfo> infos =
+ shopping_service_.GetUrlInfosForRecentlyViewedWebWrappers();
+ EXPECT_EQ(infos.size(), 1u);
+ EXPECT_EQ(infos[0].url, GURL(kNewMainFrameUrl));
+}
+
+TEST_F(CommerceTabHelperTest, NotFocusedMainFrameNavigation) {
+ NavigateAndCommit(GURL(kNewMainFrameUrl));
+
+ EXPECT_CALL(shopping_service_, GetUrlInfosForRecentlyViewedWebWrappers)
+ .WillOnce([this]() {
+ return shopping_service_
+ .ShoppingService::GetUrlInfosForRecentlyViewedWebWrappers();
+ });
+ std::vector<UrlInfo> infos =
+ shopping_service_.GetUrlInfosForRecentlyViewedWebWrappers();
+ EXPECT_EQ(infos.size(), 0u);
+}
+
+TEST_F(CommerceTabHelperTest, SubFrameNavigation) {
+ FocusWebContentsOnMainFrame();
+ content::MockNavigationHandle navigation_handle(GURL(kNewMainFrameUrl),
+ main_rfh());
+ navigation_handle.set_is_in_primary_main_frame(false);
+ navigation_handle.set_has_committed(true);
+ tab_helper_->DidFinishNavigation(&navigation_handle);
+
+ EXPECT_CALL(shopping_service_, GetUrlInfosForRecentlyViewedWebWrappers)
+ .WillOnce([this]() {
+ return shopping_service_
+ .ShoppingService::GetUrlInfosForRecentlyViewedWebWrappers();
+ });
+ std::vector<UrlInfo> infos =
+ shopping_service_.GetUrlInfosForRecentlyViewedWebWrappers();
+ EXPECT_EQ(infos.size(), 0u);
+}
+
+} // namespace commerce
diff --git a/components/commerce/core/shopping_service.cc b/components/commerce/core/shopping_service.cc
index 83035b0..9b48485 100644
--- a/components/commerce/core/shopping_service.cc
+++ b/components/commerce/core/shopping_service.cc
@@ -428,35 +428,11 @@
}
void ShoppingService::OnWebWrapperSwitched(WebWrapper* web) {
- bool already_exists_in_recents = false;
- for (auto it = recently_visited_tabs_.begin();
- it != recently_visited_tabs_.end(); ++it) {
- if (it->url == web->GetLastCommittedURL()) {
- recently_visited_tabs_.erase(it);
+ UpdateRecentlyViewedURL(web);
+}
- // Don't remove the item from the cache here in case it's the only
- // reference to a particular URL (causing deletion from the cache) since
- // we're only shifting it to the head of the list.
- already_exists_in_recents = true;
- break;
- }
- }
-
- UrlInfo info;
- info.url = web->GetLastCommittedURL();
- info.title = web->GetTitle();
-
- if (!already_exists_in_recents) {
- commerce_info_cache_.AddRef(info.url);
- }
-
- recently_visited_tabs_.insert(recently_visited_tabs_.begin(),
- std::move(info));
-
- if (recently_visited_tabs_.size() > kRecentTabsMaxSize) {
- commerce_info_cache_.RemoveRef(recently_visited_tabs_.back().url);
- recently_visited_tabs_.pop_back();
- }
+void ShoppingService::OnWebWrapperViewed(WebWrapper* web) {
+ UpdateRecentlyViewedURL(web);
}
void ShoppingService::TryRunningLocalExtractionForProductInfo(
@@ -1827,6 +1803,38 @@
std::move(callback)));
}
+void ShoppingService::UpdateRecentlyViewedURL(WebWrapper* web) {
+ bool already_exists_in_recents = false;
+ for (auto it = recently_visited_tabs_.begin();
+ it != recently_visited_tabs_.end(); ++it) {
+ if (it->url == web->GetLastCommittedURL()) {
+ recently_visited_tabs_.erase(it);
+
+ // Don't remove the item from the cache here in case it's the only
+ // reference to a particular URL (causing deletion from the cache) since
+ // we're only shifting it to the head of the list.
+ already_exists_in_recents = true;
+ break;
+ }
+ }
+
+ UrlInfo info;
+ info.url = web->GetLastCommittedURL();
+ info.title = web->GetTitle();
+
+ if (!already_exists_in_recents) {
+ commerce_info_cache_.AddRef(info.url);
+ }
+
+ recently_visited_tabs_.insert(recently_visited_tabs_.begin(),
+ std::move(info));
+
+ if (recently_visited_tabs_.size() > kRecentTabsMaxSize) {
+ commerce_info_cache_.RemoveRef(recently_visited_tabs_.back().url);
+ recently_visited_tabs_.pop_back();
+ }
+}
+
const std::vector<ProductSpecificationsSet>
ShoppingService::GetAllProductSpecificationSets() {
if (!product_specifications_service_) {
diff --git a/components/commerce/core/shopping_service.h b/components/commerce/core/shopping_service.h
index 242082f3..e875fc08 100644
--- a/components/commerce/core/shopping_service.h
+++ b/components/commerce/core/shopping_service.h
@@ -478,6 +478,10 @@
// analogous to switching tabs.
void OnWebWrapperSwitched(WebWrapper* web);
+ // Called to signal that a WebWrapper is viewed. This happens when a new
+ // navigation is committed in a focused tab.
+ void OnWebWrapperViewed(WebWrapper* web);
+
// Schedule (or reschedule) the on-page local extraction execution. Calling
// this sequentially for the same web wrapper with the same URL will cancel
// the pending task and schedule a new one. The script will, at most, run once
@@ -619,6 +623,8 @@
void GetProductIdentifierForUrl(const GURL& url,
UrlProductIdentifierTupleCallback callback);
+ void UpdateRecentlyViewedURL(WebWrapper* web);
+
// Return all ProductSpecificationsSets from ProductSpecificationsService.
virtual const std::vector<ProductSpecificationsSet>
GetAllProductSpecificationSets();
diff --git a/components/commerce/ios/DEPS b/components/commerce/ios/DEPS
index d0e62f4..0dd9adc8 100644
--- a/components/commerce/ios/DEPS
+++ b/components/commerce/ios/DEPS
@@ -4,4 +4,5 @@
"+ios/web/public",
"+services/data_decoder/public/cpp",
"+ui/base",
+ "+net/http/http_response_headers.h",
]
diff --git a/components/commerce/ios/browser/BUILD.gn b/components/commerce/ios/browser/BUILD.gn
index 7ffd4c6..e5b56e1 100644
--- a/components/commerce/ios/browser/BUILD.gn
+++ b/components/commerce/ios/browser/BUILD.gn
@@ -23,6 +23,7 @@
"//ios/web/public",
"//ios/web/public/js_messaging",
"//ios/web/public/webui",
+ "//net",
"//services/data_decoder/public/cpp",
]
diff --git a/components/commerce/ios/browser/commerce_tab_helper.mm b/components/commerce/ios/browser/commerce_tab_helper.mm
index 967593e7..a287aa8 100644
--- a/components/commerce/ios/browser/commerce_tab_helper.mm
+++ b/components/commerce/ios/browser/commerce_tab_helper.mm
@@ -6,10 +6,42 @@
#include "base/memory/ptr_util.h"
#include "ios/web/public/navigation/navigation_context.h"
+#include "ios/web/public/navigation/navigation_item.h"
+#include "ios/web/public/navigation/navigation_manager.h"
#include "ios/web/public/web_state.h"
+#include "net/http/http_response_headers.h"
namespace commerce {
+namespace {
+bool ShouldUpdateRecentlyViewedURL(web::WebState* web_state,
+ web::NavigationContext* navigation_context) {
+ if (navigation_context->GetError()) {
+ return false;
+ }
+
+ if (navigation_context->GetResponseHeaders() &&
+ navigation_context->GetResponseHeaders()->response_code() == 404) {
+ return false;
+ }
+
+ if (!navigation_context->HasCommitted() ||
+ !web_state->GetNavigationManager()->GetLastCommittedItem()) {
+ // Navigation was replaced or aborted.
+ return false;
+ }
+
+ web::NavigationItem* last_committed_item =
+ web_state->GetNavigationManager()->GetLastCommittedItem();
+ const GURL& url = last_committed_item->GetURL();
+ if (url.SchemeIs(url::kDataScheme)) {
+ return false;
+ }
+
+ return true;
+}
+} // namespace
+
CommerceTabHelper::CommerceTabHelper(web::WebState* state,
bool is_off_the_record,
ShoppingService* shopping_service)
@@ -44,6 +76,11 @@
}
shopping_service_->DidNavigatePrimaryMainFrame(web_wrapper_.get());
+
+ if (web_state->IsVisible() &&
+ ShouldUpdateRecentlyViewedURL(web_state, navigation_context)) {
+ shopping_service_->OnWebWrapperViewed(web_wrapper_.get());
+ }
}
void CommerceTabHelper::DidStopLoading(web::WebState* web_state) {
