From e19330a9dbfed9695ed7d4fea6206ab0a8715cc4 Mon Sep 17 00:00:00 2001
From: Viktar Donich <viktard@google.com>
Date: Mon, 13 Aug 2018 15:30:27 -0700
Subject: [PATCH] Install style modules in case plugins were installed early

Applies style modules ahead of loading all of the plugins. Also, apply
style modules before full rendering of the element.

This allows use of plugin styling in plugin bundles and applies plugin
styles before first render, applying site theme before the default one.

Change-Id: Ibfab5879ba49c0fa2c5eef0cdaa875fce1d35d1e
---
 .../gr-external-style/gr-external-style.js    | 27 +++++-
 .../gr-external-style_test.html               | 84 +++++++++++++++----
 2 files changed, 91 insertions(+), 20 deletions(-)

diff --git a/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.js b/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.js
index 51ad0b7eab..0e8bb459e0 100644
--- a/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.js
+++ b/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.js
@@ -22,24 +22,35 @@
 
     properties: {
       name: String,
+      _urlsImported: {
+        type: Array,
+        value() { return []; },
+      },
+      _stylesApplied: {
+        type: Array,
+        value() { return []; },
+      },
     },
 
     _import(url) {
+      if (this._urlsImported.includes(url)) { return Promise.resolve(); }
+      this._urlsImported.push(url);
       return new Promise((resolve, reject) => {
         this.importHref(url, resolve, reject);
       });
     },
 
     _applyStyle(name) {
+      if (this._stylesApplied.includes(name)) { return; }
+      this._stylesApplied.push(name);
       const s = document.createElement('style', 'custom-style');
       s.setAttribute('include', name);
       Polymer.dom(this.root).appendChild(s);
     },
 
-    ready() {
-      Gerrit.awaitPluginsLoaded().then(() => Promise.all(
-          Gerrit._endpoints.getPlugins(this.name).map(
-              pluginUrl => this._import(pluginUrl)))
+    _importAndApply() {
+      Promise.all(Gerrit._endpoints.getPlugins(this.name).map(
+          pluginUrl => this._import(pluginUrl))
       ).then(() => {
         const moduleNames = Gerrit._endpoints.getModules(this.name);
         for (const name of moduleNames) {
@@ -47,5 +58,13 @@
         }
       });
     },
+
+    attached() {
+      this._importAndApply();
+    },
+
+    ready() {
+      Gerrit.awaitPluginsLoaded().then(() => this._importAndApply());
+    },
   });
 })();
diff --git a/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style_test.html b/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style_test.html
index d1893baf46..ec2888d13f 100644
--- a/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style_test.html
+++ b/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style_test.html
@@ -32,38 +32,90 @@ limitations under the License.
 
 <script>
   suite('gr-external-style integration tests', () => {
+    const TEST_URL = 'http://some/plugin/url.html';
+
     let sandbox;
     let element;
+    let plugin;
 
-    setup(done => {
-      sandbox = sinon.sandbox.create();
-
-      // NB: Order is important.
-      let plugin;
+    const installPlugin = () => {
+      if (plugin) { return; }
       Gerrit.install(p => {
         plugin = p;
-        plugin.registerStyleModule('foo', 'some-module');
-      }, '0.1', 'http://some/plugin/url.html');
-
-      sandbox.stub(Gerrit, 'awaitPluginsLoaded').returns(Promise.resolve());
+      }, '0.1', TEST_URL);
+    };
 
+    const createElement = () => {
       element = fixture('basic');
-      sandbox.stub(element, '_applyStyle');
-      sandbox.stub(element, 'importHref', (url, resolve) => { resolve(); });
+      sandbox.spy(element, '_applyStyle');
+    };
 
-      flush(done);
+    /**
+     * Installs the plugin, creates the element, registers style module.
+     */
+    const lateRegister = () => {
+      installPlugin();
+      createElement();
+      plugin.registerStyleModule('foo', 'some-module');
+    };
+
+    /**
+     * Installs the plugin, registers style module, creates the element.
+     */
+    const earlyRegister = () => {
+      installPlugin();
+      plugin.registerStyleModule('foo', 'some-module');
+      createElement();
+    };
+
+    setup(() => {
+      sandbox = sinon.sandbox.create();
+      stub('gr-external-style', {
+        importHref: (url, resolve) => resolve(),
+      });
+      sandbox.stub(Gerrit, 'awaitPluginsLoaded').returns(Promise.resolve());
     });
 
     teardown(() => {
       sandbox.restore();
     });
 
-    test('imports plugin-provided module', () => {
-      assert.isTrue(element.importHref.calledWith(
-          new URL('http://some/plugin/url.html')));
+    test('imports plugin-provided module', async () => {
+      lateRegister();
+      await new Promise(flush);
+      assert.isTrue(element.importHref.calledWith(new URL(TEST_URL)));
     });
 
-    test('applies plugin-provided styles', () => {
+    test('applies plugin-provided styles', async () => {
+      lateRegister();
+      await new Promise(flush);
+      assert.isTrue(element._applyStyle.calledWith('some-module'));
+    });
+
+    test('does not double import', async () => {
+      earlyRegister();
+      await new Promise(flush);
+      plugin.registerStyleModule('foo', 'some-module');
+      await new Promise(flush);
+      const urlsImported =
+          element._urlsImported.filter(url => url.toString() === TEST_URL);
+      assert.strictEqual(urlsImported.length, 1);
+    });
+
+    test('does not double apply', async () => {
+      earlyRegister();
+      await new Promise(flush);
+      plugin.registerStyleModule('foo', 'some-module');
+      await new Promise(flush);
+      const stylesApplied =
+          element._stylesApplied.filter(name => name === 'some-module');
+      assert.strictEqual(stylesApplied.length, 1);
+    });
+
+    test('loads and applies preloaded modules', async () => {
+      earlyRegister();
+      await new Promise(flush);
+      assert.isTrue(element.importHref.calledWith(new URL(TEST_URL)));
       assert.isTrue(element._applyStyle.calledWith('some-module'));
     });
   });