Extensions Web UI: Use HTML Imports Polyfill

Bug: 925517 Change-Id: I0686f2462cfd8d4b1c7f03d677b890fb1345880f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1579324 Commit-Queue: Rebekah Potter <rbpotter@chromium.org> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Cr-Commit-Position: refs/heads/master@{#656614}
Denger-Network · May 4, 2019 · e29836b · e29836b
1 parent 8c63064
commit e29836b
Show file tree

Hide file tree

Showing 13 changed files with 148 additions and 49 deletions.
diff --git a/chrome/browser/resources/extensions/BUILD.gn b/chrome/browser/resources/extensions/BUILD.gn
@@ -17,6 +17,7 @@ optimize_webui("build") {
   insert_in_head = "<base href=\"chrome://extensions\">"
   input = rebase_path("$target_gen_dir/$unpak_folder", root_build_dir)
   js_out_files = [ "crisper.js" ]
+  replace_for_html_imports_polyfill = "crisper.js"
 
   deps = [
     ":unpak",

diff --git a/chrome/browser/resources/extensions/extensions.html b/chrome/browser/resources/extensions/extensions.html
@@ -55,6 +55,8 @@
   </style>
 </head>
 <body>
+  <script src="chrome://resources/polymer/v1_0/html-imports/html-imports.min.js">
+  </script>
   <extensions-manager></extensions-manager>
   <link rel="stylesheet" href="chrome://resources/css/text_defaults_md.css">
   <link rel="import" href="chrome://extensions/manager.html">

diff --git a/chrome/browser/resources/extensions/icons.html b/chrome/browser/resources/extensions/icons.html
@@ -6,14 +6,7 @@
 <defs>
   <!-- Custom SVG (bettes). -->
   <g id="unpacked">
-    <defs>
-      <style>
-        .cls-1 {
-          fill: none;
-        }
-      </style>
-    </defs>
-    <path class="cls-1" d="M0,0H24V24H0V0Z"></path>
+    <path class="cls-1" fill="none" d="M0,0H24V24H0V0Z"></path>
     <circle cx="9" cy="12" r="1"></circle>
     <path d="M20,5H4A2,2,0,0,0,2,7V17a2,2,0,0,0,2,2H20a2,2,0,0,0,2-2V7A2,2,0,0,0,20,5ZM9,17a5,5,0,1,1,5-5A5,5,0,0,1,9,17Zm11,1a1,1,0,1,1,1-1A1,1,0,0,1,20,18ZM20,8a1,1,0,1,1,1-1A1,1,0,0,1,20,8Z"></path>
   </g>

diff --git a/chrome/browser/ui/webui/extensions/extensions_ui.cc b/chrome/browser/ui/webui/extensions/extensions_ui.cc
@@ -42,6 +42,7 @@
 #include "extensions/common/extension_urls.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/resources/grit/webui_resources.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos_factory.h"
@@ -348,6 +349,9 @@ content::WebUIDataSource* CreateMdExtensionsSource(Profile* profile,
     source->AddResourcePath(kExtensionsResources[i].name,
                             kExtensionsResources[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_EXTENSIONS_EXTENSIONS_HTML);
 #endif
 

diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc
@@ -1602,9 +1602,9 @@ 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 = false;
+  bool canUsePolyfill = url.host() == chrome::kChromeUIExtensionsHost;
 #if BUILDFLAG(ENABLE_PRINT_PREVIEW)
-  canUsePolyfill = url.host() == chrome::kChromeUIPrintHost;
+  canUsePolyfill |= url.host() == chrome::kChromeUIPrintHost;
 #endif
   return url.SchemeIs(content::kChromeUIScheme) && !canUsePolyfill;
 }
diff --git a/chrome/test/data/webui/extensions/cr_extensions_browsertest.js b/chrome/test/data/webui/extensions/cr_extensions_browsertest.js
@@ -45,6 +45,25 @@ const CrExtensionsBrowserTest = class extends PolymerTest {
     return null;
   }
 
+  /** @override */
+  get loaderFile() {
+    return '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\:\/\/extensions\/(([a-zA-Z-_]+)\/)?([a-zA-Z-_]+)\.html/;
+    const result = r.exec(this.browsePreload);
+    if (result && result.length > 3) {
+      const element = result[3].replace(/_/gi, '-');
+      return result[2] === undefined ? 'extensions-' + element : element;
+    }
+
+    // Loading the main page, return extensions manager.
+    return 'extensions-manager';
+  }
+
   /** @param {string} testName The name of the test to run. */
   runMochaTest(testName) {
     runMochaTest(this.suiteName, testName);
@@ -207,6 +226,12 @@ CrExtensionsActivityLogTest = class extends CrExtensionsBrowserTest {
       'activity_log_test.js',
     ]);
   }
+
+  /** @override */
+  get customElementName() {
+    // This element's naming scheme is unusual.
+    return 'extensions-activity-log';
+  }
 };
 
 TEST_F('CrExtensionsActivityLogTest', 'All', () => {
@@ -239,7 +264,7 @@ TEST_F('CrExtensionsActivityLogHistoryTest', 'All', () => {
 CrExtensionsActivityLogHistoryItemTest = class extends CrExtensionsBrowserTest {
   /** @override */
   get browsePreload() {
-    return 'chrome://extensions/activity_log/activity_log_item.html';
+    return 'chrome://extensions/activity_log/activity_log_history_item.html';
   }
 
   get extraLibraries() {
@@ -801,6 +826,12 @@ CrExtensionsNavigationHelperTest = class extends CrExtensionsBrowserTest {
   get suiteName() {
     return extension_navigation_helper_tests.suiteName;
   }
+
+  /** @override */
+  get customElementName() {
+    // This test is verifying a class, not a custom element.
+    return null;
+  }
 };
 
 TEST_F('CrExtensionsNavigationHelperTest', 'Basic', function() {
@@ -826,8 +857,11 @@ TEST_F('CrExtensionsNavigationHelperTest', 'SupportedRoutes', function() {
 
 CrExtensionsErrorConsoleTest = class extends CrExtensionsBrowserTest {
   /** @override */
-  get suiteName() {
-    return 'ErrorConsoleTests';
+  get extraLibraries() {
+    return super.extraLibraries.concat([
+      '../settings/test_util.js',
+      'error_console_test.js',
+    ]);
   }
 
   /** @override */
@@ -848,38 +882,8 @@ CrExtensionsErrorConsoleTest = class extends CrExtensionsBrowserTest {
   }
 };
 
-TEST_F('CrExtensionsErrorConsoleTest', 'TestUpDownErrors', function() {
-  const STACK_ERRORS = 'li';
-  const ACTIVE_ERROR_IN_STACK = 'li[tabindex="0"]';
-
-  let initialFocus =
-      extension_test_util.findMatches(document, ACTIVE_ERROR_IN_STACK)[0];
-  assertTrue(!!initialFocus);
-  assertEquals(
-      1,
-      extension_test_util.findMatches(document, ACTIVE_ERROR_IN_STACK).length);
-  assertEquals(
-      4, extension_test_util.findMatches(document, STACK_ERRORS).length);
-
-  // Pressing up when the first item is focused should NOT change focus.
-  MockInteractions.keyDownOn(initialFocus, 38, '', 'ArrowUp');
-  assertEquals(
-      initialFocus,
-      extension_test_util.findMatches(document, ACTIVE_ERROR_IN_STACK)[0]);
-
-  // Pressing down when the first item is focused should change focus.
-  MockInteractions.keyDownOn(initialFocus, 40, '', 'ArrowDown');
-  assertNotEquals(
-      initialFocus,
-      extension_test_util.findMatches(document, ACTIVE_ERROR_IN_STACK)[0]);
-
-  // Pressing up when the second item is focused should focus the first again.
-  MockInteractions.keyDownOn(initialFocus, 38, '', 'ArrowUp');
-  assertEquals(
-      initialFocus,
-      extension_test_util.findMatches(document, ACTIVE_ERROR_IN_STACK)[0]);
-
-  testDone();
+TEST_F('CrExtensionsErrorConsoleTest', 'TestUpDownErrors', () => {
+  mocha.run();
 });
 
 ////////////////////////////////////////////////////////////////////////////////

diff --git a/chrome/test/data/webui/extensions/cr_extensions_interactive_ui_tests.js b/chrome/test/data/webui/extensions/cr_extensions_interactive_ui_tests.js
@@ -27,6 +27,11 @@ const CrExtensionsInteractiveUITest = class extends PolymerInteractiveUITest {
       '../settings/test_util.js',
     ];
   }
+
+  /** @override */
+  get loaderFile() {
+    return 'subpage_loader.html';
+  }
 };
 
 
@@ -44,6 +49,12 @@ CrExtensionsOptionsPageTest = class extends CrExtensionsInteractiveUITest {
     ]);
   }
 
+  /** @override */
+  get customElementName() {
+    // Wait for the manager since this test is loading the main page.
+    return 'extensions-manager';
+  }
+
   /** @override */
   testGenPreamble() {
     GEN('  InstallExtensionWithInPageOptions();');

diff --git a/chrome/test/data/webui/extensions/error_console_test.js b/chrome/test/data/webui/extensions/error_console_test.js
@@ -0,0 +1,44 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+/** @fileoverview Suite of tests for the extensions error console. */
+suite('CrExtensionsErrorConsoleTest', function() {
+  const STACK_ERRORS = 'li';
+  const ACTIVE_ERROR_IN_STACK = 'li[tabindex="0"]';
+
+  // Initialize an extension activity log item before each test.
+  setup(function() {
+    return test_util.waitForRender(document);
+  });
+
+  test('TestUpDownErrors', function() {
+    let initialFocus =
+        extension_test_util.findMatches(document, ACTIVE_ERROR_IN_STACK)[0];
+    assertTrue(!!initialFocus);
+    assertEquals(
+        1,
+        extension_test_util.findMatches(document, ACTIVE_ERROR_IN_STACK)
+            .length);
+    assertEquals(
+        4, extension_test_util.findMatches(document, STACK_ERRORS).length);
+
+    // Pressing up when the first item is focused should NOT change focus.
+    MockInteractions.keyDownOn(initialFocus, 38, '', 'ArrowUp');
+    assertEquals(
+        initialFocus,
+        extension_test_util.findMatches(document, ACTIVE_ERROR_IN_STACK)[0]);
+
+    // Pressing down when the first item is focused should change focus.
+    MockInteractions.keyDownOn(initialFocus, 40, '', 'ArrowDown');
+    assertNotEquals(
+        initialFocus,
+        extension_test_util.findMatches(document, ACTIVE_ERROR_IN_STACK)[0]);
+
+    // Pressing up when the second item is focused should focus the first again.
+    MockInteractions.keyDownOn(initialFocus, 38, '', 'ArrowUp');
+    assertEquals(
+        initialFocus,
+        extension_test_util.findMatches(document, ACTIVE_ERROR_IN_STACK)[0]);
+  });
+});
diff --git a/chrome/test/data/webui/extensions/manager_test.js b/chrome/test/data/webui/extensions/manager_test.js
@@ -30,7 +30,7 @@ cr.define('extension_manager_tests', function() {
 
     /** @param {string} viewElement */
     function assertViewActive(tagName) {
-      expectTrue(!!manager.$.viewManager.querySelector(`${tagName}.active`));
+      assertTrue(!!manager.$.viewManager.querySelector(`${tagName}.active`));
     }
 
     setup(function() {

diff --git a/chrome/test/data/webui/extensions/navigation_helper_test.js b/chrome/test/data/webui/extensions/navigation_helper_test.js
@@ -31,9 +31,18 @@ cr.define('extension_navigation_helper_tests', function() {
     let navigationHelper;
 
     setup(function() {
-      PolymerTest.clearBody();
-      Polymer.dom.flush();
-      navigationHelper = new extensions.NavigationHelper();
+      // In release mode tests, we need to wait for the page to actually import
+      // the navigation_helper.html file, since the tests do not navigate there
+      // directly. Wait for an element that imports this class to be defined.
+      const whenReady = ((typeof extensions !== 'undefined') &&
+                         !!extensions.NavigationHelper) ?
+          Promise.resolve() :
+          customElements.whenDefined('extensions-manager');
+      return whenReady.then(() => {
+        PolymerTest.clearBody();
+        Polymer.dom.flush();
+        navigationHelper = new extensions.NavigationHelper();
+      });
     });
 
     test(assert(TestNames.Basic), 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
@@ -3,6 +3,35 @@
 # HTML imports feature turned off, before the Blink team disables the feature by
 # default.
 
+CrExtensionsSidebarTest.*
+CrExtensionsToolbarTest.*
+CrExtensionsItemsTest.*
+CrExtensionsActivityLogTest.*
+CrExtensionsActivityLogHistoryTest.*
+CrExtensionsActivityLogHistoryItemTest.*
+CrExtensionsActivityLogStreamTest.*
+CrExtensionsActivityLogStreamItemTest.*
+CrExtensionsDetailViewTest.*
+CrExtensionsItemListTest.*
+CrExtensionsLoadErrorTest.*
+CrExtensionsManagerUnitTest.*
+CrExtensionsManagerUnitTestWithActivityLogFlag.*
+CrExtensionsManagerTestWithMultipleExtensionTypesInstalled.*
+CrExtensionsManagerTestWithIdQueryParam.*
+CrExtensionsManagerTestWithActivityLogFlag.*
+CrExtensionsShortcutTest.*
+CrExtensionsShortcutInputTest.*
+CrExtensionsPackDialogTest.*
+CrExtensionsOptionsDialogTest.*
+CrExtensionsErrorPageTest.*
+CrExtensionsCodeSectionTest.*
+CrExtensionsNavigationHelperTest.*
+CrExtensionsErrorConsoleTest.*
+CrExtensionsToggleRowTest.*
+CrExtensionsKioskModeTest.*
+CrExtensionsRuntimeHostPermissionsTest.*
+CrExtensionsHostPermissionsToggleListTest.*
+
 PDFAnnotationsTest.*
 PDFExtensionClipboardTest.*
 PDFExtensionHitTestTest.*

diff --git a/testing/buildbot/filters/webui_html_imports_polyfill_interactive_ui_tests.filter b/testing/buildbot/filters/webui_html_imports_polyfill_interactive_ui_tests.filter
@@ -3,6 +3,7 @@
 # HTML imports feature turned off, before the Blink team disables the feature by
 # default.
 
+CrExtensionsOptionsPageTest.*
 PrintPreviewButtonStripInteractiveTest.*
 PrintPreviewDestinationDialogInteractiveTest.*
 PrintPreviewNumberSettingsSectionInteractiveTest.BlurResetsEmptyInput

diff --git a/ui/webui/resources/html/subpage_loader.html b/ui/webui/resources/html/subpage_loader.html
@@ -1,3 +1,4 @@
+<meta charset="utf-8">
 <script src="chrome://resources/polymer/v1_0/html-imports/html-imports.min.js">
 </script>
 <script src="subpage_loader.js"></script>