ComponentBase should enqueue itself to render when it becomes connected
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 22 Mar 2017 22:50:57 +0000 (22:50 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 22 Mar 2017 22:50:57 +0000 (22:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=169905

Reviewed by Antti Koivisto.

When a component becomes connected to a document, enqueue itself to render automatically.
Also added the support for boolean attribute to ComponentBase.createElement.

* ReadMe.md: Added an instruction to raise the upload limit per r214065.
* browser-tests/component-base-tests.js: Added tests for the new behavior and createElement. Also moved
the tests related to enqueueToRenderOnResize out of defineElement tests.

* browser-tests/index.html:
(BrowsingContext.prototype.constructor): Override requestAnimationFrame so that the callback would be
involved immediately durign testing.

* public/v3/components/base.js:
(ComponentBase): Enqueue itself to render during construction if custom elements is not available.
(ComponentBase.defineElement):
(ComponentBase.defineElement.elementClass.prototype.connectedCallback): Enqueue itself to render when
the component's element became connected.
(ComponentBase.createElement): Use Array.isArray instead of instanceof to make it work with arrays made
in other realms (global objects) during testing. Added the support for boolean attributes. Setting an
attribute value to true would set the attribute, and setting it to false would not set the attribute.
(ComponentBase.useNativeCustomElements): Added. True iff window.customElements is defined.

* public/v3/components/chart-pane-base.js:
(ChartPaneBase.prototype.render): No longer need to call enqueueToRender on the commit log viewer.

* public/v3/components/commit-log-viewer.js:
(CommitLogViewer.prototype.render): No longer need to call enqueueToRender on the spinner icon.

* public/v3/models/time-series.js:
(TimeSeries): Made this a proper class declaration now that we don't include data.js after r213300.

* public/v3/pages/chart-pane.js:
(ChartPane.prototype._renderActionToolbar): No longer need to call enqueueToRender on the close icon.

* public/v3/pages/summary-page.js:
(SummaryPage.prototype._renderCell): No longer need to call enqueueToRender on the spinner icon.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@214280 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/ReadMe.md
Websites/perf.webkit.org/browser-tests/component-base-tests.js
Websites/perf.webkit.org/browser-tests/index.html
Websites/perf.webkit.org/public/v3/components/base.js
Websites/perf.webkit.org/public/v3/components/chart-pane-base.js
Websites/perf.webkit.org/public/v3/components/commit-log-viewer.js
Websites/perf.webkit.org/public/v3/models/time-series.js
Websites/perf.webkit.org/public/v3/pages/chart-pane.js
Websites/perf.webkit.org/public/v3/pages/summary-page.js

index def2144..09edbfe 100644 (file)
@@ -1,3 +1,46 @@
+2017-03-22  Ryosuke Niwa  <rniwa@webkit.org>
+
+        ComponentBase should enqueue itself to render when it becomes connected
+        https://bugs.webkit.org/show_bug.cgi?id=169905
+
+        Reviewed by Antti Koivisto.
+
+        When a component becomes connected to a document, enqueue itself to render automatically.
+        Also added the support for boolean attribute to ComponentBase.createElement.
+
+        * ReadMe.md: Added an instruction to raise the upload limit per r214065.
+        * browser-tests/component-base-tests.js: Added tests for the new behavior and createElement. Also moved
+        the tests related to enqueueToRenderOnResize out of defineElement tests.
+
+        * browser-tests/index.html:
+        (BrowsingContext.prototype.constructor): Override requestAnimationFrame so that the callback would be
+        involved immediately durign testing.
+
+        * public/v3/components/base.js:
+        (ComponentBase): Enqueue itself to render during construction if custom elements is not available.
+        (ComponentBase.defineElement):
+        (ComponentBase.defineElement.elementClass.prototype.connectedCallback): Enqueue itself to render when
+        the component's element became connected.
+        (ComponentBase.createElement): Use Array.isArray instead of instanceof to make it work with arrays made
+        in other realms (global objects) during testing. Added the support for boolean attributes. Setting an
+        attribute value to true would set the attribute, and setting it to false would not set the attribute.
+        (ComponentBase.useNativeCustomElements): Added. True iff window.customElements is defined.
+
+        * public/v3/components/chart-pane-base.js:
+        (ChartPaneBase.prototype.render): No longer need to call enqueueToRender on the commit log viewer.
+
+        * public/v3/components/commit-log-viewer.js:
+        (CommitLogViewer.prototype.render): No longer need to call enqueueToRender on the spinner icon.
+
+        * public/v3/models/time-series.js:
+        (TimeSeries): Made this a proper class declaration now that we don't include data.js after r213300.
+
+        * public/v3/pages/chart-pane.js:
+        (ChartPane.prototype._renderActionToolbar): No longer need to call enqueueToRender on the close icon.
+
+        * public/v3/pages/summary-page.js:
+        (SummaryPage.prototype._renderCell): No longer need to call enqueueToRender on the spinner icon.
+
 2017-03-20  Ryosuke Niwa  <rniwa@webkit.org>
 
         Delete another function that was supposed to be removed in the previous commit.
index 8f21410..6cfc0e3 100644 (file)
@@ -84,6 +84,13 @@ The apache logs are located at `/private/var/log/apache2`.
         Options Indexes MultiViews
         php_flag zlib.output_compression on
 
+ 4. Increase the maximum upload size as needed for accepting custom roots:
+
+        <IfModule php5_module>
+        php_value upload_max_filesize 100M
+        php_value post_max_size 100M
+        </IfModule>
+
 ### Protecting the Administrative Pages to Prevent Execution of Arbitrary Code
 
 By default, the application gives the administrative privilege to everyone. Anyone can add, remove, or edit tests,
index 86c03e2..00c7a28 100644 (file)
@@ -18,6 +18,78 @@ describe('ComponentBase', function() {
         });
     }
 
+    it('must enqueue a connected component to render', () => {
+        const context = new BrowsingContext();
+        return context.importScripts(['instrumentation.js', 'components/base.js'], 'ComponentBase').then((ComponentBase) => {
+            let renderCall = 0;
+            class SomeComponent extends ComponentBase {
+                render() { renderCall++; }
+            }
+            ComponentBase.defineElement('some-component', SomeComponent);
+
+            let requestAnimationFrameCount = 0;
+            let callback = null;
+            context.global.requestAnimationFrame = (newCallback) => {
+                callback = newCallback;
+                requestAnimationFrameCount++;
+            }
+
+            expect(requestAnimationFrameCount).to.be(0);
+            const instance = new SomeComponent;
+            context.document.body.appendChild(instance.element());
+            expect(requestAnimationFrameCount).to.be(1);
+            callback();
+            expect(renderCall).to.be(1);
+            expect(requestAnimationFrameCount).to.be(1);
+        });
+    });
+
+    it('must enqueue a connected component to render upon a resize event if enqueueToRenderOnResize is true', () => {
+        const context = new BrowsingContext();
+        return context.importScripts(['instrumentation.js', 'components/base.js'], 'ComponentBase').then((ComponentBase) => {
+            class SomeComponent extends ComponentBase {
+                static get enqueueToRenderOnResize() { return true; }
+            }
+            ComponentBase.defineElement('some-component', SomeComponent);
+
+            let requestAnimationFrameCount = 0;
+            let callback = null;
+            context.global.requestAnimationFrame = (newCallback) => {
+                callback = newCallback;
+                requestAnimationFrameCount++;
+            }
+
+            expect(requestAnimationFrameCount).to.be(0);
+            const instance = new SomeComponent;
+            context.global.dispatchEvent(new Event('resize'));
+            context.document.body.appendChild(instance.element());
+            context.global.dispatchEvent(new Event('resize'));
+            expect(requestAnimationFrameCount).to.be(1);
+        });
+    });
+
+    it('must not enqueue a disconnected component to render upon a resize event if enqueueToRenderOnResize is true', () => {
+        const context = new BrowsingContext();
+        return context.importScripts(['instrumentation.js', 'components/base.js'], 'ComponentBase').then((ComponentBase) => {
+            class SomeComponent extends ComponentBase {
+                static get enqueueToRenderOnResize() { return true; }
+            }
+            ComponentBase.defineElement('some-component', SomeComponent);
+
+            let requestAnimationFrameCount = 0;
+            let callback = null;
+            context.global.requestAnimationFrame = (newCallback) => {
+                callback = newCallback;
+                requestAnimationFrameCount++;
+            }
+
+            const instance = new SomeComponent;
+            expect(requestAnimationFrameCount).to.be(0);
+            context.global.dispatchEvent(new Event('resize'));
+            expect(requestAnimationFrameCount).to.be(0);
+        });
+    });
+
     describe('constructor', () => {
         it('is a function', () => {
             return new BrowsingContext().importScript('components/base.js', 'ComponentBase').then((ComponentBase) => {
@@ -406,6 +478,121 @@ describe('ComponentBase', function() {
         });
     });
 
+    describe('createElement()', () => {
+
+        it('should create an element of the specified name', () => {
+            const context = new BrowsingContext();
+            return context.importScript('components/base.js', 'ComponentBase').then((ComponentBase) => {
+                const div = ComponentBase.createElement('div');
+                expect(div).to.be.a(context.global.HTMLDivElement);
+            });
+        });
+
+        it('should create an element with the specified attributes', () => {
+            const context = new BrowsingContext();
+            return context.importScript('components/base.js', 'ComponentBase').then((ComponentBase) => {
+                const input = ComponentBase.createElement('input', {'title': 'hi', 'id': 'foo', 'required': false, 'checked': true});
+                expect(input).to.be.a(context.global.HTMLInputElement);
+                expect(input.attributes.length).to.be(3);
+                expect(input.attributes[0].localName).to.be('title');
+                expect(input.attributes[0].value).to.be('hi');
+                expect(input.attributes[1].localName).to.be('id');
+                expect(input.attributes[1].value).to.be('foo');
+                expect(input.attributes[2].localName).to.be('checked');
+                expect(input.attributes[2].value).to.be('checked');
+            });
+        });
+
+        it('should create an element with the specified event handlers and attributes', () => {
+            const context = new BrowsingContext();
+            return context.importScript('components/base.js', 'ComponentBase').then((ComponentBase) => {
+                let clickCount = 0;
+                const div = ComponentBase.createElement('div', {'title': 'hi', 'onclick': () => clickCount++});
+                expect(div).to.be.a(context.global.HTMLDivElement);
+                expect(div.attributes.length).to.be(1);
+                expect(div.attributes[0].localName).to.be('title');
+                expect(div.attributes[0].value).to.be('hi');
+                expect(clickCount).to.be(0);
+                div.click();
+                expect(clickCount).to.be(1);
+            });
+        });
+
+        it('should create an element with the specified children when there is no attribute specified', () => {
+            const context = new BrowsingContext();
+            return context.importScript('components/base.js', 'ComponentBase').then((ComponentBase) => {
+                const element = ComponentBase.createElement;
+                const span = element('span');
+                const div = element('div', [span, 'hi']);
+                expect(div).to.be.a(context.global.HTMLDivElement);
+                expect(div.attributes.length).to.be(0);
+                expect(div.childNodes.length).to.be(2);
+                expect(div.childNodes[0]).to.be(span);
+                expect(div.childNodes[1]).to.be.a(context.global.Text);
+                expect(div.childNodes[1].data).to.be('hi');
+            });
+        });
+
+        it('should create an element with the specified children when the second argument is a span', () => {
+            const context = new BrowsingContext();
+            return context.importScript('components/base.js', 'ComponentBase').then((ComponentBase) => {
+                const element = ComponentBase.createElement;
+                const span = element('span');
+                const div = element('div', span);
+                expect(div).to.be.a(context.global.HTMLDivElement);
+                expect(div.attributes.length).to.be(0);
+                expect(div.childNodes.length).to.be(1);
+                expect(div.childNodes[0]).to.be(span);
+            });
+        });
+
+        it('should create an element with the specified children when the second argument is a Text node', () => {
+            const context = new BrowsingContext();
+            return context.importScript('components/base.js', 'ComponentBase').then((ComponentBase) => {
+                const element = ComponentBase.createElement;
+                const text = context.document.createTextNode('hi');
+                const div = element('div', text);
+                expect(div).to.be.a(context.global.HTMLDivElement);
+                expect(div.attributes.length).to.be(0);
+                expect(div.childNodes.length).to.be(1);
+                expect(div.childNodes[0]).to.be(text);
+            });
+        });
+
+        it('should create an element with the specified children when the second argument is a component', () => {
+            const context = new BrowsingContext();
+            return context.importScript('components/base.js', 'ComponentBase').then((ComponentBase) => {
+                class SomeComponent extends ComponentBase { };
+                ComponentBase.defineElement('some-component', SomeComponent);
+                const element = ComponentBase.createElement;
+                const component = new SomeComponent;
+                const div = element('div', component);
+                expect(div).to.be.a(context.global.HTMLDivElement);
+                expect(div.attributes.length).to.be(0);
+                expect(div.childNodes.length).to.be(1);
+                expect(div.childNodes[0]).to.be(component.element());
+            });
+        });
+
+        it('should create an element with the specified attributes and children', () => {
+            const context = new BrowsingContext();
+            return context.importScript('components/base.js', 'ComponentBase').then((ComponentBase) => {
+                const element = ComponentBase.createElement;
+                const span = element('span');
+                const div = element('div', {'lang': 'en'}, [span, 'hi']);
+                expect(div).to.be.a(context.global.HTMLDivElement);
+                expect(div.attributes.length).to.be(1);
+                expect(div.attributes[0].localName).to.be('lang');
+                expect(div.attributes[0].value).to.be('en');
+                expect(div.childNodes.length).to.be(2);
+                expect(div.childNodes[0]).to.be(span);
+                expect(div.childNodes[1]).to.be.a(context.global.Text);
+                expect(div.childNodes[1].data).to.be('hi');
+            });
+        });
+
+    });
+
     describe('defineElement()', () => {
 
         it('must define a custom element with a class of an appropriate name', () => {
@@ -466,52 +653,6 @@ describe('ComponentBase', function() {
             });
         });
 
-        it('must enqueue a connected component to render upon a resize event if enqueueToRenderOnResize is true', () => {
-            const context = new BrowsingContext();
-            return context.importScripts(['instrumentation.js', 'components/base.js'], 'ComponentBase').then((ComponentBase) => {
-                class SomeComponent extends ComponentBase {
-                    static get enqueueToRenderOnResize() { return true; }
-                }
-                ComponentBase.defineElement('some-component', SomeComponent);
-
-                let requestAnimationFrameCount = 0;
-                let callback = null;
-                context.global.requestAnimationFrame = (newCallback) => {
-                    callback = newCallback;
-                    requestAnimationFrameCount++;
-                }
-
-                expect(requestAnimationFrameCount).to.be(0);
-                const instance = new SomeComponent;
-                context.global.dispatchEvent(new Event('resize'));
-                context.document.body.appendChild(instance.element());
-                context.global.dispatchEvent(new Event('resize'));
-                expect(requestAnimationFrameCount).to.be(1);
-            });
-        });
-
-        it('must not enqueue a disconnected component to render upon a resize event if enqueueToRenderOnResize is true', () => {
-            const context = new BrowsingContext();
-            return context.importScripts(['instrumentation.js', 'components/base.js'], 'ComponentBase').then((ComponentBase) => {
-                class SomeComponent extends ComponentBase {
-                    static get enqueueToRenderOnResize() { return true; }
-                }
-                ComponentBase.defineElement('some-component', SomeComponent);
-
-                let requestAnimationFrameCount = 0;
-                let callback = null;
-                context.global.requestAnimationFrame = (newCallback) => {
-                    callback = newCallback;
-                    requestAnimationFrameCount++;
-                }
-
-                const instance = new SomeComponent;
-                expect(requestAnimationFrameCount).to.be(0);
-                context.global.dispatchEvent(new Event('resize'));
-                expect(requestAnimationFrameCount).to.be(0);
-            });
-        });
-
     });
 
 });
index 6330452..f17b70e 100644 (file)
@@ -40,6 +40,9 @@ class BrowsingContext {
         iframe.style.top = '0px';
         BrowsingContext._iframes.push(iframe);
 
+        // Expedite calls to callbacks to make tests go faster.
+        iframe.contentWindow.requestAnimationFrame = (callback) => setTimeout(callback, 0);
+
         this.iframe = iframe;
         this.symbols = {};
         this.global = this.iframe.contentWindow;
index 365363d..caa8c91 100644 (file)
@@ -17,7 +17,9 @@ class ComponentBase {
         this._shadow = null;
         this._actionCallbacks = new Map;
 
-        if (!window.customElements && new.target.enqueueToRenderOnResize)
+        if (!ComponentBase.useNativeCustomElements)
+            this.enqueueToRender();
+        if (!ComponentBase.useNativeCustomElements && new.target.enqueueToRenderOnResize)
             ComponentBase._connectedComponentToRenderOnResize(this);
     }
 
@@ -174,7 +176,7 @@ class ComponentBase {
 
         const enqueueToRenderOnResize = elementInterface.enqueueToRenderOnResize;
 
-        if (!window.customElements)
+        if (!ComponentBase.useNativeCustomElements)
             return;
 
         class elementClass extends HTMLElement {
@@ -194,6 +196,7 @@ class ComponentBase {
 
             connectedCallback()
             {
+                this.component().enqueueToRender();
                 if (enqueueToRenderOnResize)
                     ComponentBase._connectedComponentToRenderOnResize(this.component());
             }
@@ -215,17 +218,19 @@ class ComponentBase {
     static createElement(name, attributes, content)
     {
         var element = document.createElement(name);
-        if (!content && (attributes instanceof Array || attributes instanceof Node
+        if (!content && (Array.isArray(attributes) || attributes instanceof Node
             || attributes instanceof ComponentBase || typeof(attributes) != 'object')) {
             content = attributes;
             attributes = {};
         }
 
         if (attributes) {
-            for (var name in attributes) {
+            for (let name in attributes) {
                 if (name.startsWith('on'))
                     element.addEventListener(name.substring(2), attributes[name]);
-                else
+                else if (attributes[name] === true)
+                    element.setAttribute(name, name);
+                else if (attributes[name] !== false)
                     element.setAttribute(name, attributes[name]);
             }
         }
@@ -283,6 +288,7 @@ class ComponentBase {
     }
 }
 
+ComponentBase.useNativeCustomElements = !!window.customElements;
 ComponentBase._componentByName = new Map;
 ComponentBase._componentByClass = new Map;
 ComponentBase._currentlyConstructedByInterface = new Map;
index ae1f1ea..ad37968 100644 (file)
@@ -256,10 +256,9 @@ class ChartPaneBase extends ComponentBase {
             this._mainChartStatus.enqueueToRender();
 
         var body = this.content().querySelector('.chart-pane-body');
-        if (this._openRepository) {
+        if (this._openRepository)
             body.classList.add('has-second-sidebar');
-            this._commitLogViewer.enqueueToRender();
-        } else
+        else
             body.classList.remove('has-second-sidebar');
 
         Instrumentation.endMeasuringTime('ChartPane', 'render');
index 9a5d174..215072a 100644 (file)
@@ -55,8 +55,6 @@ class CommitLogViewer extends ComponentBase {
 
     render()
     {
-        this.part('spinner').enqueueToRender();
-
         const shouldShowRepositoryName = this._repository && (this._commits || this._fetchingPromise);
         this.content('repository-name').textContent = shouldShowRepositoryName ? this._repository.name() : '';
         this.content('spinner-container').style.display = this._fetchingPromise ? null : 'none';
index 0b55868..2f76d53 100644 (file)
@@ -1,8 +1,6 @@
 'use strict';
 
-// v3 UI still relies on RunsData for associating metrics with units.
-// Use declartive syntax once that dependency has been removed.
-var TimeSeries = class {
+class TimeSeries {
     constructor()
     {
         this._data = [];
index 586cfe6..2dcf1d9 100644 (file)
@@ -247,8 +247,6 @@ class ChartPane extends ChartPaneBase {
         var link = ComponentBase.createLink;
         var self = this;
 
-        this.part('close').enqueueToRender();
-
         if (this._chartsPage.canBreakdown(platform, metric)) {
             actions.push(element('li', link('Breakdown', function () {
                 self._chartsPage.insertBreakdownPanesAfter(platform, metric, self);
index 595a6d3..9515b17 100644 (file)
@@ -121,8 +121,6 @@ class SummaryPage extends PageWithHeading {
 
     _renderCell(cell, spinner, anchor, ratioGraph, configurationGroup)
     {
-        spinner.enqueueToRender();
-
         if (configurationGroup.isFetching())
             cell.classList.add('fetching');
         else