Send server experiment IDs in feed reliability log
Bug: 348442524
Change-Id: I0bf51f6dc8a652d142c3d55e0fd58dc75347d8da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5663717
Reviewed-by: Jonathan Freed <[email protected]>
Reviewed-by: Adam Arcaro <[email protected]>
Code-Coverage: [email protected] <[email protected]>
Commit-Queue: Jian Li <[email protected]>
Reviewed-by: Tommy Nyquist <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1322338}
diff --git a/chrome/browser/feed/android/feed_reliability_logging_bridge.cc b/chrome/browser/feed/android/feed_reliability_logging_bridge.cc
index 7cb8a75..bb6907b9 100644
--- a/chrome/browser/feed/android/feed_reliability_logging_bridge.cc
+++ b/chrome/browser/feed/android/feed_reliability_logging_bridge.cc
@@ -3,7 +3,9 @@
// found in the LICENSE file.
#include "chrome/browser/feed/android/feed_reliability_logging_bridge.h"
+
#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"
#include "base/time/time.h"
#include "components/feed/core/v2/public/types.h"
#include "net/base/net_errors.h"
@@ -13,6 +15,8 @@
// Must come after all headers that specialize FromJniType() / ToJniType().
#include "chrome/browser/feed/android/jni_headers/FeedReliabilityLoggingBridge_jni.h"
+using base::android::ScopedJavaLocalRef;
+
namespace feed {
namespace android {
namespace {
@@ -248,6 +252,13 @@
base::android::AttachCurrentThread(), java_ref_, success);
}
+void FeedReliabilityLoggingBridge::ReportExperiments(
+ const std::vector<int32_t>& experiment_ids) {
+ JNIEnv* env = base::android::AttachCurrentThread();
+ Java_FeedReliabilityLoggingBridge_reportExperiments(env, java_ref_,
+ experiment_ids);
+}
+
void FeedReliabilityLoggingBridge::Destroy(JNIEnv* env) {
delete this;
}
diff --git a/chrome/browser/feed/android/feed_reliability_logging_bridge.h b/chrome/browser/feed/android/feed_reliability_logging_bridge.h
index a5f9ee2..549e5870f 100644
--- a/chrome/browser/feed/android/feed_reliability_logging_bridge.h
+++ b/chrome/browser/feed/android/feed_reliability_logging_bridge.h
@@ -62,6 +62,7 @@
int64_t server_send_timestamp_ns) override;
void LogLoadMoreRequestFinished(int combined_network_status_code) override;
void LogLoadMoreEnded(bool success) override;
+ void ReportExperiments(const std::vector<int32_t>& experiment_ids) override;
private:
base::android::ScopedJavaGlobalRef<jobject> java_ref_;
diff --git a/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/FeedReliabilityLoggingBridge.java b/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/FeedReliabilityLoggingBridge.java
index d9aa472..1165e29 100644
--- a/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/FeedReliabilityLoggingBridge.java
+++ b/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/FeedReliabilityLoggingBridge.java
@@ -8,6 +8,7 @@
import org.jni_zero.CalledByNative;
import org.jni_zero.JNINamespace;
+import org.jni_zero.JniType;
import org.jni_zero.NativeMethods;
import org.chromium.chrome.browser.xsurface.feed.FeedLaunchReliabilityLogger;
@@ -198,6 +199,14 @@
}
}
+ @CalledByNative
+ public void reportExperiments(@JniType("std::vector<int32_t>") int[] experimentIds) {
+ mLaunchLogger.reportExperiments(experimentIds);
+ if (mUserInteractionLogger != null) {
+ mUserInteractionLogger.reportExperiments(experimentIds);
+ }
+ }
+
public void onStreamUpdateFinished() {
if (!mLaunchLogger.isLaunchInProgress()) return;
diff --git a/chrome/browser/feed/feed_service_factory.cc b/chrome/browser/feed/feed_service_factory.cc
index 29f2624..933f356 100644
--- a/chrome/browser/feed/feed_service_factory.cc
+++ b/chrome/browser/feed/feed_service_factory.cc
@@ -121,9 +121,9 @@
// by design. We do not provide the variations IDs from the backend
// and do not attach them to the X-Client-Data header.
for (const auto& exp : experiments) {
- for (const auto& group_name : exp.second) {
+ for (const auto& group : exp.second) {
ChromeMetricsServiceAccessor::RegisterSyntheticFieldTrial(exp.first,
- group_name);
+ group.name);
}
}
}
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index 5107440..9debef6 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -250,6 +250,7 @@
#include "components/feed/buildflags.h"
#include "components/feed/core/common/pref_names.h"
#include "components/feed/core/shared_prefs/pref_names.h"
+#include "components/feed/core/v2/ios_shared_prefs.h"
#if BUILDFLAG(IS_ANDROID)
#include "chrome/browser/accessibility/accessibility_prefs/android/accessibility_prefs_controller.h"
@@ -2808,6 +2809,11 @@
syncer::SyncPrefs::MaybeMigrateAutofillToPerAccountPref(profile_prefs);
#endif // !BUILDFLAG(IS_ANDROID)
+#if BUILDFLAG(IS_ANDROID)
+ // Added 06/2024
+ feed::prefs::MigrateObsoleteFeedExperimentPref_Jun_2024(profile_prefs);
+#endif // BUILDFLAG(IS_ANDROID)
+
// Please don't delete the following line. It is used by PRESUBMIT.py.
// END_MIGRATE_OBSOLETE_PROFILE_PREFS
diff --git a/components/BUILD.gn b/components/BUILD.gn
index c6399dc..2db2912 100644
--- a/components/BUILD.gn
+++ b/components/BUILD.gn
@@ -447,6 +447,7 @@
"//components/enterprise/content:unit_tests",
"//components/favicon/content:unit_tests",
"//components/feed/core/v2:core_unit_tests",
+ "//components/feed/core/v2:ios_shared_unit_tests",
"//components/file_access:unit_tests",
"//components/gcm_driver/instance_id:unit_tests",
"//components/heavy_ad_intervention:unit_tests",
diff --git a/components/feed/core/common/BUILD.gn b/components/feed/core/common/BUILD.gn
index a8ac913..2fb22043 100644
--- a/components/feed/core/common/BUILD.gn
+++ b/components/feed/core/common/BUILD.gn
@@ -18,16 +18,3 @@
"//components/feed:feature_list",
]
}
-
-source_set("feed_core_common_unit_tests") {
- testonly = true
- sources = [ "pref_names_unittest.cc" ]
-
- # public_deps = [ ":test_helpers" ]
- deps = [
- ":feed_core_common",
- "//base",
- "//components/prefs:test_support",
- "//testing/gtest",
- ]
-}
diff --git a/components/feed/core/common/pref_names.cc b/components/feed/core/common/pref_names.cc
index 7693c77..d061153c 100644
--- a/components/feed/core/common/pref_names.cc
+++ b/components/feed/core/common/pref_names.cc
@@ -48,19 +48,19 @@
const char kInfoCardStates[] = "feed.info_card_states";
const char kHasSeenWebFeed[] = "webfeed.has_seen_feed";
const char kLastBadgeAnimationTime[] = "webfeed.last_badge_animation_time";
-const char kExperimentsV2[] = "feedv2.experiments_v2";
+const char kExperimentsV3[] = "feedv2.experiments_v3";
const char kInfoCardTrackingStateDict[] = "info-card-tracking-state-dict";
-// Deprecated October 2022
-const char kExperimentsDeprecated[] = "feedv2.experiments";
+// Deprecated June 2024
+const char kExperimentsV2Deprecated[] = "feedv2.experiments_v2";
} // namespace prefs
// Deprecated prefs:
namespace {
-void RegisterObsoletePrefsOct_2022(PrefRegistrySimple* registry) {
- registry->RegisterDictionaryPref(prefs::kExperimentsDeprecated);
+void RegisterObsoletePrefsJun_2024(PrefRegistrySimple* registry) {
+ registry->RegisterDictionaryPref(prefs::kExperimentsV2Deprecated);
}
} // namespace
@@ -95,7 +95,7 @@
registry->RegisterBooleanPref(feed::prefs::kHasSeenWebFeed, false);
registry->RegisterTimePref(feed::prefs::kLastBadgeAnimationTime,
base::Time());
- registry->RegisterDictionaryPref(feed::prefs::kExperimentsV2);
+ registry->RegisterDictionaryPref(feed::prefs::kExperimentsV3);
registry->RegisterDictionaryPref(feed::prefs::kInfoCardTrackingStateDict);
#if BUILDFLAG(IS_IOS)
@@ -103,23 +103,7 @@
false);
#endif // BUILDFLAG(IS_IOS)
- RegisterObsoletePrefsOct_2022(registry);
-}
-
-void MigrateObsoleteProfilePrefsOct_2022(PrefService* prefs) {
- const base::Value* val =
- prefs->GetUserPrefValue(prefs::kExperimentsDeprecated);
- const base::Value::Dict* old = val ? val->GetIfDict() : nullptr;
- if (old) {
- base::Value::Dict dict;
- for (const auto kv : *old) {
- base::Value::List list;
- list.Append(kv.second.GetString());
- dict.Set(kv.first, std::move(list));
- }
- prefs->SetDict(feed::prefs::kExperimentsV2, std::move(dict));
- }
- prefs->ClearPref(prefs::kExperimentsDeprecated);
+ RegisterObsoletePrefsJun_2024(registry);
}
} // namespace feed
diff --git a/components/feed/core/common/pref_names.h b/components/feed/core/common/pref_names.h
index 5fcb126..2d298063 100644
--- a/components/feed/core/common/pref_names.h
+++ b/components/feed/core/common/pref_names.h
@@ -87,14 +87,14 @@
// The pref name for when the user last saw badge animation for web feed.
extern const char kLastBadgeAnimationTime[];
// The pref name for storing the server experiments the client is in.
-extern const char kExperimentsV2[];
+extern const char kExperimentsV3[];
// Contains a dictionary of tracking states for all info cards in the feed.
extern const char kInfoCardTrackingStateDict[];
// Deprecated prefs
// The pref name for storing the server experiments the client is in.
-extern const char kExperimentsDeprecated[];
+extern const char kExperimentsV2Deprecated[];
} // namespace prefs
diff --git a/components/feed/core/common/pref_names_unittest.cc b/components/feed/core/common/pref_names_unittest.cc
deleted file mode 100644
index 2d99467..0000000
--- a/components/feed/core/common/pref_names_unittest.cc
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright 2022 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/feed/core/common/pref_names.h"
-
-#include <string>
-
-#include "base/values.h"
-#include "components/prefs/testing_pref_service.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace feed {
-
-class PrefNamesTest : public testing::Test {
- protected:
- PrefNamesTest() { feed::RegisterProfilePrefs(prefs_.registry()); }
-
- TestingPrefServiceSimple prefs_;
-};
-
-TEST_F(PrefNamesTest, MigrateExperiments) {
- base::Value::Dict dict;
- dict.Set("Trial1", "Group1");
- base::Value::Dict expected;
- base::Value::List group_list;
- group_list.Append("Group1");
- expected.Set("Trial1", std::move(group_list));
-
- // Set the old prefs dictionary.
- prefs_.SetDict(prefs::kExperimentsDeprecated, std::move(dict));
-
- // Migrate the prefs.
- MigrateObsoleteProfilePrefsOct_2022(&prefs_);
-
- ASSERT_TRUE(prefs_.HasPrefPath(prefs::kExperimentsV2));
- EXPECT_EQ(expected, prefs_.GetDict(prefs::kExperimentsV2));
-}
-
-} // namespace feed
diff --git a/components/feed/core/v2/api_test/feed_api_stream_unittest.cc b/components/feed/core/v2/api_test/feed_api_stream_unittest.cc
index 9f45c59..18d98906 100644
--- a/components/feed/core/v2/api_test/feed_api_stream_unittest.cc
+++ b/components/feed/core/v2/api_test/feed_api_stream_unittest.cc
@@ -2657,8 +2657,8 @@
TEST_F(FeedApiTest, ExperimentsAreClearedOnClearAll) {
Experiments e;
- std::vector<std::string> group_list1{"Group1"};
- std::vector<std::string> group_list2{"Group2"};
+ std::vector<ExperimentGroup> group_list1{{"Group1", 123}};
+ std::vector<ExperimentGroup> group_list2{{"Group2", 9999}};
e["Trial1"] = group_list1;
e["Trial2"] = group_list2;
prefs::SetExperiments(e, profile_prefs_);
diff --git a/components/feed/core/v2/api_test/feed_api_test.cc b/components/feed/core/v2/api_test/feed_api_test.cc
index 35f0f7f0..4cf430c 100644
--- a/components/feed/core/v2/api_test/feed_api_test.cc
+++ b/components/feed/core/v2/api_test/feed_api_test.cc
@@ -511,6 +511,9 @@
base::StrCat({"LogLoadMoreEnded success=", success ? "true" : "false"}));
}
+void TestReliabilityLoggingBridge::ReportExperiments(
+ const std::vector<int32_t>& experiment_ids) {}
+
TestImageFetcher::TestImageFetcher(
scoped_refptr<::network::SharedURLLoaderFactory> url_loader_factory)
: ImageFetcher(url_loader_factory) {}
diff --git a/components/feed/core/v2/api_test/feed_api_test.h b/components/feed/core/v2/api_test/feed_api_test.h
index 7a84ee2d..7376db5 100644
--- a/components/feed/core/v2/api_test/feed_api_test.h
+++ b/components/feed/core/v2/api_test/feed_api_test.h
@@ -122,6 +122,7 @@
int64_t server_send_timestamp_ns) override;
void LogLoadMoreRequestFinished(int canonical_status) override;
void LogLoadMoreEnded(bool success) override;
+ void ReportExperiments(const std::vector<int32_t>& experiment_ids) override;
private:
std::vector<std::string> events_;
diff --git a/components/feed/core/v2/feed_stream.cc b/components/feed/core/v2/feed_stream.cc
index 39bf2d8e..00be21dc 100644
--- a/components/feed/core/v2/feed_stream.cc
+++ b/components/feed/core/v2/feed_stream.cc
@@ -475,6 +475,19 @@
void FeedStream::UpdateExperiments(Experiments experiments) {
delegate_->RegisterExperiments(experiments);
prefs::SetExperiments(experiments, *profile_prefs_);
+
+ std::vector<int32_t> experiment_ids;
+ for (const auto& e : experiments) {
+ for (const auto& g : e.second) {
+ experiment_ids.push_back(g.experiment_id);
+ }
+ }
+ for (auto& item : streams_) {
+ if (!item.second.surfaces.empty()) {
+ item.second.surface_updater->launch_reliability_logger()
+ .ReportExperiments(experiment_ids);
+ }
+ }
}
SurfaceId FeedStream::CreateSurface(const StreamType& type,
diff --git a/components/feed/core/v2/ios_shared_experiments_translator.cc b/components/feed/core/v2/ios_shared_experiments_translator.cc
index b2b3289a..e933cff 100644
--- a/components/feed/core/v2/ios_shared_experiments_translator.cc
+++ b/components/feed/core/v2/ios_shared_experiments_translator.cc
@@ -8,10 +8,15 @@
#include <string>
#include <vector>
+#include "base/strings/string_number_conversions.h"
#include "components/feed/feed_feature_list.h"
namespace feed {
+bool ExperimentGroup::operator==(const ExperimentGroup& rhs) const {
+ return name == rhs.name && experiment_id == rhs.experiment_id;
+}
+
std::optional<Experiments> TranslateExperiments(
const google::protobuf::RepeatedPtrField<feedwire::Experiment>&
wire_experiments) {
@@ -23,20 +28,16 @@
if (exp.has_trial_name() && exp.has_group_name()) {
// Extract experiment in response that contains both trial and
// group names.
- if (e.find(exp.trial_name()) != e.end()) {
- e[exp.trial_name()].push_back(exp.group_name());
- } else {
- e[exp.trial_name()] = {exp.group_name()};
+ ExperimentGroup group;
+ group.name = exp.group_name();
+ group.experiment_id = 0;
+ if (exp.has_experiment_id()) {
+ base::StringToInt(exp.experiment_id(), &(group.experiment_id));
}
- } else if (exp.has_experiment_id() &&
- base::FeatureList::IsEnabled(kFeedExperimentIDTagging)) {
- // Extract experiment in response that contains an experiment ID.
- std::string trial_name =
- exp.has_trial_name() ? exp.trial_name() : kDiscoverFeedExperiments;
- if (e.find(trial_name) != e.end()) {
- e[trial_name].push_back(exp.experiment_id());
+ if (e.find(exp.trial_name()) != e.end()) {
+ e[exp.trial_name()].push_back(group);
} else {
- e[trial_name] = {exp.experiment_id()};
+ e[exp.trial_name()] = {group};
}
}
}
diff --git a/components/feed/core/v2/ios_shared_experiments_translator.h b/components/feed/core/v2/ios_shared_experiments_translator.h
index 1f462129..4f3f41d 100644
--- a/components/feed/core/v2/ios_shared_experiments_translator.h
+++ b/components/feed/core/v2/ios_shared_experiments_translator.h
@@ -14,11 +14,16 @@
namespace feed {
-// A map of trial names (key) and list of group names/IDs (value)
-// sent from the server.
-typedef std::map<std::string, std::vector<std::string>> Experiments;
+struct ExperimentGroup {
+ std::string name;
+ int experiment_id;
-constexpr const char kDiscoverFeedExperiments[] = "DiscoverFeedExperiments";
+ bool operator==(const ExperimentGroup& rhs) const;
+};
+
+// A map of trial names (key) and list of groups.
+// sent from the server.
+typedef std::map<std::string, std::vector<ExperimentGroup>> Experiments;
std::optional<Experiments> TranslateExperiments(
const google::protobuf::RepeatedPtrField<feedwire::Experiment>&
diff --git a/components/feed/core/v2/ios_shared_experiments_translator_unittest.cc b/components/feed/core/v2/ios_shared_experiments_translator_unittest.cc
index 5c3202a..0fa70f6a 100644
--- a/components/feed/core/v2/ios_shared_experiments_translator_unittest.cc
+++ b/components/feed/core/v2/ios_shared_experiments_translator_unittest.cc
@@ -18,17 +18,13 @@
delete;
~ExperimentsTranslatorTest() override = default;
- void SetUp() override {
- feature_list_.InitAndEnableFeature(kFeedExperimentIDTagging);
- }
-
protected:
base::test::ScopedFeatureList feature_list_;
};
-TEST_F(ExperimentsTranslatorTest, ExperimentsAreTranslated) {
+TEST_F(ExperimentsTranslatorTest, ExperimentsAreTranslatedForGroupNameOnly) {
Experiments expected;
- std::vector<std::string> group_list{"Group1"};
+ std::vector<ExperimentGroup> group_list{{"Group1"}};
expected["Trial1"] = group_list;
feedwire::ChromeFeedResponseMetadata metadata;
@@ -42,38 +38,19 @@
EXPECT_EQ(e, expected);
}
-TEST_F(ExperimentsTranslatorTest, ExperimentsAreTranslatedIDTaggingEnabled) {
+TEST_F(ExperimentsTranslatorTest,
+ ExperimentsAreTranslatedForBothGroupNameAndExperimentId) {
Experiments expected;
- std::vector<std::string> group_list1{"ID1"};
- std::vector<std::string> group_list2{"ID2"};
- expected[kDiscoverFeedExperiments] = group_list1;
- expected["Trial1"] = group_list2;
-
- feedwire::ChromeFeedResponseMetadata metadata;
- auto* exp1 = metadata.add_experiments();
- exp1->set_experiment_id("ID1");
- auto* exp2 = metadata.add_experiments();
- exp2->set_trial_name("Trial1");
- exp2->set_experiment_id("ID2");
-
- std::optional<Experiments> e = TranslateExperiments(metadata.experiments());
- ASSERT_TRUE(e.has_value());
-
- EXPECT_EQ(e, expected);
-}
-
-TEST_F(ExperimentsTranslatorTest, ExperimentsAreTranslatedIDTaggingDisabled) {
- base::test::ScopedFeatureList feature_list;
- feature_list.InitAndDisableFeature(kFeedExperimentIDTagging);
- Experiments expected;
- std::vector<std::string> group_list{"Group1"};
+ std::vector<ExperimentGroup> group_list{{"Group1", 12345}};
expected["Trial1"] = group_list;
feedwire::ChromeFeedResponseMetadata metadata;
auto* exp1 = metadata.add_experiments();
exp1->set_trial_name("Trial1");
exp1->set_group_name("Group1");
+ exp1->set_experiment_id("12345");
+ // Group name must be present.
auto* exp2 = metadata.add_experiments();
exp2->set_experiment_id("EXP_ID_NOT_TRANSLATED");
auto* exp3 = metadata.add_experiments();
diff --git a/components/feed/core/v2/ios_shared_prefs.cc b/components/feed/core/v2/ios_shared_prefs.cc
index 63b2ebb..092d8de 100644
--- a/components/feed/core/v2/ios_shared_prefs.cc
+++ b/components/feed/core/v2/ios_shared_prefs.cc
@@ -10,6 +10,11 @@
namespace feed {
namespace prefs {
+namespace {
+const char kNameKey[] = "name";
+const char kIdKey[] = "id";
+} // namespace
+
void SetLastFetchHadNoticeCard(PrefService& pref_service, bool value) {
pref_service.SetBoolean(feed::prefs::kLastFetchHadNoticeCard, value);
}
@@ -53,25 +58,59 @@
for (const auto& exp : experiments) {
base::Value::List list;
for (auto elem : exp.second) {
- list.Append(elem);
+ base::Value::Dict group_dict;
+ group_dict.Set(kNameKey, elem.name);
+ group_dict.Set(kIdKey, elem.experiment_id);
+ list.Append(std::move(group_dict));
}
dict.Set(exp.first, std::move(list));
}
- pref_service.SetDict(kExperimentsV2, std::move(dict));
+ pref_service.SetDict(kExperimentsV3, std::move(dict));
}
Experiments GetExperiments(PrefService& pref_service) {
- const auto& dict = pref_service.GetDict(kExperimentsV2);
+ const auto& dict = pref_service.GetDict(kExperimentsV3);
Experiments experiments;
for (auto kv : dict) {
- std::vector<std::string> vect;
+ std::vector<ExperimentGroup> vect;
for (const auto& v : kv.second.GetList()) {
- vect.push_back(v.GetString());
+ ExperimentGroup group;
+ auto* group_dict = v.GetIfDict();
+ if (group_dict) {
+ const std::string* name = group_dict->FindString(kNameKey);
+ if (!name) {
+ continue;
+ }
+ group.name = *name;
+ group.experiment_id = group_dict->FindInt(kIdKey).value_or(0);
+ }
+ vect.push_back(group);
}
experiments[kv.first] = vect;
}
return experiments;
}
+void MigrateObsoleteFeedExperimentPref_Jun_2024(PrefService* prefs) {
+ const base::Value* val =
+ prefs->GetUserPrefValue(prefs::kExperimentsV2Deprecated);
+ const base::Value::Dict* old = val ? val->GetIfDict() : nullptr;
+ if (old) {
+ Experiments experiments;
+ for (const auto kv : *old) {
+ std::vector<ExperimentGroup> vect;
+ for (const auto& v : kv.second.GetList()) {
+ ExperimentGroup group;
+ group.name = v.GetString();
+ group.experiment_id = 0;
+ vect.push_back(group);
+ }
+ experiments[kv.first] = vect;
+ }
+ SetExperiments(experiments, *prefs);
+ }
+ prefs->ClearPref(prefs::kExperimentsV2Deprecated);
+}
+
} // namespace prefs
} // namespace feed
diff --git a/components/feed/core/v2/ios_shared_prefs.h b/components/feed/core/v2/ios_shared_prefs.h
index b4823e79..98f3389 100644
--- a/components/feed/core/v2/ios_shared_prefs.h
+++ b/components/feed/core/v2/ios_shared_prefs.h
@@ -14,6 +14,7 @@
using ::feed::Experiments;
namespace prefs {
+
void SetLastFetchHadNoticeCard(PrefService& pref_service, bool value);
bool GetLastFetchHadNoticeCard(const PrefService& pref_service);
@@ -34,6 +35,9 @@
// Set/get experiments into prefs.
void SetExperiments(const Experiments& experiments, PrefService& pref_service);
Experiments GetExperiments(PrefService& pref_service);
+
+void MigrateObsoleteFeedExperimentPref_Jun_2024(PrefService* prefs);
+
} // namespace prefs
} // namespace feed
diff --git a/components/feed/core/v2/ios_shared_prefs_unittest.cc b/components/feed/core/v2/ios_shared_prefs_unittest.cc
index d1e3afe..0011033 100644
--- a/components/feed/core/v2/ios_shared_prefs_unittest.cc
+++ b/components/feed/core/v2/ios_shared_prefs_unittest.cc
@@ -24,12 +24,40 @@
TEST_F(IOSSharedPrefsTest, TestSetAndGetExperiments) {
Experiments e;
- std::vector<std::string> group_list{"Group1"};
- e["Trial1"] = group_list;
+ std::vector<ExperimentGroup> group_list1{{"Group1", 123}, {"Group2", 9999}};
+ e["Trial1"] = group_list1;
+ std::vector<ExperimentGroup> group_list2{{"Hello", 12345}};
+ e["Trial2"] = group_list2;
prefs::SetExperiments(e, prefs_);
- ASSERT_TRUE(prefs_.HasPrefPath(prefs::kExperimentsV2));
+ ASSERT_TRUE(prefs_.HasPrefPath(prefs::kExperimentsV3));
+ EXPECT_EQ(e, prefs::GetExperiments(prefs_));
+}
+
+TEST_F(IOSSharedPrefsTest, MigrateExperimentsV2) {
+ // Save the experiments in the old format.
+ base::Value::Dict dict;
+ base::Value::List list1;
+ list1.Append("Group1");
+ list1.Append("Group2");
+ dict.Set("Trial1", std::move(list1));
+ base::Value::List list2;
+ list2.Append("Hello");
+ dict.Set("Trial2", std::move(list2));
+ prefs_.SetDict(prefs::kExperimentsV2Deprecated, std::move(dict));
+
+ prefs::MigrateObsoleteFeedExperimentPref_Jun_2024(&prefs_);
+
+ // Validate the migration.
+ ASSERT_FALSE(prefs_.HasPrefPath(prefs::kExperimentsV2Deprecated));
+ ASSERT_TRUE(prefs_.HasPrefPath(prefs::kExperimentsV3));
+
+ Experiments e;
+ std::vector<ExperimentGroup> group_list1{{"Group1", 0}, {"Group2", 0}};
+ e["Trial1"] = group_list1;
+ std::vector<ExperimentGroup> group_list2{{"Hello", 0}};
+ e["Trial2"] = group_list2;
EXPECT_EQ(e, prefs::GetExperiments(prefs_));
}
diff --git a/components/feed/core/v2/launch_reliability_logger.cc b/components/feed/core/v2/launch_reliability_logger.cc
index 263070d..e827afc 100644
--- a/components/feed/core/v2/launch_reliability_logger.cc
+++ b/components/feed/core/v2/launch_reliability_logger.cc
@@ -183,4 +183,12 @@
}
}
+void LaunchReliabilityLogger::ReportExperiments(
+ const std::vector<int32_t>& experiment_ids) {
+ for (const StreamSurfaceSet::Entry& entry : surfaces_->surfaces()) {
+ entry.renderer->GetReliabilityLoggingBridge().ReportExperiments(
+ experiment_ids);
+ }
+}
+
} // namespace feed
diff --git a/components/feed/core/v2/launch_reliability_logger.h b/components/feed/core/v2/launch_reliability_logger.h
index 4ab060c..16985bc 100644
--- a/components/feed/core/v2/launch_reliability_logger.h
+++ b/components/feed/core/v2/launch_reliability_logger.h
@@ -8,6 +8,7 @@
#include "base/memory/raw_ptr.h"
#include "base/time/time.h"
#include "components/feed/core/proto/v2/wire/reliability_logging_enums.pb.h"
+#include "components/feed/core/v2/ios_shared_experiments_translator.h"
#include "components/feed/core/v2/public/reliability_logging_bridge.h"
#include "components/feed/core/v2/public/surface_renderer.h"
#include "components/feed/core/v2/public/types.h"
@@ -65,6 +66,8 @@
void LogLaunchFinishedAfterStreamUpdate(
feedwire::DiscoverLaunchResult result);
+ void ReportExperiments(const std::vector<int32_t>& experiment_ids);
+
private:
raw_ptr<StreamSurfaceSet, DanglingUntriaged> surfaces_;
NetworkRequestId::Generator request_id_gen_;
diff --git a/components/feed/core/v2/protocol_translator_unittest.cc b/components/feed/core/v2/protocol_translator_unittest.cc
index c217997..55638ca 100644
--- a/components/feed/core/v2/protocol_translator_unittest.cc
+++ b/components/feed/core/v2/protocol_translator_unittest.cc
@@ -120,10 +120,6 @@
ProtocolTranslatorTest& operator=(const ProtocolTranslatorTest&) = delete;
~ProtocolTranslatorTest() override = default;
- void SetUp() override {
- feature_list_.InitAndEnableFeature(kFeedExperimentIDTagging);
- }
-
protected:
base::test::ScopedFeatureList feature_list_;
};
@@ -225,7 +221,7 @@
TEST_F(ProtocolTranslatorTest, ExperimentsAreTranslated) {
Experiments expected;
- std::vector<std::string> group_list{"Group1"};
+ std::vector<ExperimentGroup> group_list{{"Group1", 123}};
expected["Trial1"] = group_list;
feedwire::Response response = EmptyWireResponse();
@@ -235,65 +231,7 @@
->add_experiments();
exp->set_trial_name("Trial1");
exp->set_group_name("Group1");
-
- RefreshResponseData refresh = TranslateWireResponse(response);
- ASSERT_TRUE(refresh.experiments.has_value());
-
- EXPECT_EQ(refresh.experiments.value(), expected);
-}
-
-TEST_F(ProtocolTranslatorTest, ExperimentsAreTranslatedIDTaggingEnabled) {
- Experiments expected;
- std::vector<std::string> group_list1{"ID1"};
- std::vector<std::string> group_list2{"ID2"};
- expected[kDiscoverFeedExperiments] = group_list1;
- expected["Trial1"] = group_list2;
-
- feedwire::Response response = EmptyWireResponse();
- auto* exp1 = response.mutable_feed_response()
- ->mutable_feed_response_metadata()
- ->mutable_chrome_feed_response_metadata()
- ->add_experiments();
- exp1->set_experiment_id("ID1");
- auto* exp2 = response.mutable_feed_response()
- ->mutable_feed_response_metadata()
- ->mutable_chrome_feed_response_metadata()
- ->add_experiments();
- exp2->set_trial_name("Trial1");
- exp2->set_experiment_id("ID2");
-
- RefreshResponseData refresh = TranslateWireResponse(response);
- ASSERT_TRUE(refresh.experiments.has_value());
-
- EXPECT_EQ(refresh.experiments.value(), expected);
-}
-
-TEST_F(ProtocolTranslatorTest, ExperimentsAreTranslatedIDTaggingDisabled) {
- base::test::ScopedFeatureList feature_list;
- feature_list.InitAndDisableFeature(kFeedExperimentIDTagging);
- Experiments expected;
- std::vector<std::string> group_list{"Group1"};
- expected["Trial1"] = group_list;
-
- feedwire::Response response = EmptyWireResponse();
- auto* exp1 = response.mutable_feed_response()
- ->mutable_feed_response_metadata()
- ->mutable_chrome_feed_response_metadata()
- ->add_experiments();
- exp1->set_trial_name("Trial1");
- exp1->set_group_name("Group1");
-
- auto* exp2 = response.mutable_feed_response()
- ->mutable_feed_response_metadata()
- ->mutable_chrome_feed_response_metadata()
- ->add_experiments();
- exp2->set_experiment_id("EXP_ID_NOT_TRANSLATED");
- auto* exp3 = response.mutable_feed_response()
- ->mutable_feed_response_metadata()
- ->mutable_chrome_feed_response_metadata()
- ->add_experiments();
- exp3->set_trial_name("Trial_NOT_TRANSLATED");
- exp3->set_experiment_id("EXP_ID_NOT_TRANSLATED");
+ exp->set_experiment_id("123");
RefreshResponseData refresh = TranslateWireResponse(response);
ASSERT_TRUE(refresh.experiments.has_value());
diff --git a/components/feed/core/v2/public/reliability_logging_bridge.h b/components/feed/core/v2/public/reliability_logging_bridge.h
index 2bed5d6..1e2d3a0 100644
--- a/components/feed/core/v2/public/reliability_logging_bridge.h
+++ b/components/feed/core/v2/public/reliability_logging_bridge.h
@@ -7,6 +7,7 @@
#include "base/time/time.h"
#include "components/feed/core/proto/v2/wire/reliability_logging_enums.pb.h"
+#include "components/feed/core/v2/ios_shared_experiments_translator.h"
#include "components/feed/core/v2/public/stream_type.h"
#include "components/feed/core/v2/public/types.h"
@@ -64,6 +65,9 @@
virtual void LogLoadMoreRequestFinished(int canonical_status) = 0;
virtual void LogLoadMoreEnded(bool success) = 0;
+ virtual void ReportExperiments(
+ const std::vector<int32_t>& experiment_ids) = 0;
+
virtual ~ReliabilityLoggingBridge() = default;
};
diff --git a/components/feed/feed_feature_list.cc b/components/feed/feed_feature_list.cc
index 6adb25b..9fe10f0 100644
--- a/components/feed/feed_feature_list.cc
+++ b/components/feed/feed_feature_list.cc
@@ -106,10 +106,6 @@
"FeedNoViewCache",
base::FEATURE_ENABLED_BY_DEFAULT);
-BASE_FEATURE(kFeedExperimentIDTagging,
- "FeedExperimentIDTagging",
- base::FEATURE_ENABLED_BY_DEFAULT);
-
BASE_FEATURE(kFeedShowSignInCommand,
"FeedShowSignInCommand",
base::FEATURE_ENABLED_BY_DEFAULT);
diff --git a/components/feed/feed_feature_list.h b/components/feed/feed_feature_list.h
index 92297f1..c1e5dc35 100644
--- a/components/feed/feed_feature_list.h
+++ b/components/feed/feed_feature_list.h
@@ -88,9 +88,6 @@
// When enabled, no view cache is used.
BASE_DECLARE_FEATURE(kFeedNoViewCache);
-// When enabled, allow tagging experiments with only an experiment ID.
-BASE_DECLARE_FEATURE(kFeedExperimentIDTagging);
-
// When enabled, allow show sign in command to request a user signs in / syncs.
BASE_DECLARE_FEATURE(kFeedShowSignInCommand);
diff --git a/ios/chrome/browser/discover_feed/model/discover_feed_experiments_tracker.mm b/ios/chrome/browser/discover_feed/model/discover_feed_experiments_tracker.mm
index f8215004..c5bf0fd 100644
--- a/ios/chrome/browser/discover_feed/model/discover_feed_experiments_tracker.mm
+++ b/ios/chrome/browser/discover_feed/model/discover_feed_experiments_tracker.mm
@@ -27,9 +27,9 @@
// by design. We do not provide the variations IDs from the backend
// and do not attach them to the X-Client-Data header.
for (const auto& exp : experiments) {
- for (const auto& group_name : exp.second) {
+ for (const auto& group : exp.second) {
IOSChromeMetricsServiceAccessor::RegisterSyntheticFieldTrial(exp.first,
- group_name);
+ group.name);
}
}
}
