Revert "Adding page language to sync data."
This reverts commit 84466676ed06dc575bff5d10a5476493cb261667.
Reason for revert: Introduced flakiness to `sync_integration_tests`'s `SingleClientSessionsSyncTest.NavigationChainAlteredDestructively`.
https://analysis.chromium.org/p/chromium/flake-portal/analysis/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vODQ0NjY2NzZlZDA2ZGM1NzViZmY1ZDEwYTU0NzY0OTNjYjI2MTY2Nww
Original change's description:
> Adding page language to sync data.
>
> Bug: 957657
> Change-Id: I3e03e000c3c377fbf1417bf8a9a63e52d71af965
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1613789
> Commit-Queue: Alexandre Frechette <frechette@chromium.org>
> Reviewed-by: anthonyvd <anthonyvd@chromium.org>
> Reviewed-by: Mikel Astiz <mastiz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#662649}
TBR=anthonyvd@chromium.org,mastiz@chromium.org,frechette@chromium.org
Change-Id: If7c671ccd8912ab53169d1647c96c17319bd8fab
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 957657
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1627455
Reviewed-by: Alexandre Frechette <frechette@chromium.org>
Commit-Queue: Alexandre Frechette <frechette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662728}
diff --git a/chrome/browser/sync/sessions/sync_sessions_router_tab_helper.cc b/chrome/browser/sync/sessions/sync_sessions_router_tab_helper.cc
index 4d6ed91..49b0c89e 100644
--- a/chrome/browser/sync/sessions/sync_sessions_router_tab_helper.cc
+++ b/chrome/browser/sync/sessions/sync_sessions_router_tab_helper.cc
@@ -7,7 +7,6 @@
#include "base/memory/ptr_util.h"
#include "chrome/browser/sessions/session_tab_helper.h"
#include "chrome/browser/sync/sessions/sync_sessions_web_contents_router.h"
-#include "components/language/core/common/language_experiments.h"
#include "components/sync_sessions/synced_tab_delegate.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/navigation_handle.h"
@@ -81,8 +80,8 @@
void SyncSessionsRouterTabHelper::OnLanguageDetermined(
const translate::LanguageDetectionDetails& details) {
- if (base::FeatureList::IsEnabled(language::kNotifySyncOnLanguageDetermined))
- NotifyRouter();
+ // TODO (crbug.com/957657): NotifyRouter() when language is synced on
+ // notification.
}
void SyncSessionsRouterTabHelper::SetSourceTabIdForChild(
diff --git a/chrome/browser/ui/sync/tab_contents_synced_tab_delegate.cc b/chrome/browser/ui/sync/tab_contents_synced_tab_delegate.cc
index 63fbac5..ddb33ec 100644
--- a/chrome/browser/ui/sync/tab_contents_synced_tab_delegate.cc
+++ b/chrome/browser/ui/sync/tab_contents_synced_tab_delegate.cc
@@ -12,7 +12,6 @@
#include "components/sync_sessions/sync_sessions_client.h"
#include "components/sync_sessions/synced_window_delegate.h"
#include "components/sync_sessions/synced_window_delegates_getter.h"
-#include "components/translate/content/browser/content_record_page_language.h"
#include "content/public/browser/favicon_status.h"
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/navigation_entry.h"
@@ -73,13 +72,11 @@
}
GURL TabContentsSyncedTabDelegate::GetVirtualURLAtIndex(int i) const {
- DCHECK(web_contents_);
NavigationEntry* entry = GetPossiblyPendingEntryAtIndex(web_contents_, i);
return entry ? entry->GetVirtualURL() : GURL();
}
GURL TabContentsSyncedTabDelegate::GetFaviconURLAtIndex(int i) const {
- DCHECK(web_contents_);
NavigationEntry* entry = GetPossiblyPendingEntryAtIndex(web_contents_, i);
return entry ? (entry->GetFavicon().valid ? entry->GetFavicon().url : GURL())
: GURL();
@@ -87,7 +84,6 @@
ui::PageTransition TabContentsSyncedTabDelegate::GetTransitionAtIndex(
int i) const {
- DCHECK(web_contents_);
NavigationEntry* entry = GetPossiblyPendingEntryAtIndex(web_contents_, i);
// If we don't have an entry, there's not a coherent PageTransition we can
// supply. There's no PageTransition::Unknown, so we just use the default,
@@ -96,18 +92,9 @@
: ui::PageTransition::PAGE_TRANSITION_LINK;
}
-std::string TabContentsSyncedTabDelegate::GetPageLanguageAtIndex(int i) const {
- DCHECK(web_contents_);
- NavigationEntry* entry = GetPossiblyPendingEntryAtIndex(web_contents_, i);
- // If we don't have an entry, return empty language.
- return entry ? translate::GetPageLanguageFromNavigation(entry)
- : std::string();
-}
-
void TabContentsSyncedTabDelegate::GetSerializedNavigationAtIndex(
int i,
sessions::SerializedNavigationEntry* serialized_entry) const {
- DCHECK(web_contents_);
NavigationEntry* entry = GetPossiblyPendingEntryAtIndex(web_contents_, i);
if (entry) {
// Explicitly exclude page state when serializing the navigation entry.
diff --git a/chrome/browser/ui/sync/tab_contents_synced_tab_delegate.h b/chrome/browser/ui/sync/tab_contents_synced_tab_delegate.h
index 5fe387d0..c500cde 100644
--- a/chrome/browser/ui/sync/tab_contents_synced_tab_delegate.h
+++ b/chrome/browser/ui/sync/tab_contents_synced_tab_delegate.h
@@ -37,7 +37,6 @@
GURL GetVirtualURLAtIndex(int i) const override;
GURL GetFaviconURLAtIndex(int i) const override;
ui::PageTransition GetTransitionAtIndex(int i) const override;
- std::string GetPageLanguageAtIndex(int i) const override;
void GetSerializedNavigationAtIndex(
int i,
sessions::SerializedNavigationEntry* serialized_entry) const override;
diff --git a/components/language/core/common/language_experiments.cc b/components/language/core/common/language_experiments.cc
index d7cedb9..5e86dfe 100644
--- a/components/language/core/common/language_experiments.cc
+++ b/components/language/core/common/language_experiments.cc
@@ -23,13 +23,9 @@
base::FEATURE_ENABLED_BY_DEFAULT};
const base::Feature kUseFluentLanguageModel{"UseFluentLanguageModel",
base::FEATURE_DISABLED_BY_DEFAULT};
-const base::Feature kNotifySyncOnLanguageDetermined{
- "NotifySyncOnLanguageDetermined", base::FEATURE_ENABLED_BY_DEFAULT};
-
// Base feature for Translate desktop UI experiment
const base::Feature kUseButtonTranslateBubbleUI{
"UseButtonTranslateBubbleUI", base::FEATURE_DISABLED_BY_DEFAULT};
-
// Params:
const char kBackoffThresholdKey[] = "backoff_threshold";
const char kOverrideModelKey[] = "override_model";
diff --git a/components/language/core/common/language_experiments.h b/components/language/core/common/language_experiments.h
index e7b0e98..529b28e 100644
--- a/components/language/core/common/language_experiments.h
+++ b/components/language/core/common/language_experiments.h
@@ -33,9 +33,6 @@
extern const char kOverrideModelDefaultValue[];
extern const char kBackoffThresholdKey[];
-// Notify sync to update data on language determined.
-extern const base::Feature kNotifySyncOnLanguageDetermined;
-
// This feature uses the existing UI for translate bubble.
extern const base::Feature kUseButtonTranslateBubbleUI;
diff --git a/components/sync/protocol/proto_visitors.h b/components/sync/protocol/proto_visitors.h
index b2cc8cb..c603fd2a 100644
--- a/components/sync/protocol/proto_visitors.h
+++ b/components/sync/protocol/proto_visitors.h
@@ -895,7 +895,6 @@
VISIT_REP(navigation_redirect);
VISIT(last_navigation_redirect_url);
VISIT(correct_referrer_policy);
- VISIT(page_language);
VISIT_ENUM(password_state);
VISIT(task_id);
VISIT_REP(ancestor_task_id);
diff --git a/components/sync/protocol/session_specifics.proto b/components/sync/protocol/session_specifics.proto
index 12cdd28..fdae362 100644
--- a/components/sync/protocol/session_specifics.proto
+++ b/components/sync/protocol/session_specifics.proto
@@ -173,10 +173,6 @@
// an entry is replaced multiple times, it represents data prior to the
// *first* replace.
optional ReplacedNavigation replaced_navigation = 29;
-
- // The page language as determined by its textual content. An ISO 639 language
- // code (two letters, except for Chinese where a localization is necessary).
- optional string page_language = 30;
}
// Navigation information for a single redirection within a single navigation.
diff --git a/components/sync_sessions/local_session_event_handler_impl.cc b/components/sync_sessions/local_session_event_handler_impl.cc
index dd55bca..bf7d848 100644
--- a/components/sync_sessions/local_session_event_handler_impl.cc
+++ b/components/sync_sessions/local_session_event_handler_impl.cc
@@ -426,10 +426,6 @@
sync_pb::TabNavigation* navigation = specifics.add_navigation();
SessionNavigationToSyncData(serialized_entry).Swap(navigation);
- const std::string page_language = tab_delegate.GetPageLanguageAtIndex(i);
- if (!page_language.empty())
- navigation->set_page_language(page_language);
-
if (is_supervised) {
navigation->set_blocked_state(
sync_pb::TabNavigation_BlockedState_STATE_ALLOWED);
diff --git a/components/sync_sessions/local_session_event_handler_impl_unittest.cc b/components/sync_sessions/local_session_event_handler_impl_unittest.cc
index 79fe3ed..65b4985 100644
--- a/components/sync_sessions/local_session_event_handler_impl_unittest.cc
+++ b/components/sync_sessions/local_session_event_handler_impl_unittest.cc
@@ -143,9 +143,6 @@
TestSyncedTabDelegate* tab = AddTabWithTime(kWindowId1, kFoo1, kTime1);
tab->Navigate(kBar1, kTime2);
tab->Navigate(kBaz1, kTime3);
- tab->SetPageLanguageAtIndex(0, "en");
- tab->SetPageLanguageAtIndex(1, "fr");
- tab->SetPageLanguageAtIndex(2, "in");
InitHandler();
const sync_pb::SessionTab session_tab =
@@ -174,9 +171,6 @@
EXPECT_FALSE(session_tab.navigation(0).has_blocked_state());
EXPECT_FALSE(session_tab.navigation(1).has_blocked_state());
EXPECT_FALSE(session_tab.navigation(2).has_blocked_state());
- EXPECT_EQ("en", session_tab.navigation(0).page_language());
- EXPECT_EQ("fr", session_tab.navigation(1).page_language());
- EXPECT_EQ("in", session_tab.navigation(2).page_language());
}
// Ensure the current_navigation_index gets set properly when the navigation
diff --git a/components/sync_sessions/synced_tab_delegate.h b/components/sync_sessions/synced_tab_delegate.h
index bc1f2d4..45ee11ab 100644
--- a/components/sync_sessions/synced_tab_delegate.h
+++ b/components/sync_sessions/synced_tab_delegate.h
@@ -50,7 +50,6 @@
virtual GURL GetVirtualURLAtIndex(int i) const = 0;
virtual GURL GetFaviconURLAtIndex(int i) const = 0;
virtual ui::PageTransition GetTransitionAtIndex(int i) const = 0;
- virtual std::string GetPageLanguageAtIndex(int i) const = 0;
virtual void GetSerializedNavigationAtIndex(
int i,
sessions::SerializedNavigationEntry* serialized_entry) const = 0;
diff --git a/components/sync_sessions/test_synced_window_delegates_getter.cc b/components/sync_sessions/test_synced_window_delegates_getter.cc
index 5519f44b..4add4ea 100644
--- a/components/sync_sessions/test_synced_window_delegates_getter.cc
+++ b/components/sync_sessions/test_synced_window_delegates_getter.cc
@@ -45,7 +45,6 @@
entry.get());
entries_.push_back(std::move(entry));
- page_language_per_index_.push_back(std::string());
set_current_entry_index(GetCurrentEntryIndex() + 1);
notify_cb_.Run(this);
}
@@ -63,12 +62,6 @@
}
}
-void TestSyncedTabDelegate::SetPageLanguageAtIndex(
- int i,
- const std::string& language) {
- page_language_per_index_[i] = language;
-}
-
bool TestSyncedTabDelegate::IsInitialBlankNavigation() const {
// This differs from NavigationControllerImpl, which has an initial blank
// NavigationEntry.
@@ -95,11 +88,6 @@
return entries_[i]->transition_type();
}
-std::string TestSyncedTabDelegate::GetPageLanguageAtIndex(int i) const {
- DCHECK(static_cast<size_t>(i) < page_language_per_index_.size());
- return page_language_per_index_[i];
-}
-
void TestSyncedTabDelegate::GetSerializedNavigationAtIndex(
int i,
sessions::SerializedNavigationEntry* serialized_entry) const {
@@ -228,11 +216,6 @@
return ui::PageTransition();
}
-std::string PlaceholderTabDelegate::GetPageLanguageAtIndex(int i) const {
- NOTREACHED();
- return std::string();
-}
-
void PlaceholderTabDelegate::GetSerializedNavigationAtIndex(
int i,
sessions::SerializedNavigationEntry* serialized_entry) const {
diff --git a/components/sync_sessions/test_synced_window_delegates_getter.h b/components/sync_sessions/test_synced_window_delegates_getter.h
index 8f28300..621beaf 100644
--- a/components/sync_sessions/test_synced_window_delegates_getter.h
+++ b/components/sync_sessions/test_synced_window_delegates_getter.h
@@ -39,15 +39,12 @@
const std::vector<std::unique_ptr<sessions::SerializedNavigationEntry>>&
navs);
- void SetPageLanguageAtIndex(int i, const std::string& language);
-
// SyncedTabDelegate overrides.
bool IsInitialBlankNavigation() const override;
int GetCurrentEntryIndex() const override;
GURL GetVirtualURLAtIndex(int i) const override;
GURL GetFaviconURLAtIndex(int i) const override;
ui::PageTransition GetTransitionAtIndex(int i) const override;
- std::string GetPageLanguageAtIndex(int i) const override;
void GetSerializedNavigationAtIndex(
int i,
sessions::SerializedNavigationEntry* serialized_entry) const override;
@@ -76,7 +73,6 @@
blocked_navigations_;
std::vector<std::unique_ptr<const sessions::SerializedNavigationEntry>>
entries_;
- std::vector<std::string> page_language_per_index_;
DISALLOW_COPY_AND_ASSIGN(TestSyncedTabDelegate);
};
@@ -103,7 +99,6 @@
GURL GetVirtualURLAtIndex(int i) const override;
GURL GetFaviconURLAtIndex(int i) const override;
ui::PageTransition GetTransitionAtIndex(int i) const override;
- std::string GetPageLanguageAtIndex(int i) const override;
void GetSerializedNavigationAtIndex(
int i,
sessions::SerializedNavigationEntry* serialized_entry) const override;
diff --git a/components/translate/content/browser/BUILD.gn b/components/translate/content/browser/BUILD.gn
index 8b35e2d..df077d0 100644
--- a/components/translate/content/browser/BUILD.gn
+++ b/components/translate/content/browser/BUILD.gn
@@ -6,8 +6,6 @@
static_library("browser") {
sources = [
- "content_record_page_language.cc",
- "content_record_page_language.h",
"content_translate_driver.cc",
"content_translate_driver.h",
]
diff --git a/components/translate/content/browser/content_record_page_language.cc b/components/translate/content/browser/content_record_page_language.cc
deleted file mode 100644
index 3af62de..0000000
--- a/components/translate/content/browser/content_record_page_language.cc
+++ /dev/null
@@ -1,35 +0,0 @@
-// Copyright 2019 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.
-
-#include "components/translate/content/browser/content_record_page_language.h"
-
-#include "content/public/browser/navigation_entry.h"
-
-namespace translate {
-
-namespace {
-// The key used to store page language in the NavigationEntry;
-const char kPageLanguageKey[] = "page_language";
-
-struct LanguageDetectionData : public base::SupportsUserData::Data {
- // The adopted page language. An ISO 639 language code (two letters, except
- // for Chinese where a localization is necessary).
- std::string page_language;
-};
-} // namespace
-
-std::string GetPageLanguageFromNavigation(content::NavigationEntry* entry) {
- auto* data =
- static_cast<LanguageDetectionData*>(entry->GetUserData(kPageLanguageKey));
- return data ? data->page_language : "";
-}
-
-void SetPageLanguageInNavigation(const std::string& page_language,
- content::NavigationEntry* entry) {
- auto data = std::make_unique<LanguageDetectionData>();
- data->page_language = page_language;
- entry->SetUserData(kPageLanguageKey, std::move(data));
-}
-
-} // namespace translate
diff --git a/components/translate/content/browser/content_record_page_language.h b/components/translate/content/browser/content_record_page_language.h
deleted file mode 100644
index 2983993..0000000
--- a/components/translate/content/browser/content_record_page_language.h
+++ /dev/null
@@ -1,24 +0,0 @@
-// Copyright 2019 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.
-
-#ifndef COMPONENTS_TRANSLATE_CONTENT_BROWSER_CONTENT_RECORD_PAGE_LANGUAGE_H_
-#define COMPONENTS_TRANSLATE_CONTENT_BROWSER_CONTENT_RECORD_PAGE_LANGUAGE_H_
-
-#include <string>
-
-namespace content {
-class NavigationEntry;
-}
-
-namespace translate {
-
-// Helper functions for storing/getting page language in a NavigationEntry.
-std::string GetPageLanguageFromNavigation(content::NavigationEntry* entry);
-
-void SetPageLanguageInNavigation(const std::string& page_language,
- content::NavigationEntry* entry);
-
-} // namespace translate
-
-#endif // COMPONENTS_TRANSLATE_CONTENT_BROWSER_CONTENT_RECORD_PAGE_LANGUAGE_H_
diff --git a/components/translate/content/browser/content_translate_driver.cc b/components/translate/content/browser/content_translate_driver.cc
index 60f54353..70f6833 100644
--- a/components/translate/content/browser/content_translate_driver.cc
+++ b/components/translate/content/browser/content_translate_driver.cc
@@ -17,7 +17,6 @@
#include "base/threading/thread_task_runner_handle.h"
#include "components/google/core/common/google_util.h"
#include "components/language/core/browser/url_language_histogram.h"
-#include "components/translate/content/browser/content_record_page_language.h"
#include "components/translate/core/browser/translate_download_manager.h"
#include "components/translate/core/browser/translate_manager.h"
#include "components/translate/core/common/translate_util.h"
@@ -45,6 +44,15 @@
// loading before giving up the translation
const int kMaxTranslateLoadCheckAttempts = 20;
+// The key used to store page language in the NavigationEntry;
+const char kPageLanguageKey[] = "page_language";
+
+struct LanguageDectionData : public base::SupportsUserData::Data {
+ // The adopted language. An ISO 639 language code (two letters, except for
+ // Chinese where a localization is necessary).
+ std::string adopted_language;
+};
+
} // namespace
ContentTranslateDriver::ContentTranslateDriver(
@@ -303,8 +311,11 @@
// Save the page language on the navigation entry so it can be synced.
auto* const entry = web_contents()->GetController().GetLastCommittedEntry();
- if (entry != nullptr)
- SetPageLanguageInNavigation(details.adopted_language, entry);
+ if (entry != nullptr) {
+ auto data = std::make_unique<LanguageDectionData>();
+ data->adopted_language = details.adopted_language;
+ entry->SetUserData(kPageLanguageKey, std::move(data));
+ }
}
for (auto& observer : observer_list_)
diff --git a/ios/chrome/browser/sync/ios_chrome_synced_tab_delegate.h b/ios/chrome/browser/sync/ios_chrome_synced_tab_delegate.h
index 01a098d..e8aaeed 100644
--- a/ios/chrome/browser/sync/ios_chrome_synced_tab_delegate.h
+++ b/ios/chrome/browser/sync/ios_chrome_synced_tab_delegate.h
@@ -33,7 +33,6 @@
GURL GetVirtualURLAtIndex(int i) const override;
GURL GetFaviconURLAtIndex(int i) const override;
ui::PageTransition GetTransitionAtIndex(int i) const override;
- std::string GetPageLanguageAtIndex(int i) const override;
void GetSerializedNavigationAtIndex(
int i,
sessions::SerializedNavigationEntry* serialized_entry) const override;
diff --git a/ios/chrome/browser/sync/ios_chrome_synced_tab_delegate.mm b/ios/chrome/browser/sync/ios_chrome_synced_tab_delegate.mm
index 63a95323..af9d7c9 100644
--- a/ios/chrome/browser/sync/ios_chrome_synced_tab_delegate.mm
+++ b/ios/chrome/browser/sync/ios_chrome_synced_tab_delegate.mm
@@ -90,11 +90,6 @@
return item->GetTransitionType();
}
-std::string IOSChromeSyncedTabDelegate::GetPageLanguageAtIndex(int i) const {
- // TODO(crbug.com/957657): Add page language to NavigationItem.
- return std::string();
-}
-
void IOSChromeSyncedTabDelegate::GetSerializedNavigationAtIndex(
int i,
sessions::SerializedNavigationEntry* serialized_entry) const {
