Merge changes I2b470424,I8b45988a,Ib1f79cf4 into stable-3.3

* changes:
  Ship a list of default experiments from the backend and allow disabling
  Revert "Clean up comment experiment flags"
  Make UI experiments configurable from gerrit.config

Documentation/config-gerrit.txt

@@ -3358,6 +3358,37 @@ Defaults to all available options minus `CHANGE_ACTIONS`,
 config is backwards compatible with what the default was before the config
 was added.
 
+[[experiments]]
+=== Section experiments
+
+This section covers experimental new features. Gerrit's frontend uses experiments
+to research new behavior. Once the research is done, the experimental feature
+either stays and the experimentation flag gets removed, or the feature as a whole
+gets removed
+
+[[experiments.enabled]]experiments.enabled::
++
+List of experiments that are currently enabled. The release notes contain currently
+available experiments.
++
+We will not remove experiments in stable patch releases. They are likely to be
+removed in the next stable version.
+
+----
+[experiments]
+  enabled = ExperimentKey
+----
+
+[[experiments.disabled]]experiments.disabled::
++
+List of experiments that are currently disabled. The release notes contain currently
+available experiments. This list disables experiments with the given key that are
+either enabled by default or explicitly in the config.
+
+----
+[experiments]
+  disabled = ExperimentKey
+----
 
 [[ldap]]
 === Section ldap

java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java

@@ -20,6 +20,7 @@ import static java.util.stream.Collectors.toSet;
 
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.UsedAt;
@@ -37,15 +38,21 @@ import java.net.URISyntaxException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
+import org.eclipse.jgit.lib.Config;
 
 /** Helper for generating parts of {@code index.html}. */
 @UsedAt(Project.GOOGLE)
 public class IndexHtmlUtil {
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
+  static final ImmutableSet<String> DEFAULT_EXPERIMENTS =
+      ImmutableSet.of(
+          "UiFeature__patchset_comments", "UiFeature__patchset_choice_for_comment_links");
+
   private static final Gson GSON = OutputFormat.JSON_COMPACT.newGson();
   /**
    * Returns both static and dynamic parameters of {@code index.html}. The result is to be used when
@@ -53,6 +60,7 @@ public class IndexHtmlUtil {
    */
   public static ImmutableMap<String, Object> templateData(
       GerritApi gerritApi,
+      Config gerritServerConfig,
       String canonicalURL,
       String cdnPath,
       String faviconPath,
@@ -66,7 +74,13 @@ public class IndexHtmlUtil {
                 canonicalURL, cdnPath, faviconPath, urlParameterMap, urlInScriptTagOrdainer))
         .putAll(dynamicTemplateData(gerritApi, requestedURL));
 
-    Set<String> enabledExperiments = experimentData(urlParameterMap);
+    Set<String> enabledExperiments = new HashSet<>();
+    Arrays.stream(gerritServerConfig.getStringList("experiments", null, "enabled"))
+        .forEach(enabledExperiments::add);
+    DEFAULT_EXPERIMENTS.forEach(enabledExperiments::add);
+    Arrays.stream(gerritServerConfig.getStringList("experiments", null, "disabled"))
+        .forEach(enabledExperiments::remove);
+    experimentData(urlParameterMap).forEach(enabledExperiments::add);
     if (!enabledExperiments.isEmpty()) {
       data.put("enabledExperiments", serializeObject(GSON, enabledExperiments).toString());
     }

java/com/google/gerrit/httpd/raw/IndexServlet.java

@@ -34,6 +34,7 @@ import java.util.function.Function;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import org.eclipse.jgit.lib.Config;
 
 public class IndexServlet extends HttpServlet {
   private static final long serialVersionUID = 1L;
@@ -42,6 +43,7 @@ public class IndexServlet extends HttpServlet {
   @Nullable private final String cdnPath;
   @Nullable private final String faviconPath;
   private final GerritApi gerritApi;
+  private final Config gerritServerConfig;
   private final SoySauce soySauce;
   private final Function<String, SanitizedContent> urlOrdainer;
 
@@ -49,11 +51,13 @@ public class IndexServlet extends HttpServlet {
       @Nullable String canonicalUrl,
       @Nullable String cdnPath,
       @Nullable String faviconPath,
-      GerritApi gerritApi) {
+      GerritApi gerritApi,
+      Config gerritServerConfig) {
     this.canonicalUrl = canonicalUrl;
     this.cdnPath = cdnPath;
     this.faviconPath = faviconPath;
     this.gerritApi = gerritApi;
+    this.gerritServerConfig = gerritServerConfig;
     this.soySauce =
         SoyFileSet.builder()
             .add(Resources.getResource("com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy"))
@@ -74,7 +78,14 @@ public class IndexServlet extends HttpServlet {
       // TODO(hiesel): Remove URL ordainer as parameter once Soy is consistent
       ImmutableMap<String, Object> templateData =
           IndexHtmlUtil.templateData(
-              gerritApi, canonicalUrl, cdnPath, faviconPath, parameterMap, urlOrdainer, requestUrl);
+              gerritApi,
+              gerritServerConfig,
+              canonicalUrl,
+              cdnPath,
+              faviconPath,
+              parameterMap,
+              urlOrdainer,
+              requestUrl);
       renderer = soySauce.renderTemplate("com.google.gerrit.httpd.raw.Index").setData(templateData);
     } catch (URISyntaxException | RestApiException e) {
       throw new IOException(e);

java/com/google/gerrit/httpd/raw/StaticModule.java

@@ -225,7 +225,7 @@ public class StaticModule extends ServletModule {
       String cdnPath =
           options.useDevCdn() ? options.devCdn() : cfg.getString("gerrit", null, "cdnPath");
       String faviconPath = cfg.getString("gerrit", null, "faviconPath");
-      return new IndexServlet(canonicalUrl, cdnPath, faviconPath, gerritApi);
+      return new IndexServlet(canonicalUrl, cdnPath, faviconPath, gerritApi, cfg);
     }
 
     @Provides

javatests/com/google/gerrit/httpd/raw/IndexServletTest.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.httpd.raw;
 
 import static com.google.common.truth.Truth.assertThat;
+import static java.util.stream.Collectors.joining;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -53,8 +54,15 @@ public class IndexServletTest {
     String testCanonicalUrl = "foo-url";
     String testCdnPath = "bar-cdn";
     String testFaviconURL = "zaz-url";
+
+    String disabledDefault = IndexHtmlUtil.DEFAULT_EXPERIMENTS.asList().get(0);
+    org.eclipse.jgit.lib.Config serverConfig = new org.eclipse.jgit.lib.Config();
+    serverConfig.setStringList(
+        "experiments", null, "enabled", ImmutableList.of("NewFeature", "DisabledFeature"));
+    serverConfig.setStringList(
+        "experiments", null, "disabled", ImmutableList.of("DisabledFeature", disabledDefault));
     IndexServlet servlet =
-        new IndexServlet(testCanonicalUrl, testCdnPath, testFaviconURL, gerritApi);
+        new IndexServlet(testCanonicalUrl, testCdnPath, testFaviconURL, gerritApi, serverConfig);
 
     FakeHttpServletResponse response = new FakeHttpServletResponse();
 
@@ -76,6 +84,15 @@ public class IndexServletTest {
                 + "'\\x7b\\x22\\/config\\/server\\/version\\x22: \\x22123\\x22, "
                 + "\\x22\\/config\\/server\\/info\\x22: \\x7b\\x22default_theme\\x22:"
                 + "\\x22my-default-theme\\x22\\x7d, \\x22\\/config\\/server\\/top-menus\\x22: "
-                + "\\x5b\\x5d\\x7d');</script>");
+                + "\\x5b\\x5d\\x7d');");
+    String enabledDefaults =
+        IndexHtmlUtil.DEFAULT_EXPERIMENTS.stream()
+            .filter(e -> !e.equals(disabledDefault))
+            .collect(joining("\\x22,"));
+    assertThat(output)
+        .contains(
+            "window.ENABLED_EXPERIMENTS = JSON.parse('\\x5b\\x22NewFeature\\x22,\\x22"
+                + enabledDefaults
+                + "\\x22\\x5d');</script>");
   }
 }

polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts

@@ -43,6 +43,7 @@ import {
   ReviewerState,
   SpecialFilePath,
 } from '../../../constants/constants';
+import {KnownExperimentId} from '../../../services/flags/flags';
 import {fetchChangeUpdates} from '../../../utils/patch-set-util';
 import {KeyboardShortcutMixin} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
 import {accountKey, removeServiceUsers} from '../../../utils/account-util';
@@ -379,6 +380,8 @@ export class GrReplyDialog extends KeyboardShortcutMixin(
     };
   }
 
+  _isPatchsetCommentsExperimentEnabled = false;
+
   constructor() {
     super();
     this.filterReviewerSuggestion = this._filterReviewerSuggestionGenerator(
@@ -418,6 +421,9 @@ export class GrReplyDialog extends KeyboardShortcutMixin(
   /** @override */
   ready() {
     super.ready();
+    this._isPatchsetCommentsExperimentEnabled = this.flagsService.isEnabled(
+      KnownExperimentId.PATCHSET_COMMENTS
+    );
     this.$.jsAPI.addElement(TargetElement.REPLY_DIALOG, this);
   }
 
@@ -680,13 +686,17 @@ export class GrReplyDialog extends KeyboardShortcutMixin(
     }
 
     if (this.draft) {
-      const comment: CommentInput = {
-        message: this.draft,
-        unresolved: !this._isResolvedPatchsetLevelComment,
-      };
-      reviewInput.comments = {
-        [SpecialFilePath.PATCHSET_LEVEL_COMMENTS]: [comment],
-      };
+      if (this._isPatchsetCommentsExperimentEnabled) {
+        const comment: CommentInput = {
+          message: this.draft,
+          unresolved: !this._isResolvedPatchsetLevelComment,
+        };
+        reviewInput.comments = {
+          [SpecialFilePath.PATCHSET_LEVEL_COMMENTS]: [comment],
+        };
+      } else {
+        reviewInput.message = this.draft;
+      }
     }
 
     const accountAdditions = new Map<AccountId | EmailAddress, boolean>();

polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts

@@ -303,14 +303,16 @@ export const htmlTemplate = html`
       </gr-endpoint-decorator>
     </section>
     <section class="previewContainer">
-      <label>
-        <input
-          id="resolvedPatchsetLevelCommentCheckbox"
-          type="checkbox"
-          checked="{{_isResolvedPatchsetLevelComment::change}}"
-        />
-        Resolved
-      </label>
+      <template is="dom-if" if="[[_isPatchsetCommentsExperimentEnabled]]">
+        <label>
+          <input
+            id="resolvedPatchsetLevelCommentCheckbox"
+            type="checkbox"
+            checked="{{_isResolvedPatchsetLevelComment::change}}"
+          />
+          Resolved
+        </label>
+      </template>
       <label class="preview-formatting">
         <input type="checkbox" checked="{{_previewFormatting::change}}" />
         Preview formatting

polygerrit-ui/app/services/flags/flags.ts

@@ -24,5 +24,7 @@ export interface FlagsService {
  * @desc Experiment ids used in Gerrit.
  */
 export enum KnownExperimentId {
+  PATCHSET_COMMENTS = 'UiFeature__patchset_comments',
+  PATCHSET_CHOICE_FOR_COMMENT_LINKS = 'UiFeature__patchset_choice_for_comment_links',
   NEW_CONTEXT_CONTROLS = 'UiFeature__new_context_controls',
 }