Cherry pick PR #7678: Change getFeature to async (#8172)

Refer to the original PR: #7678

Bug: 450080869

getFeature needs mojo call to access the browser process, therefore
changing its definition to async. getFeatureParam can access all the
params within the renderer process, so removing the mojo calls and
implementations to keep it synchronous.

---------

Co-authored-by: Yijia Zhang <yijiazhang@google.com>

Author: cobalt-github-releaser-bot

cobalt/browser/h5vcc_experiments/h5vcc_experiments_impl.cc

@@ -144,14 +144,6 @@ void H5vccExperimentsImpl::GetFeature(const std::string& feature_name,
   std::move(callback).Run(GetFeatureInternal(feature_name));
 }
 
-void H5vccExperimentsImpl::GetFeatureParam(
-    const std::string& feature_param_name,
-    GetFeatureParamCallback callback) {
-  std::string param_value = base::GetFieldTrialParamValue(
-      cobalt::kCobaltExperimentName, feature_param_name);
-  std::move(callback).Run(param_value);
-}
-
 void H5vccExperimentsImpl::GetLatestExperimentConfigHashData(
     GetLatestExperimentConfigHashDataCallback callback) {
   std::move(callback).Run(

cobalt/browser/h5vcc_experiments/h5vcc_experiments_impl.h

@@ -50,8 +50,6 @@ class H5vccExperimentsImpl
   void GetActiveExperimentConfigData(
       GetActiveExperimentConfigDataCallback) override;
   void GetFeature(const std::string& feature_name, GetFeatureCallback) override;
-  void GetFeatureParam(const std::string& feature_param_name,
-                       GetFeatureParamCallback) override;
   void GetLatestExperimentConfigHashData(
       GetLatestExperimentConfigHashDataCallback) override;
   void SetLatestExperimentConfigHashData(

cobalt/browser/h5vcc_experiments/public/mojom/h5vcc_experiments.mojom

@@ -29,10 +29,7 @@ enum OverrideState {
 interface H5vccExperiments {
     SetExperimentState(mojo_base.mojom.DictionaryValue experiment_config) => ();
     ResetExperimentState() => ();
-    [Sync]
     GetFeature(string feature_name) => (OverrideState feature_value);
-    [Sync]
-    GetFeatureParam(string feature_param_name) => (string feature_param_value);
     GetActiveExperimentConfigData() => (string active_experiment_config_data);
     GetLatestExperimentConfigHashData() => (string latest_experiment_config_hash_data);
     SetLatestExperimentConfigHashData(string hash_data) => ();

third_party/blink/renderer/modules/cobalt/h5vcc_experiments/h_5_vcc_experiments.cc

@@ -14,6 +14,7 @@
 
 #include "third_party/blink/renderer/modules/cobalt/h5vcc_experiments/h_5_vcc_experiments.h"
 
+#include "base/metrics/field_trial_params.h"
 #include "base/values.h"
 #include "cobalt/browser/constants/cobalt_experiment_names.h"
 #include "cobalt/browser/h5vcc_experiments/public/mojom/h5vcc_experiments.mojom-blink.h"
@@ -78,29 +79,26 @@ ScriptPromise H5vccExperiments::resetExperimentState(
   return resolver->Promise();
 }
 
-String H5vccExperiments::getFeature(const String& feature_name) {
+ScriptPromise H5vccExperiments::getFeature(ScriptState* script_state,
+                                           const String& feature_name,
+                                           ExceptionState& exception_state) {
+  auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(
+      script_state, exception_state.GetContext());
+  auto promise = resolver->Promise();
   EnsureReceiverIsBound();
-  h5vcc_experiments::mojom::blink::OverrideState feature_state;
-  remote_h5vcc_experiments_->GetFeature(feature_name, &feature_state);
-  switch (feature_state) {
-    case h5vcc_experiments::mojom::blink::OverrideState::OVERRIDE_USE_DEFAULT:
-      return V8OverrideState(V8OverrideState::Enum::kDEFAULT);
-    case h5vcc_experiments::mojom::blink::OverrideState::
-        OVERRIDE_ENABLE_FEATURE:
-      return V8OverrideState(V8OverrideState::Enum::kENABLED);
-    case h5vcc_experiments::mojom::blink::OverrideState::
-        OVERRIDE_DISABLE_FEATURE:
-      return V8OverrideState(V8OverrideState::Enum::kDISABLED);
-  }
-  NOTREACHED_NORETURN() << "Invalid feature OverrideState for feature "
-                        << feature_name;
+
+  ongoing_requests_.insert(resolver);
+  remote_h5vcc_experiments_->GetFeature(
+      feature_name,
+      WTF::BindOnce(&H5vccExperiments::OnGetFeature, WrapPersistent(this),
+                    WrapPersistent(resolver)));
+  return promise;
 }
 
 const String& H5vccExperiments::getFeatureParam(
     const String& feature_param_name) {
-  EnsureReceiverIsBound();
-  remote_h5vcc_experiments_->GetFeatureParam(feature_param_name,
-                                             &feature_param_value_);
+  feature_param_value_ = String::FromUTF8(base::GetFieldTrialParamValue(
+      cobalt::kCobaltExperimentName, feature_param_name.Utf8()));
   return feature_param_value_;
 }
 
@@ -217,6 +215,26 @@ void H5vccExperiments::OnResetExperimentState(ScriptPromiseResolver* resolver) {
   resolver->Resolve();
 }
 
+void H5vccExperiments::OnGetFeature(
+    ScriptPromiseResolver* resolver,
+    h5vcc_experiments::mojom::blink::OverrideState feature_state) {
+  ongoing_requests_.erase(resolver);
+  switch (feature_state) {
+    case h5vcc_experiments::mojom::blink::OverrideState::OVERRIDE_USE_DEFAULT:
+      resolver->Resolve(V8OverrideState(V8OverrideState::Enum::kDEFAULT));
+      return;
+    case h5vcc_experiments::mojom::blink::OverrideState::
+        OVERRIDE_ENABLE_FEATURE:
+      resolver->Resolve(V8OverrideState(V8OverrideState::Enum::kENABLED));
+      return;
+    case h5vcc_experiments::mojom::blink::OverrideState::
+        OVERRIDE_DISABLE_FEATURE:
+      resolver->Resolve(V8OverrideState(V8OverrideState::Enum::kDISABLED));
+      return;
+  }
+  NOTREACHED() << "Invalid feature OverrideState for feature";
+}
+
 void H5vccExperiments::OnConnectionError() {
   remote_h5vcc_experiments_.reset();
   HeapHashSet<Member<ScriptPromiseResolver>> h5vcc_experiments_promises;

third_party/blink/renderer/modules/cobalt/h5vcc_experiments/h_5_vcc_experiments.h

@@ -46,7 +46,7 @@ class MODULES_EXPORT H5vccExperiments final
                                    const ExperimentConfiguration*,
                                    ExceptionState&);
   ScriptPromise resetExperimentState(ScriptState*, ExceptionState&);
-  String getFeature(const String&);
+  ScriptPromise getFeature(ScriptState*, const String&, ExceptionState&);
   const String& getFeatureParam(const String&);
   ScriptPromise getActiveExperimentConfigData(ScriptState*, ExceptionState&);
   ScriptPromise getLatestExperimentConfigHashData(ScriptState*,
@@ -71,6 +71,8 @@ class MODULES_EXPORT H5vccExperiments final
   void OnSetFinchParameters(ScriptPromiseResolver*);
   void OnSetLatestExperimentConfigHashData(ScriptPromiseResolver*);
   void OnResetExperimentState(ScriptPromiseResolver*);
+  void OnGetFeature(ScriptPromiseResolver*,
+                    h5vcc_experiments::mojom::blink::OverrideState);
   void OnConnectionError();
   void EnsureReceiverIsBound();
   HeapMojoRemote<h5vcc_experiments::mojom::blink::H5vccExperiments>

third_party/blink/renderer/modules/cobalt/h5vcc_experiments/h_5_vcc_experiments.idl

@@ -40,7 +40,8 @@ interface H5vccExperiments {
 
   // The following APIs fetch Cobalt's current value for the given feature name
   // or feature param.
-  OverrideState getFeature(DOMString feature_name);
+  [CallWith=ScriptState, RaisesException]
+  Promise<OverrideState> getFeature(DOMString feature_name);
 
   DOMString getFeatureParam(DOMString feature_param_name);
 

third_party/blink/web_tests/wpt_internal/cobalt/h5vcc-experiments/h5vcc-experiments-getFeature.https.any.js

@@ -0,0 +1,20 @@
+// META: global=window
+// META: script=/resources/test-only-api.js
+// META: script=resources/automation.js
+h5vcc_experiments_tests(async (t, mockH5vccExperiments) => {
+  const { OverrideState } = await import(
+    '/gen/cobalt/browser/h5vcc_experiments/public/mojom/h5vcc_experiments.mojom.m.js');
+  const test_feature_name = "test_feature_name";
+
+  mockH5vccExperiments.stubResult(test_feature_name, OverrideState.OVERRIDE_USE_DEFAULT);
+  let actual = await window.h5vcc.experiments.getFeature(test_feature_name);
+  assert_equals(actual, 'DEFAULT');
+
+  mockH5vccExperiments.stubResult(test_feature_name, OverrideState.OVERRIDE_DISABLE_FEATURE);
+  actual = await window.h5vcc.experiments.getFeature(test_feature_name);
+  assert_equals(actual, 'DISABLED');
+
+  mockH5vccExperiments.stubResult(test_feature_name, OverrideState.OVERRIDE_ENABLE_FEATURE);
+  actual = await window.h5vcc.experiments.getFeature(test_feature_name);
+  assert_equals(actual, 'ENABLED');
+}, 'exercises H5vccExperiments.getFeature()');

third_party/blink/web_tests/wpt_internal/cobalt/h5vcc-experiments/h5vcc-experiments-setExperimentState.https.any.js

@@ -6,7 +6,21 @@ const test_experiment_config = {
     "fake_feature": true
   },
   "featureParams": {
-    "fake_feature_param": 1,
+    "fake_feature_param": 2.0,
+    "fake_feature_param_2": 2,
+    "fake_feature_param_3": 1.234,
+    "fake_feature_param_4": true,
+    "fake_feature_param_5": "test_param_string",
+  },
+  "activeExperimentConfigData": "test_config_data",
+  "latestExperimentConfigHashData": "test_config_hash",
+}
+
+const test_experiment_config_2 = {
+  "features": {
+    "fake_feature": true
+  },
+  "featureParams": {
   },
   "activeExperimentConfigData": "test_config_data",
   "latestExperimentConfigHashData": "test_config_hash",
@@ -18,12 +32,14 @@ h5vcc_experiments_tests(async (t, mockH5vccExperiments) => {
   assert_true(mockH5vccExperiments.hasCalledSetExperimentState());
 
   assert_true(mockH5vccExperiments.getStubResult('fake_feature'));
-  // All feature params are stored as strings and all numbers are 64-bit
-  // floating-point value in JS.
-  assert_equals(mockH5vccExperiments.getStubResult('fake_feature_param'), "1.000000");
+  // Floats that represent integers get converted into integer.
+  assert_equals(mockH5vccExperiments.getStubResult('fake_feature_param'), "2");
+  assert_equals(mockH5vccExperiments.getStubResult('fake_feature_param_2'), "2");
+  assert_equals(mockH5vccExperiments.getStubResult('fake_feature_param_3'), "1.234");
+  assert_equals(mockH5vccExperiments.getStubResult('fake_feature_param_4'), "true");
+  assert_equals(mockH5vccExperiments.getStubResult('fake_feature_param_5'), "test_param_string");
 }, 'setExperimentState() sends expected config content to browser endpoint');
 
-
 h5vcc_experiments_mojo_disconnection_tests(async (t) => {
   return promise_rejects_exactly(t, 'Mojo connection error.',
     window.h5vcc.experiments.setExperimentState(test_experiment_config));

…LatestExperimentConfigHashData.https.any …estExperimentConfigHashData.https.any.jsthird_party/blink/web_tests/wpt_internal/cobalt/h5vcc-experiments/h5vcc-experiments-setLatestExperimentConfigHashData.https.any renamed to third_party/blink/web_tests/wpt_internal/cobalt/h5vcc-experiments/h5vcc-experiments-setLatestExperimentConfigHashData.https.any.js third_party/blink/web_tests/wpt_internal/cobalt/h5vcc-experiments/h5vcc-experiments-setLatestExperimentConfigHashData.https.any renamed to third_party/blink/web_tests/wpt_internal/cobalt/h5vcc-experiments/h5vcc-experiments-setLatestExperimentConfigHashData.https.any.js

@@ -12,6 +12,7 @@ h5vcc_experiments_tests(async (t, mockH5vccExperiments) => {
 
 
 h5vcc_experiments_mojo_disconnection_tests(async (t) => {
+  const test_config_hash = "test_config_hash";
   return promise_rejects_exactly(t, 'Mojo connection error.',
     window.h5vcc.experiments.setLatestExperimentConfigHashData(test_config_hash));
 }, 'setLatestExperimentConfigHashData() rejects when unimplemented due to pipe closure');

third_party/blink/web_tests/wpt_internal/cobalt/h5vcc-experiments/resources/mock-h5vcc-experiments.js

@@ -49,25 +49,25 @@ class MockH5vccExperiments {
     this.called_set_latest_experiment_config_hash_data = true;
   }
 
-  // Added for stubbing getFeature() and getFeatureParam() result in tests.
+  // Added for stubbing getFeature() result in tests.
   stubResult(key, value) {
     this.stub_result_.set(key, value);
   }
 
   async getActiveExperimentConfigData() {
-    return this.stub_result_.get(this.STUB_KEY_ACTIVE_CONFIG_DATA);
+    return {
+      activeExperimentConfigData: this.stub_result_.get(this.STUB_KEY_ACTIVE_CONFIG_DATA)
+    };
   }
 
   getConfigHash() {
     return this.STUB_KEY_CONFIG_CONFIG_HASH;
   }
 
-  getFeature(feature_name) {
-    throw new Error('Sync methods not supported in MojoJS (b/406809316');
-  }
-
-  getFeatureParam(feature_param_name) {
-    throw new Error('Sync methods not supported in MojoJS (b/406809316');
+  async getFeature(feature_name) {
+    return {
+      featureValue: this.stub_result_.get(feature_name)
+    };
   }
 
   // Helper function to get stubbed feature state.
@@ -76,7 +76,9 @@ class MockH5vccExperiments {
   }
 
   async getLatestExperimentConfigHashData() {
-    return this.stub_result_.get(this.STUB_KEY_CONFIG_CONFIG_HASH);
+    return {
+      latestExperimentConfigHashData: this.stub_result_.get(this.STUB_KEY_CONFIG_CONFIG_HASH)
+    };
   }
 
   hasCalledResetExperimentState() {
@@ -154,8 +156,8 @@ class MockH5vccExperiments {
     return;
   }
 
-  async setLatestExperimentConfigHashData(config_hash) {
-    this.stubConfigHashData(config_hash);
+  async setLatestExperimentConfigHashData(hash_data) {
+    this.stubConfigHashData(hash_data);
     return;
   }
 }