Downloads Web UI: Use HTML Imports Polyfill

Update the chrome://downloads page to use the HTML imports polyfill, and remove the exception for this page to always enable HTML imports when it is disabled in Blink. There are a few additional changes required for this conversion, compared to Print Preview and extensions: (1) Main script needs to be wrapped in HTMLImports.whenReady(), since it is in the main HTML document. (2) Mojo web UI tests need to be updated, as they assume that all dependencies from the imports are ready in browsePreload, which is not correct when using the polyfill. (3) Add a non-bundled HTML file to import the downloads.mojom-lite.js script, so that it does not become a <script> in the main document when optimized. Bug: 925517 Change-Id: I430d9817b20ee83f849847d9978037c5a8d1ac58 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1610487 Commit-Queue: Rebekah Potter <rbpotter@chromium.org> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Cr-Commit-Position: refs/heads/master@{#659800}
Denger-Network · May 15, 2019 · 38eb56f · 38eb56f
1 parent 6417de7
commit 38eb56f
Show file tree

Hide file tree

Showing 13 changed files with 88 additions and 13 deletions.
diff --git a/chrome/browser/resources/downloads/BUILD.gn b/chrome/browser/resources/downloads/BUILD.gn
@@ -21,7 +21,9 @@ if (optimize_webui) {
     excludes = [
       "chrome://resources/js/mojo_bindings_lite.js",
       "chrome://downloads/downloads.mojom-lite.js",
+      "chrome://downloads/downloads.mojom-lite.html",
     ]
+    replace_for_html_imports_polyfill = "crisper.js"
 
     deps = [
       ":unpak",
@@ -92,6 +94,7 @@ js_library("downloads") {
     ":manager",
     "//ui/webui/resources/js:cr",
   ]
+  externs_list = [ "$externs_path/html_imports.js" ]
 }
 
 js_library("icon_loader") {

diff --git a/chrome/browser/resources/downloads/browser_proxy.html b/chrome/browser/resources/downloads/browser_proxy.html
@@ -1,4 +1,4 @@
 <link rel="import" href="chrome://resources/html/cr.html">
 <link rel="import" href="chrome://resources/mojo/mojo/public/js/mojo_bindings_lite.html">
-<script src="downloads.mojom-lite.js"></script>
+<link rel="import" href="chrome://downloads/downloads.mojom-lite.html">
 <script src="browser_proxy.js"></script>
diff --git a/chrome/browser/resources/downloads/downloads.html b/chrome/browser/resources/downloads/downloads.html
@@ -42,6 +42,8 @@
   <command id="clear-all-command" shortcut="Alt|c">
   <command id="undo-command" shortcut="Ctrl|z">
 </if>
+  <script src="chrome://resources/polymer/v1_0/html-imports/html-imports.min.js">
+  </script>
   <link rel="stylesheet" href="chrome://resources/css/text_defaults_md.css">
   <link rel="import" href="chrome://resources/html/cr.html">
   <link rel="import" href="chrome://resources/html/polymer.html">

diff --git a/chrome/browser/resources/downloads/downloads.js b/chrome/browser/resources/downloads/downloads.js
@@ -2,13 +2,21 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-window.addEventListener('load', function() {
-  downloads.Manager.onLoad().then(function() {
-    requestIdleCallback(function() {
-      chrome.send(
-          'metricsHandler:recordTime',
-          ['Download.ResultsRenderedTime', window.performance.now()]);
-      document.fonts.load('bold 12px Roboto');
+function loadDownloads() {
+  return HTMLImports.whenReady(function() {
+    downloads.Manager.onLoad().then(function() {
+      requestIdleCallback(function() {
+        chrome.send(
+            'metricsHandler:recordTime',
+            ['Download.ResultsRenderedTime', window.performance.now()]);
+        document.fonts.load('bold 12px Roboto');
+      });
     });
   });
-});
+}
+
+if (document.readyState === 'complete') {
+  loadDownloads();
+} else {
+  window.addEventListener('load', loadDownloads);
+}
diff --git a/chrome/browser/resources/downloads/downloads.mojom-lite.html b/chrome/browser/resources/downloads/downloads.mojom-lite.html
@@ -0,0 +1 @@
+<script src="downloads.mojom-lite.js"></script>
diff --git a/chrome/browser/resources/downloads/downloads_resources.grd b/chrome/browser/resources/downloads/downloads_resources.grd
@@ -18,6 +18,9 @@
       <include name="IDR_DOWNLOADS_IMAGES_NO_DOWNLOADS_SVG"
           file="images\no_downloads.svg"
           type="BINDATA" />
+      <include name="IDR_DOWNLOADS_MOJO_LITE_HTML"
+          file="downloads.mojom-lite.html"
+          type="BINDATA" />
       <include name="IDR_DOWNLOADS_MOJO_LITE_JS"
           file="${root_gen_dir}\chrome\browser\ui\webui\downloads\downloads.mojom-lite.js"
           use_base_dir="false"

diff --git a/chrome/browser/resources/downloads/downloads_resources_vulcanized.grd b/chrome/browser/resources/downloads/downloads_resources_vulcanized.grd
@@ -18,6 +18,9 @@
       <include name="IDR_DOWNLOADS_IMAGES_NO_DOWNLOADS_SVG"
                file="images\no_downloads.svg"
                type="BINDATA" />
+      <include name="IDR_DOWNLOADS_MOJO_LITE_HTML"
+               file="downloads.mojom-lite.html"
+               type="BINDATA" />
       <include name="IDR_DOWNLOADS_MOJO_LITE_JS"
                file="${root_gen_dir}\chrome\browser\ui\webui\downloads\downloads.mojom-lite.js"
                use_base_dir="false"

diff --git a/chrome/browser/ui/webui/downloads/downloads_ui.cc b/chrome/browser/ui/webui/downloads/downloads_ui.cc
@@ -39,6 +39,7 @@
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/resources/grit/webui_resources.h"
 
 using content::BrowserContext;
 using content::DownloadManager;
@@ -138,15 +139,19 @@ content::WebUIDataSource* CreateDownloadsUIHTMLSource(Profile* profile) {
                           IDR_DOWNLOADS_IMAGES_NO_DOWNLOADS_SVG);
   source->AddResourcePath("downloads.mojom-lite.js",
                           IDR_DOWNLOADS_MOJO_LITE_JS);
-
 #if BUILDFLAG(OPTIMIZE_WEBUI)
+  source->AddResourcePath("downloads.mojom-lite.html",
+                          IDR_DOWNLOADS_MOJO_LITE_HTML);
   source->AddResourcePath("crisper.js", IDR_DOWNLOADS_CRISPER_JS);
   source->SetDefaultResource(IDR_DOWNLOADS_VULCANIZED_HTML);
 #else
   for (size_t i = 0; i < kDownloadsResourcesSize; ++i) {
     source->AddResourcePath(kDownloadsResources[i].name,
                             kDownloadsResources[i].value);
   }
+  // Add the subpage loader, to load subpages in non-optimized builds.
+  source->AddResourcePath("subpage_loader.html", IDR_WEBUI_HTML_SUBPAGE_LOADER);
+  source->AddResourcePath("subpage_loader.js", IDR_WEBUI_JS_SUBPAGE_LOADER);
   source->SetDefaultResource(IDR_DOWNLOADS_DOWNLOADS_HTML);
 #endif
 

diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc
@@ -1603,7 +1603,8 @@ bool ChromeContentRendererClient::RequiresHtmlImports(const GURL& url) {
   // Imports Polyfill so that they will not require native imports. Return true
   // for only pages that have not been updated yet. See
   // https://crbug.com/937747.
-  bool canUsePolyfill = url.host() == chrome::kChromeUIExtensionsHost;
+  bool canUsePolyfill = url.host() == chrome::kChromeUIExtensionsHost ||
+                        url.host() == chrome::kChromeUIDownloadsHost;
 #if BUILDFLAG(ENABLE_PRINT_PREVIEW)
   canUsePolyfill |= url.host() == chrome::kChromeUIPrintHost;
 #endif

diff --git a/chrome/test/base/mojo_web_ui_browser_test.cc b/chrome/test/base/mojo_web_ui_browser_test.cc
@@ -8,6 +8,7 @@
 #include "base/bind_helpers.h"
 #include "base/macros.h"
 #include "base/path_service.h"
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/webui/web_ui_test_handler.h"
 #include "chrome/common/chrome_paths.h"
@@ -104,9 +105,22 @@ void MojoWebUIBrowserTest::BrowsePreload(const GURL& browse_to) {
   content::WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   if (use_mojo_lite_bindings_) {
+    base::string16 file_content =
+        l10n_util::GetStringUTF16(IDR_WEB_UI_TEST_MOJO_LITE_JS);
+    // The generated script assumes that mojo has already been imported by the
+    // page. This is not the case when native HTML imports are disabled. If
+    // the polyfill is in place, wait for HTMLImports.whenReady().
+    base::string16 wrapped_file_content =
+        base::UTF8ToUTF16(
+            "const promise = typeof HTMLImports === 'undefined' ? "
+            "Promise.resolve() : "
+            "new Promise(resolve => { "
+            "HTMLImports.whenReady(resolve); "
+            "}); "
+            "promise.then(() => {") +
+        file_content + base::UTF8ToUTF16("});");
     web_contents->GetMainFrame()->ExecuteJavaScriptForTests(
-        l10n_util::GetStringUTF16(IDR_WEB_UI_TEST_MOJO_LITE_JS),
-        base::NullCallback());
+        wrapped_file_content, base::NullCallback());
   } else {
     web_contents->GetMainFrame()->ExecuteJavaScriptForTests(
         l10n_util::GetStringUTF16(IDR_WEB_UI_TEST_MOJO_JS),

diff --git a/chrome/test/data/webui/downloads/downloads_browsertest.js b/chrome/test/data/webui/downloads/downloads_browsertest.js
@@ -22,6 +22,21 @@ DownloadsTest.prototype = {
     this.accessibilityAuditConfig.ignoreSelectors('humanLangMissing', 'html');
   },
 
+  /** @override */
+  loaderFile: 'subpage_loader.html',
+
+  // The name of the custom element under test. Should be overridden by
+  // subclasses that are loading the URL of a non-element.
+  get customElementName() {
+    const r = /chrome\:\/\/downloads\/([a-zA-Z-_]+)\.html/;
+    const result = r.exec(this.browsePreload);
+    if (!result || result.length < 1) {
+      // Loading the main page, so wait for downloads manager.
+      return 'downloads-manager';
+    }
+    return 'downloads-' + result[1].replace(/_/gi, '-');
+  },
+
   /** @override */
   runAccessibilityChecks: true,
 };
@@ -110,6 +125,14 @@ DownloadsUrlTest.prototype = {
 
   /** @override */
   browsePreload: 'chrome://downloads/a/b/',
+
+  /** @override */
+  loaderFile: '',
+
+  /** @override */
+  get customElementName() {
+    return null;
+  }
 };
 
 TEST_F('DownloadsUrlTest', 'All', function() {

diff --git a/testing/buildbot/filters/webui_html_imports_polyfill_browser_tests.filter b/testing/buildbot/filters/webui_html_imports_polyfill_browser_tests.filter
@@ -32,6 +32,11 @@ CrExtensionsKioskModeTest.*
 CrExtensionsRuntimeHostPermissionsTest.*
 CrExtensionsHostPermissionsToggleListTest.*
 
+DownloadsItemTest.*
+DownloadsManagerTest.*
+DownloadsToolbarTest.*
+DownloadsUrlTest.*
+
 PDFAnnotationsTest.*
 PDFExtensionClipboardTest.*
 PDFExtensionHitTestTest.*

diff --git a/third_party/closure_compiler/externs/html_imports.js b/third_party/closure_compiler/externs/html_imports.js
@@ -0,0 +1,7 @@
+/** @fileoverview Externs for HTML imports polyfill. */
+
+/** @see https://github.com/webcomponents/html-imports */
+var HTMLImports = {};
+
+/** @param {!Function} callback */
+HTMLImports.whenReady = function(callback) {};