Created attachment 182909 [details] [review]
viewSelector: allow programmatically switching to tabs

Add the idea of an 'id' for a tab, and add a public switchTab method
so you can switch to 'applications' or 'windows'. This will be useful
for performance tests that test tab switching performance.

Created attachment 182910 [details] [review]
perf: Add metrics for switching to the application tab

Measure how long it takes to switch from the windows tab to the
applications tab the first time and the second time.

Review of attachment 182909 [details] [review]:

Looks good.

::: js/ui/viewSelector.js
@@ +86,3 @@
 
+    _init: function(id, label, pageActor, a11yIcon) {
+        this.id = id;

Not that it matters much but any reason why this is public? Doesn't seem to be accessed from outside of the class.

Review of attachment 182910 [details] [review]:

Looks good modulo description inconsistency.

::: js/perf/core.js
@@ +69,3 @@
     Scripting.defineScriptEvent("afterShowHide", "After a show/hide cycle for the overview");
+    Scripting.defineScriptEvent("applicationsShowStart", "Starting to switch to application browser");
+    Scripting.defineScriptEvent("applicationsShowDone", "Done switching to application browser");

This is inconsistent with the above ... we should stick to either "application browser" or "applications tab" having to names for the same thing in the descriptions is kind of odd.

(In reply to comment #3)
> Review of attachment 182909 [details] [review]:
> 
> Looks good.
> 
> ::: js/ui/viewSelector.js
> @@ +86,3 @@
> 
> +    _init: function(id, label, pageActor, a11yIcon) {
> +        this.id = id;
> 
> Not that it matters much but any reason why this is public? Doesn't seem to be
> accessed from outside of the class.

It is:

+        for (let i = 0; i < this._tabs.length; i++)
+            if (this._tabs[i].id == id) {
+                this._switchTab(this._tabs[i]);
+                break;
+            }

(In reply to comment #4)
> Review of attachment 182910 [details] [review]:
> 
> Looks good modulo description inconsistency.
> 
> ::: js/perf/core.js
> @@ +69,3 @@
>      Scripting.defineScriptEvent("afterShowHide", "After a show/hide cycle for
> the overview");
> +    Scripting.defineScriptEvent("applicationsShowStart", "Starting to switch
> to application browser");
> +    Scripting.defineScriptEvent("applicationsShowDone", "Done switching to
> application browser");
> 
> This is inconsistent with the above ... we should stick to either "application
> browser" or "applications tab" having to names for the same thing in the
> descriptions is kind of odd.

Went with "applications view" for both, as closest to what we call it in the code.

Attachment 182909 [details] pushed as d16acc4 - viewSelector: allow programmatically switching to tabs