bind-gen: Fix context-dependent property installation

Bug: 839389
Change-Id: I99856b3f70ed21a8bd1b618dd877d41b6b83f3d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2220419Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774038}

third_party/blink/renderer/bindings/scripts/bind_gen/codegen_expr.py

@@ -143,7 +143,9 @@ def expr_uniq(terms):
     return uniq_terms
 
 
-def expr_from_exposure(exposure, global_names=None):
+def expr_from_exposure(exposure,
+                       global_names=None,
+                       may_use_feature_selector=False):
     """
     Returns an expression to determine whether this property should be exposed
     or not.
@@ -152,30 +154,75 @@ def expr_from_exposure(exposure, global_names=None):
         exposure: web_idl.Exposure of the target construct.
         global_names: When specified, it's taken into account that the global
             object implements |global_names|.
+        may_use_feature_selector: True enables use of ${feature_selector} iff
+            the exposure is context dependent.
     """
     assert isinstance(exposure, web_idl.Exposure)
     assert (global_names is None
             or (isinstance(global_names, (list, tuple))
                 and all(isinstance(name, str) for name in global_names)))
 
+    # The property exposures are categorized into three.
+    # - Unconditional: Always exposed.
+    # - Context-independent: Enabled per v8::Isolate.
+    # - Context-dependent: Enabled per v8::Context, e.g. origin trials.
+    #
+    # Context-dependent properties can be installed in two phases.
+    # - The first phase installs all the properties that are associated with the
+    #   features enabled at the moment.  This phase is represented by
+    #   FeatureSelector as FeatureSelector.IsAll().
+    # - The second phase installs the properties associated with the specified
+    #   feature.  This phase is represented as FeatureSelector.IsAny(feature).
+    #
+    # The exposure condition is represented as;
+    #   (and feature_selector-independent-term
+    #        (or
+    #         feature_selector-1st-phase-term
+    #         feature_selector-2nd-phase-term))
+    # which can be represented in more details as:
+    #   (and secure_context_term
+    #        uncond_exposed_term
+    #        (or
+    #         (and feature_selector.IsAll()  # 1st phase; all enabled
+    #              cond_exposed_term
+    #              feature_enabled_term)
+    #         (or exposed_selector_term      # 2nd phase; any selected
+    #             feature_selector_term)))
+    # where
+    #   secure_context_term represents [SecureContext=F1]
+    #   uncond_exposed_term represents [Exposed=(G1, G2)]
+    #   cond_exposed_term represents [Exposed(G1 F1, G2 F2)]
+    #   feature_enabled_term represents [RuntimeEnabled=(F1, F2)]
+    #   exposed_selector_term represents [Exposed(G1 F1, G2 F2)]
+    #   feature_selector_term represents [RuntimeEnabled=(F1, F2)]
+    uncond_exposed_terms = []
+    cond_exposed_terms = []
+    feature_enabled_terms = []
+    exposed_selector_terms = []
+    feature_selector_names = []  # Will turn into feature_selector.IsAnyOf(...)
+
     def ref_enabled(feature):
         arg = "${execution_context}" if feature.is_context_dependent else ""
         return _Expr("RuntimeEnabledFeatures::{}Enabled({})".format(
             feature, arg))
 
-    top_terms = [_Expr(True)]
+    def ref_selected(features):
+        feature_tokens = map(
+            lambda feature: "OriginTrialFeature::k{}".format(feature),
+            features)
+        return _Expr("${{feature_selector}}.IsAnyOf({})".format(
+            ", ".join(feature_tokens)))
 
     # [SecureContext]
     if exposure.only_in_secure_contexts is True:
-        top_terms.append(_Expr("${is_in_secure_context}"))
+        secure_context_term = _Expr("${is_in_secure_context}")
     elif exposure.only_in_secure_contexts is False:
-        top_terms.append(_Expr(True))
+        secure_context_term = _Expr(True)
     else:
         terms = map(ref_enabled, exposure.only_in_secure_contexts)
-        top_terms.append(
-            expr_or(
-                [_Expr("${is_in_secure_context}"),
-                 expr_not(expr_and(terms))]))
+        secure_context_term = expr_or(
+            [_Expr("${is_in_secure_context}"),
+             expr_not(expr_and(terms))])
 
     # [Exposed]
     GLOBAL_NAME_TO_EXECUTION_CONTEXT_TEST = {
@@ -190,7 +237,6 @@ def expr_from_exposure(exposure, global_names=None):
         "Worker": "IsWorkerGlobalScope",
         "Worklet": "IsWorkletGlobalScope",
     }
-    exposed_terms = []
     if global_names:
         matched_global_count = 0
         for entry in exposure.global_names_and_features:
@@ -198,39 +244,62 @@ def expr_from_exposure(exposure, global_names=None):
                 continue
             matched_global_count += 1
             if entry.feature:
-                exposed_terms.append(ref_enabled(entry.feature))
+                cond_exposed_terms.append(ref_enabled(entry.feature))
+                if entry.feature.is_context_dependent:
+                    feature_selector_names.append(entry.feature)
         assert (not exposure.global_names_and_features
                 or matched_global_count > 0)
     else:
         for entry in exposure.global_names_and_features:
-            terms = []
-            pred = GLOBAL_NAME_TO_EXECUTION_CONTEXT_TEST[entry.global_name]
-            terms.append(_Expr("${{execution_context}}->{}()".format(pred)))
-            if entry.feature:
-                terms.append(ref_enabled(entry.feature))
-            if terms:
-                exposed_terms.append(expr_and(terms))
-    if exposed_terms:
-        top_terms.append(expr_or(exposed_terms))
+            pred_term = _Expr("${{execution_context}}->{}()".format(
+                GLOBAL_NAME_TO_EXECUTION_CONTEXT_TEST[entry.global_name]))
+            if not entry.feature:
+                uncond_exposed_terms.append(pred_term)
+            else:
+                cond_exposed_terms.append(
+                    expr_and([pred_term, ref_enabled(entry.feature)]))
+                if entry.feature.is_context_dependent:
+                    exposed_selector_terms.append(
+                        expr_and([pred_term,
+                                  ref_selected([entry.feature])]))
 
     # [RuntimeEnabled]
     if exposure.runtime_enabled_features:
-        terms = map(ref_enabled, exposure.runtime_enabled_features)
-        top_terms.append(expr_or(terms))
-
-    return expr_and(top_terms)
-
-
-def expr_of_feature_selector(exposure):
-    """
-    Returns an expression that tells whether this property is a target of the
-    feature selector or not.
-
-    Args:
-        exposure: web_idl.Exposure of the target construct.
-    """
-    assert isinstance(exposure, web_idl.Exposure)
-
-    features = map(lambda feature: "OriginTrialFeature::k{}".format(feature),
-                   exposure.context_dependent_runtime_enabled_features)
-    return _Expr("${{feature_selector}}.AnyOf({})".format(", ".join(features)))
+        feature_enabled_terms.extend(
+            map(ref_enabled, exposure.runtime_enabled_features))
+        feature_selector_names.extend(
+            exposure.context_dependent_runtime_enabled_features)
+
+    # Build an expression.
+    top_level_terms = []
+    top_level_terms.append(secure_context_term)
+    if uncond_exposed_terms:
+        top_level_terms.append(expr_or(uncond_exposed_terms))
+
+    if not (may_use_feature_selector
+            and exposure.is_context_dependent(global_names)):
+        if cond_exposed_terms:
+            top_level_terms.append(expr_or(cond_exposed_terms))
+        if feature_enabled_terms:
+            top_level_terms.append(expr_and(feature_enabled_terms))
+        return expr_and(top_level_terms)
+
+    all_enabled_terms = [_Expr("${feature_selector}.IsAll()")]
+    if cond_exposed_terms:
+        all_enabled_terms.append(expr_or(cond_exposed_terms))
+    if feature_enabled_terms:
+        all_enabled_terms.append(expr_or(feature_enabled_terms))
+
+    selector_terms = []
+    if exposed_selector_terms:
+        selector_terms.append(expr_or(exposed_selector_terms))
+    if feature_selector_names:
+        selector_terms.append(ref_selected(feature_selector_names))
+
+    terms = []
+    terms.append(expr_and(all_enabled_terms))
+    if selector_terms:
+        terms.append(expr_or(selector_terms))
+    top_level_terms.append(expr_or(terms))
+
+    return expr_and(top_level_terms)

third_party/blink/renderer/bindings/scripts/bind_gen/interface.py

@@ -37,7 +37,6 @@ from .codegen_context import CodeGenContext
 from .codegen_expr import CodeGenExpr
 from .codegen_expr import expr_and
 from .codegen_expr import expr_from_exposure
-from .codegen_expr import expr_of_feature_selector
 from .codegen_expr import expr_or
 from .codegen_format import format_template as _format
 from .codegen_utils import component_export
@@ -4483,8 +4482,10 @@ def _make_property_entries_and_callback_defs(
         for member in members:
             is_context_dependent = member.exposure.is_context_dependent(
                 global_names)
-            exposure_conditional = expr_from_exposure(member.exposure,
-                                                      global_names)
+            exposure_conditional = expr_from_exposure(
+                member.exposure,
+                global_names=global_names,
+                may_use_feature_selector=True)
 
             if "PerWorldBindings" in member.extended_attributes:
                 worlds = (CodeGenContext.MAIN_WORLD,
@@ -5151,9 +5152,8 @@ def make_install_properties(cg_context, function_name, class_name,
 
     if is_context_dependent and install_prototype_object_node:
         body.extend([
-            CxxLikelyIfNode(
-                cond="${feature_selector}.AnyOf()",
-                body=[install_prototype_object_node]),
+            CxxLikelyIfNode(cond="${feature_selector}.IsAll()",
+                            body=[install_prototype_object_node]),
             EmptyNode(),
         ])
 
@@ -5181,11 +5181,6 @@ def make_install_properties(cg_context, function_name, class_name,
                 ]))
             body.append(EmptyNode())
         for conditional, entries in conditional_to_entries.items():
-            if is_context_dependent:
-                conditional = expr_and([
-                    expr_of_feature_selector(entries[0].property_.exposure),
-                    conditional,
-                ])
             body.append(
                 CxxUnlikelyIfNode(
                     cond=conditional,
@@ -6634,11 +6629,16 @@ using InstallFuncType =
 
         for member in itertools.chain(interface.attributes,
                                       interface.constants,
-                                      interface.operation_groups):
-            for feature in (member.exposure.
-                            context_dependent_runtime_enabled_features):
+                                      interface.operation_groups,
+                                      interface.exposed_constructs):
+            features = list(
+                member.exposure.context_dependent_runtime_enabled_features)
+            for entry in member.exposure.global_names_and_features:
+                if entry.feature and entry.feature.is_context_dependent:
+                    features.append(entry.feature)
+            for feature in features:
                 feature_to_interfaces.setdefault(feature, set()).add(interface)
-            if member.exposure.context_dependent_runtime_enabled_features:
+            if features:
                 set_of_interfaces.add(interface)
 
     switch_node = CxxSwitchNode(cond="${feature}")

third_party/blink/renderer/platform/bindings/v8_interface_bridge.h

@@ -53,16 +53,19 @@ class PLATFORM_EXPORT V8InterfaceBridgeBase {
     FeatureSelector& operator=(const FeatureSelector&) = default;
     FeatureSelector& operator=(FeatureSelector&&) = default;
 
+    // Returns true if all properties that are associated with the features
+    // enabled at this moment should be installed.
+    bool IsAll() const { return does_select_all_; }
+
     // Returns true if properties should be installed.  Arguments |featureN|
     // represent the origin trial features to which the properties are
-    // associated.  No argument means that the properties are not associated
-    // with any origin trial feature.
-    bool AnyOf() const { return does_select_all_; }
-    bool AnyOf(OriginTrialFeature feature1) const {
-      return does_select_all_ || selector_ == feature1;
+    // associated.
+    bool IsAnyOf(OriginTrialFeature feature1) const {
+      return selector_ == feature1;
     }
-    bool AnyOf(OriginTrialFeature feature1, OriginTrialFeature feature2) const {
-      return does_select_all_ || selector_ == feature1 || selector_ == feature2;
+    bool IsAnyOf(OriginTrialFeature feature1,
+                 OriginTrialFeature feature2) const {
+      return selector_ == feature1 || selector_ == feature2;
     }
 
    private: