[Gardener] Revert "Add HistoryEmbeddings to fieldtrial_testing_config.json"
This reverts commit 2d674f95cce0d40952f3933ff7a9802938f158c1.
Reason for revert: Causes HistoryTest.Metrics to fail on multiple
builders.
First failure on Win10 Tests x64:
https://ci.chromium.org/ui/p/chromium/builders/ci/Win10%20Tests%20x64/90291/test-results?sortby=&groupby=
Original change's description:
> Add HistoryEmbeddings to fieldtrial_testing_config.json
>
> Needed to support any Variations configuration.
>
> This CL also fixes a tab helper crash that shows up when the flag is
> default enabled in all tests.
>
> This also avoids constructing the service for ephemeral and kiosk mode
> profiles.
>
> Bug: b/338245900
> Change-Id: Ie6a0f923bc5432762cdae3675a55cdccf725d028
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5506718
> Reviewed-by: manuk hovanesian <manukh@chromium.org>
> Commit-Queue: Tommy Li <tommycli@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1295804}
Bug: b/338245900
Change-Id: Ib17be8b92c5b9daad88c9c315462c3411247475f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5514223
Owners-Override: Michelle Chen <michellegc@google.com>
Commit-Queue: Michelle Chen <michellegc@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1295929}
diff --git a/chrome/browser/history_embeddings/history_embeddings_service_factory.cc b/chrome/browser/history_embeddings/history_embeddings_service_factory.cc
index 76ba4334..4784a8750 100644
--- a/chrome/browser/history_embeddings/history_embeddings_service_factory.cc
+++ b/chrome/browser/history_embeddings/history_embeddings_service_factory.cc
@@ -4,44 +4,16 @@
#include "chrome/browser/history_embeddings/history_embeddings_service_factory.h"
-#include "chrome/browser/app_mode/app_mode_utils.h"
-#include "chrome/browser/browser_process.h"
#include "chrome/browser/history/history_service_factory.h"
#include "chrome/browser/history_embeddings/chrome_passage_embeddings_service_controller.h"
#include "chrome/browser/optimization_guide/optimization_guide_keyed_service.h"
#include "chrome/browser/optimization_guide/optimization_guide_keyed_service_factory.h"
#include "chrome/browser/page_content_annotations/page_content_annotations_service_factory.h"
#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/profiles/profile_attributes_entry.h"
-#include "chrome/browser/profiles/profile_attributes_storage.h"
-#include "chrome/browser/profiles/profile_manager.h"
#include "components/history/core/browser/history_service.h"
#include "components/history_embeddings/history_embeddings_service.h"
#include "components/keyed_service/core/service_access_type.h"
-#if BUILDFLAG(IS_CHROMEOS_ASH)
-#include "chrome/browser/ash/profiles/profile_helper.h"
-#endif
-
-namespace {
-
-bool IsEphemeralProfile(Profile* profile) {
-#if BUILDFLAG(IS_CHROMEOS_ASH)
- if (ash::ProfileHelper::IsEphemeralUserProfile(profile)) {
- return true;
- }
-#endif
-
- // Catch additional logic that may not be caught by the existing Ash check.
- ProfileAttributesStorage& storage =
- g_browser_process->profile_manager()->GetProfileAttributesStorage();
- ProfileAttributesEntry* entry =
- storage.GetProfileAttributesWithPath(profile->GetPath());
- return entry && entry->IsEphemeral();
-}
-
-} // namespace
-
// static
history_embeddings::HistoryEmbeddingsService*
HistoryEmbeddingsServiceFactory::GetForProfile(Profile* profile) {
@@ -70,13 +42,6 @@
HistoryEmbeddingsServiceFactory::BuildServiceInstanceForBrowserContext(
content::BrowserContext* context) const {
auto* profile = Profile::FromBrowserContext(context);
-
- // Embeddings don't last long enough to help users in kiosk or ephemeral
- // profile mode, so simply never construct the service for those users.
- if (chrome::IsRunningInAppMode() || IsEphemeralProfile(profile)) {
- return nullptr;
- }
-
auto* history_service = HistoryServiceFactory::GetForProfile(
profile, ServiceAccessType::EXPLICIT_ACCESS);
// The history service is never null; even unit tests build and use one.
diff --git a/chrome/browser/history_embeddings/history_embeddings_tab_helper.cc b/chrome/browser/history_embeddings/history_embeddings_tab_helper.cc
index 9a3d26ad..e9f47b8 100644
--- a/chrome/browser/history_embeddings/history_embeddings_tab_helper.cc
+++ b/chrome/browser/history_embeddings/history_embeddings_tab_helper.cc
@@ -86,11 +86,6 @@
if (!history_service) {
return;
}
- history_embeddings::HistoryEmbeddingsService* embeddings_service =
- GetHistoryEmbeddingsService();
- if (!embeddings_service) {
- return;
- }
history_service->GetMostRecentVisitsForGurl(
history_url_.value(), 1,
@@ -129,7 +124,7 @@
history_embeddings_service->RetrievePassages(latest_visit,
*render_frame_host);
},
- embeddings_service->AsWeakPtr(), render_frame_host,
+ GetHistoryEmbeddingsService()->AsWeakPtr(), render_frame_host,
history_visit_time_.value()),
&task_tracker_);
diff --git a/chrome/browser/resources/history/app.ts b/chrome/browser/resources/history/app.ts
index 2323e64..2cac89d 100644
--- a/chrome/browser/resources/history/app.ts
+++ b/chrome/browser/resources/history/app.ts
@@ -679,9 +679,6 @@
private recordHistoryPageView_() {
let histogramValue = HistoryPageViewHistogram.END;
switch (this.selectedPage_) {
- case Page.HISTORY:
- histogramValue = HistoryPageViewHistogram.HISTORY;
- break;
case Page.HISTORY_CLUSTERS:
histogramValue = HistoryPageViewHistogram.JOURNEYS;
break;
@@ -694,9 +691,8 @@
histogramValue = HistoryPageViewHistogram.PRODUCT_SPECIFICATIONS_LISTS;
break;
default:
- // Explicitly early exit and don't record anything for unknown selected
- // pages. This can occur during test initialization.
- return;
+ histogramValue = HistoryPageViewHistogram.HISTORY;
+ break;
}
this.browserService_!.recordHistogram(
diff --git a/chrome/browser/resources/history/router.ts b/chrome/browser/resources/history/router.ts
index e97921e..747b47e 100644
--- a/chrome/browser/resources/history/router.ts
+++ b/chrome/browser/resources/history/router.ts
@@ -147,9 +147,6 @@
// Must change selectedPage before `change-query`, otherwise the
// query-manager will call serializeUrl() with the old page.
- // TODO(b/338245900): This is kind of nasty. Without cr-tabs to constrain
- // `selectedPage`, this can be set to an arbitrary value from the URL.
- // To fix this, we should constrain the selected pages to an actual enum.
this.selectedPage = page;
this.dispatchEvent(new CustomEvent(
'change-query', {bubbles: true, composed: true, detail: changes}));
diff --git a/chrome/test/data/webui/history/history_routing_test.ts b/chrome/test/data/webui/history/history_routing_test.ts
index d1928b4..584374f7 100644
--- a/chrome/test/data/webui/history/history_routing_test.ts
+++ b/chrome/test/data/webui/history/history_routing_test.ts
@@ -31,9 +31,6 @@
enableHistoryClusters: 'Enable',
isHistoryClustersEnabled,
isHistoryClustersVisible: true,
- /* For this test suite we want to keep testing the old cr-tabs which
- is the default Stable configuration. */
- enableHistoryEmbeddings: false,
});
});
diff --git a/components/history_embeddings/history_embeddings_service.cc b/components/history_embeddings/history_embeddings_service.cc
index c637470..773919d 100644
--- a/components/history_embeddings/history_embeddings_service.cc
+++ b/components/history_embeddings/history_embeddings_service.cc
@@ -141,10 +141,6 @@
void HistoryEmbeddingsService::RetrievePassages(
const history::VisitRow& visit_row,
content::RenderFrameHost& host) {
- if (!host.IsRenderFrameLive()) {
- return;
- }
-
const base::TimeTicks start_time = base::TimeTicks::Now();
mojo::Remote<blink::mojom::InnerTextAgent> agent;
host.GetRemoteInterfaces()->GetInterface(agent.BindNewPipeAndPassReceiver());
diff --git a/testing/variations/fieldtrial_testing_config.json b/testing/variations/fieldtrial_testing_config.json
index e69f71d..3343d61 100644
--- a/testing/variations/fieldtrial_testing_config.json
+++ b/testing/variations/fieldtrial_testing_config.json
@@ -9176,25 +9176,6 @@
]
}
],
- "HistoryEmbeddings": [
- {
- "platforms": [
- "chromeos",
- "chromeos_lacros",
- "linux",
- "mac",
- "windows"
- ],
- "experiments": [
- {
- "name": "Enabled",
- "enable_features": [
- "HistoryEmbeddings"
- ]
- }
- ]
- }
- ],
"HlsBuiltinPlayer": [
{
"platforms": [
