[ntp] Add timeout which aborts module load

All modules wait for each other before showing up. A timeout protects
against long loading modules. It is not clear yet what the best timeout
is. Thus, the timeout duration is Finch-configurable.

Fixed: 1154008
Change-Id: I1dcd29a168c4f01408979c439edb47903837371f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2623718
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843371}

chrome/browser/resources/new_tab_page/app.js

@@ -536,7 +536,8 @@ class AppElement extends PolymerElement {
       return;
     }
     this.moduleDescriptors_ =
-        await ModuleRegistry.getInstance().initializeModules();
+        await ModuleRegistry.getInstance().initializeModules(
+            loadTimeData.getInteger('modulesLoadTimeout'));
   }
 
   /** @private */

chrome/browser/resources/new_tab_page/modules/module_descriptor.js

@@ -47,9 +47,22 @@ export class ModuleDescriptor {
     return this.element_;
   }
 
-  async initialize() {
+  /**
+   * Initializes the module. On success, |this.element| will be populated after
+   * the returned promise has resolved.
+   * @param {number} timeout Timeout in milliseconds after which initialization
+   *     aborts.
+   * @return {!Promise}
+   */
+  async initialize(timeout) {
     const loadStartTime = BrowserProxy.getInstance().now();
-    this.element_ = await this.initializeCallback_();
+    this.element_ = await Promise.race([
+      this.initializeCallback_(), new Promise(resolve => {
+        BrowserProxy.getInstance().setTimeout(() => {
+          resolve(null);
+        }, timeout);
+      })
+    ]);
     if (!this.element_) {
       return;
     }

chrome/browser/resources/new_tab_page/modules/module_registry.js

@@ -29,10 +29,12 @@ export class ModuleRegistry {
   /**
    * Initializes the modules previously set via |registerModules| and returns
    * the initialized descriptors.
+   * @param {number} timeout Timeout in milliseconds after which initialization
+   *     of a particular module aborts.
    * @return {!Promise<!Array<!ModuleDescriptor>>}
    */
-  async initializeModules() {
-    await Promise.all(this.descriptors_.map(d => d.initialize()));
+  async initializeModules(timeout) {
+    await Promise.all(this.descriptors_.map(d => d.initialize(timeout)));
     return this.descriptors_.filter(descriptor => !!descriptor.element);
   }
 }

chrome/browser/search/ntp_features_unittest.cc

@@ -78,4 +78,22 @@ TEST(NTPFeaturesTest, LocalHistoryRepeatableQueriesFrequencyExponent) {
   EXPECT_EQ(2, frequency_exponent);
 }
 
+TEST(NTPFeaturesTest, ModulesLoadTimeout) {
+  base::test::ScopedFeatureList scoped_feature_list_;
+
+  // The default value can be overridden.
+  scoped_feature_list_.InitWithFeaturesAndParameters(
+      {{kModules, {{kNtpModulesLoadTimeoutMillisecondsParam, "123"}}}}, {});
+  base::TimeDelta timeout = GetModulesLoadTimeout();
+  EXPECT_EQ(123, timeout.InMilliseconds());
+
+  // If the timeout is not parsable to an unsigned integer, the default value is
+  // used.
+  scoped_feature_list_.Reset();
+  scoped_feature_list_.InitWithFeaturesAndParameters(
+      {{kModules, {{kNtpModulesLoadTimeoutMillisecondsParam, "j"}}}}, {});
+  timeout = GetModulesLoadTimeout();
+  EXPECT_EQ(3, timeout.InSeconds());
+}
+
 }  // namespace ntp_features

chrome/browser/ui/webui/new_tab_page/new_tab_page_ui.cc

@@ -100,6 +100,8 @@ content::WebUIDataSource* CreateNewTabPageUiHtmlSource(Profile* profile) {
       base::FeatureList::IsEnabled(ntp_features::kWebUIThemeModeDoodles));
   source->AddBoolean("modulesEnabled",
                      base::FeatureList::IsEnabled(ntp_features::kModules));
+  source->AddInteger("modulesLoadTimeout",
+                     ntp_features::GetModulesLoadTimeout().InMilliseconds());
 
   static constexpr webui::LocalizedString kStrings[] = {
       {"doneButton", IDS_DONE},

chrome/test/data/webui/new_tab_page/app_test.js

@@ -31,6 +31,7 @@ suite('NewTabPageAppTest', () => {
 
   suiteSetup(() => {
     loadTimeData.overrideValues({
+      modulesLoadTimeout: 0,
       realboxEnabled: false,
     });
   });

chrome/test/data/webui/new_tab_page/modules/module_descriptor_test.js

@@ -53,4 +53,20 @@ suite('NewTabPageModulesModuleDescriptorTest', () => {
     assertEquals(null, moduleDescriptor.element);
     assertEquals(0, testProxy.handler.getCallCount('onModuleLoaded'));
   });
+
+  test('module load times out', async () => {
+    // Arrange.
+    const moduleDescriptor = new ModuleDescriptor(
+        'foo', 100, () => new Promise(() => {}) /* Never resolves. */);
+
+    // Act.
+    const initializePromise = moduleDescriptor.initialize(123);
+    const [callback, timeout] = await testProxy.whenCalled('setTimeout');
+    callback();
+    await initializePromise;
+
+    // Assert.
+    assertEquals(null, moduleDescriptor.element);
+    assertEquals(123, timeout);
+  });
 });

chrome/test/data/webui/new_tab_page/modules/module_registry_test.js

@@ -18,7 +18,7 @@ suite('NewTabPageModulesModuleRegistryTest', () => {
     ]);
 
     // Act.
-    const modulesPromise = ModuleRegistry.getInstance().initializeModules();
+    const modulesPromise = ModuleRegistry.getInstance().initializeModules(0);
     // Delayed promise resolution to test async module instantiation.
     bazModuleResolver.resolve(bazModule);
     const modules = await modulesPromise;

components/search/ntp_features.cc

@@ -99,6 +99,8 @@ const char kNtpRepeatableQueriesFrequencyExponentParam[] =
 const char kNtpRepeatableQueriesInsertPositionParam[] =
     "NtpRepeatableQueriesInsertPosition";
 
+const char kNtpModulesLoadTimeoutMillisecondsParam[] =
+    "NtpModulesLoadTimeoutMillisecondsParam";
 const char kNtpStatefulTasksModuleDataParam[] =
     "NtpStatefulTasksModuleDataParam";
 
@@ -156,4 +158,16 @@ RepeatableQueriesInsertPosition GetRepeatableQueriesInsertPosition() {
                               : RepeatableQueriesInsertPosition::kStart;
 }
 
+base::TimeDelta GetModulesLoadTimeout() {
+  std::string param_value = base::GetFieldTrialParamValueByFeature(
+      kModules, kNtpModulesLoadTimeoutMillisecondsParam);
+  // If the field trial param is not found or cannot be parsed to an unsigned
+  // integer, return the default value.
+  unsigned int param_value_as_int = 0;
+  if (!base::StringToUint(param_value, &param_value_as_int)) {
+    return base::TimeDelta::FromSeconds(3);
+  }
+  return base::TimeDelta::FromMilliseconds(param_value_as_int);
+}
+
 }  // namespace ntp_features

components/search/ntp_features.h

@@ -57,6 +57,8 @@ enum class RepeatableQueriesInsertPosition {
   kEnd,        // At the end of MV tiles.
 };
 
+// Parameter determining the module load timeout.
+extern const char kNtpModulesLoadTimeoutMillisecondsParam[];
 // Parameter determining the type of stateful data to request.
 extern const char kNtpStatefulTasksModuleDataParam[];
 
@@ -72,6 +74,8 @@ double GetLocalHistoryRepeatableQueriesFrequencyExponent();
 // queries should be inserted.
 RepeatableQueriesInsertPosition GetRepeatableQueriesInsertPosition();
 
+// Returns the timeout after which the load of a module should be aborted.
+base::TimeDelta GetModulesLoadTimeout();
 }  // namespace ntp_features
 
 #endif  // COMPONENTS_SEARCH_NTP_FEATURES_H_

testing/variations/fieldtrial_testing_config.json

@@ -2349,6 +2349,9 @@
             "experiments": [
                 {
                     "name": "Enabled",
+                    "params": {
+                        "NtpModulesLoadTimeoutMillisecondsParam": "3000"
+                    },
                     "enable_features": [
                         "NtpModules",
                         "NtpShoppingTasksModule"