Bug 1574343 - Improve process labels and icons. r=davidwalsh,yulia
authorJason Laster <jlaster@mozilla.com>
Thu, 22 Aug 2019 20:03:58 +0000
changeset 489492 002962ee07d0856c78d5c9b7bb3097f81847ca9a
parent 489491 b973f83de2521ee23e6b2f9a96fa1ec37fb68a42
child 489493 1cf0189e17311d7a7793ad4935658f03881940c6
push id93383
push userjlaster@mozilla.com
push dateThu, 22 Aug 2019 20:09:51 +0000
treeherderautoland@002962ee07d0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdavidwalsh, yulia
bugs1574343
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1574343 - Improve process labels and icons. r=davidwalsh,yulia Differential Revision: https://phabricator.services.mozilla.com/D42591
devtools/client/debugger/src/actions/navigation.js
devtools/client/debugger/src/client/firefox/create.js
devtools/client/debugger/src/client/firefox/types.js
devtools/client/debugger/src/components/PrimaryPanes/SourcesTreeItem.js
devtools/client/debugger/src/components/SecondaryPanes/Thread.js
devtools/client/debugger/src/reducers/threads.js
devtools/client/debugger/src/types.js
devtools/client/debugger/src/utils/sources-tree/tests/addToTree.spec.js
devtools/client/debugger/src/utils/sources-tree/tests/getDirectories.spec.js
devtools/client/debugger/src/utils/sources-tree/tests/updateTree.spec.js
devtools/client/debugger/src/utils/threads.js
devtools/server/actors/targets/content-process.js
devtools/shared/fronts/descriptors/process.js
devtools/shared/fronts/targets/content-process.js
devtools/shared/fronts/targets/target-mixin.js
devtools/shared/fronts/targets/worker.js
--- a/devtools/client/debugger/src/actions/navigation.js
+++ b/devtools/client/debugger/src/actions/navigation.js
@@ -51,17 +51,22 @@ export function connect(
   canRewind: boolean,
   isWebExtension: boolean
 ) {
   return async function({ dispatch }: ThunkArgs) {
     await dispatch(updateThreads());
     dispatch(
       ({
         type: "CONNECT",
-        mainThread: { url, actor, type: -1, name: "" },
+        mainThread: {
+          url,
+          actor,
+          type: "main-thread",
+          name: L10N.getStr("mainThread"),
+        },
         canRewind,
         isWebExtension,
       }: Action)
     );
   };
 }
 
 /**
--- a/devtools/client/debugger/src/client/firefox/create.js
+++ b/devtools/client/debugger/src/client/firefox/create.js
@@ -68,17 +68,28 @@ export function createPause(
   return {
     ...packet,
     thread,
     frame: createFrame(thread, frame),
     frames: response.frames.map(createFrame.bind(null, thread)),
   };
 }
 
+function getTargetType(target: Target) {
+  if (target.isWorkerTarget) {
+    return "worker";
+  }
+
+  if (target.isContentProcess) {
+    return "content-process";
+  }
+
+  return "main-thread";
+}
+
 export function createThread(actor: string, target: Target): Thread {
   return {
     actor,
-    url: target.url || "",
-    // Ci.nsIWorkerDebugger.TYPE_DEDICATED
-    type: actor.includes("process") ? 1 : 0,
-    name: "",
+    url: target.url,
+    type: getTargetType(target),
+    name: target.name,
   };
 }
--- a/devtools/client/debugger/src/client/firefox/types.js
+++ b/devtools/client/debugger/src/client/firefox/types.js
@@ -220,18 +220,20 @@ export type Target = {
   root: any,
   navigateTo: ({ url: string }) => Promise<*>,
   listWorkers: () => Promise<*>,
   reload: () => Promise<*>,
   destroy: () => void,
   threadFront: ThreadFront,
   activeConsole: ConsoleClient,
 
+  name: string,
   isBrowsingContext: boolean,
   isContentProcess: boolean,
+  isWorkerTarget: boolean,
   traits: Object,
   chrome: Boolean,
   url: string,
   isAddon: Boolean,
 };
 
 /**
  * Clients for accessing the Firefox debug server and browser
--- a/devtools/client/debugger/src/components/PrimaryPanes/SourcesTreeItem.js
+++ b/devtools/client/debugger/src/components/PrimaryPanes/SourcesTreeItem.js
@@ -6,17 +6,17 @@
 
 import React, { Component } from "react";
 import { connect } from "../../utils/connect";
 import classnames from "classnames";
 import { showMenu } from "devtools-contextmenu";
 
 import SourceIcon from "../shared/SourceIcon";
 import AccessibleImage from "../shared/AccessibleImage";
-import { getDisplayName, isWorker } from "../../utils/threads";
+import { isWorker } from "../../utils/threads";
 
 import {
   getGeneratedSourceByURL,
   getHasSiblingOfSameName,
   hasPrettySource as checkHasPrettySource,
   getContext,
   getMainThread,
   getExtensionNameBySourceUrl,
@@ -224,17 +224,17 @@ class SourceTreeItem extends Component<P
       return <AccessibleImage className="extension" />;
     }
 
     // Threads level
     if (depth === 0 && projectRoot === "") {
       const thread = threads.find(thrd => thrd.actor == item.name);
 
       if (thread) {
-        const icon = thread === this.props.mainThread ? "window" : "worker";
+        const icon = isWorker(thread) ? "worker" : "window";
         return (
           <AccessibleImage
             className={classnames(icon, {
               debuggee: debuggeeUrl && debuggeeUrl.includes(item.name),
             })}
           />
         );
       }
@@ -264,19 +264,17 @@ class SourceTreeItem extends Component<P
   }
 
   renderItemName(depth) {
     const { item, threads, extensionName } = this.props;
 
     if (depth === 0) {
       const thread = threads.find(({ actor }) => actor == item.name);
       if (thread) {
-        return isWorker(thread)
-          ? getDisplayName((thread: any))
-          : L10N.getStr("mainThread");
+        return thread.name;
       }
     }
 
     if (isExtensionDirectory(depth, extensionName)) {
       return extensionName;
     }
 
     switch (item.name) {
--- a/devtools/client/debugger/src/components/SecondaryPanes/Thread.js
+++ b/devtools/client/debugger/src/components/SecondaryPanes/Thread.js
@@ -5,17 +5,17 @@
 // @flow
 
 import React, { Component } from "react";
 import { connect } from "../../utils/connect";
 import classnames from "classnames";
 
 import actions from "../../actions";
 import { getCurrentThread, getIsPaused, getContext } from "../../selectors";
-import { getDisplayName, isWorker } from "../../utils/threads";
+import { isWorker } from "../../utils/threads";
 import AccessibleImage from "../shared/AccessibleImage";
 
 import type { Context, Thread as ThreadType } from "../../types";
 
 type Props = {
   cx: Context,
   selectThread: typeof actions.selectThread,
   isPaused: boolean,
@@ -28,17 +28,17 @@ export class Thread extends Component<Pr
     const { thread } = this.props;
     this.props.selectThread(this.props.cx, thread.actor);
   };
 
   render() {
     const { currentThread, isPaused, thread } = this.props;
 
     const worker = isWorker(thread);
-    const label = worker ? getDisplayName(thread) : L10N.getStr("mainThread");
+    const label = thread.name;
 
     return (
       <div
         className={classnames("thread", {
           selected: thread.actor == currentThread,
         })}
         key={thread.actor}
         onClick={this.onSelectThread}
--- a/devtools/client/debugger/src/reducers/threads.js
+++ b/devtools/client/debugger/src/reducers/threads.js
@@ -20,38 +20,42 @@ export type ThreadsState = {
   threads: ThreadList,
   mainThread: Thread,
   isWebExtension: boolean,
 };
 
 export function initialThreadsState(): ThreadsState {
   return {
     threads: [],
-    mainThread: { actor: "", url: "", type: -1, name: "" },
+    mainThread: {
+      actor: "",
+      url: "",
+      type: "main-thread",
+      name: "",
+    },
     isWebExtension: false,
   };
 }
 
 export default function update(
   state: ThreadsState = initialThreadsState(),
   action: Action
 ): ThreadsState {
   switch (action.type) {
     case "CONNECT":
       return {
         ...state,
-        mainThread: { ...action.mainThread, name: L10N.getStr("mainThread") },
+        mainThread: action.mainThread,
         isWebExtension: action.isWebExtension,
       };
     case "INSERT_THREADS":
       return {
         ...state,
         threads: [...state.threads, ...action.threads],
       };
-
     case "REMOVE_THREADS":
       const { threads } = action;
       return {
         ...state,
         threads: state.threads.filter(w => !threads.includes(w.actor)),
       };
     case "NAVIGATE":
       return {
--- a/devtools/client/debugger/src/types.js
+++ b/devtools/client/debugger/src/types.js
@@ -459,17 +459,17 @@ export type Scope = {|
   },
   type: string,
   scopeKind: string,
 |};
 
 export type Thread = {
   +actor: ThreadId,
   +url: string,
-  +type: number,
+  +type: string,
   +name: string,
 };
 
 export type Worker = Thread;
 export type ThreadList = Array<Thread>;
 
 export type Cancellable = {
   cancel: () => void,
--- a/devtools/client/debugger/src/utils/sources-tree/tests/addToTree.spec.js
+++ b/devtools/client/debugger/src/utils/sources-tree/tests/addToTree.spec.js
@@ -189,17 +189,17 @@ describe("sources-tree", () => {
       const tree = createTree({
         sources: sourceMap,
         debuggeeUrl: "",
         threads: [
           {
             actor: "FakeThread",
             name: "FakeThread",
             url: "https://davidwalsh.name",
-            type: 1,
+            type: "worker",
           },
         ],
       }).sourceTree;
 
       expect(tree.contents[0].contents).toHaveLength(1);
       const subtree = tree.contents[0].contents[0];
       expect(subtree.contents).toHaveLength(2);
       expect(formatTree(tree)).toMatchSnapshot();
@@ -216,17 +216,17 @@ describe("sources-tree", () => {
       const sourceMap = { FakeThread: createSourcesMap(sources) };
       const tree = createTree({
         sources: sourceMap,
         debuggeeUrl: "",
         threads: [
           {
             actor: "FakeThread",
             url: "https://davidwalsh.name",
-            type: 1,
+            type: "worker",
             name: "FakeThread",
           },
         ],
       }).sourceTree;
       expect(formatTree(tree)).toMatchSnapshot();
     });
 
     it("does not attempt to add two of the same file", () => {
@@ -249,23 +249,23 @@ describe("sources-tree", () => {
       const tree = createTree({
         sources: sourceMap,
         debuggeeUrl: "https://davidwalsh.name",
         threads: [
           {
             actor: "FakeThread",
             name: "FakeThread",
             url: "https://davidwalsh.name",
-            type: 1,
+            type: "worker",
           },
           {
             actor: "FakeThread2",
             name: "FakeThread2",
             url: "https://davidwalsh.name/WorkerA.js",
-            type: 2,
+            type: "worker",
           },
         ],
       }).sourceTree;
 
       expect(tree.contents[0].contents).toHaveLength(1);
       const subtree = tree.contents[0].contents[0];
       expect(subtree.contents).toHaveLength(2);
       const subtree2 = tree.contents[1].contents[0];
--- a/devtools/client/debugger/src/utils/sources-tree/tests/getDirectories.spec.js
+++ b/devtools/client/debugger/src/utils/sources-tree/tests/getDirectories.spec.js
@@ -30,17 +30,17 @@ describe("getDirectories", () => {
       "http://a/c.js",
       "http://b/c.js",
     ]);
 
     const threads = [
       {
         actor: "FakeThread",
         url: "http://a",
-        type: 1,
+        type: "worker",
         name: "FakeThread",
       },
     ];
 
     const debuggeeUrl = "http://a/";
     const { sourceTree } = createTree({
       sources,
       debuggeeUrl,
@@ -73,17 +73,17 @@ describe("findSourceTreeNodes", () => {
       "http://src/utils/print.js",
       "http://workers/worker.js",
     ]);
 
     const threads = [
       {
         actor: "FakeThread",
         url: "http://a",
-        type: 1,
+        type: "worker",
         name: "FakeThread",
       },
     ];
 
     const debuggeeUrl = "http://a/";
     const { sourceTree } = createTree({
       sources,
       debuggeeUrl,
--- a/devtools/client/debugger/src/utils/sources-tree/tests/updateTree.spec.js
+++ b/devtools/client/debugger/src/utils/sources-tree/tests/updateTree.spec.js
@@ -36,17 +36,17 @@ const sources = [
     url: "https://davidwalsh.name/source2.js",
   },
 ];
 
 const threads = [
   {
     actor: "FakeThread",
     url: "https://davidwalsh.name",
-    type: 1,
+    type: "worker",
     name: "FakeThread",
   },
 ];
 
 const debuggeeUrl = "blah";
 
 describe("calls updateTree.js", () => {
   it("adds one source", () => {
--- a/devtools/client/debugger/src/utils/threads.js
+++ b/devtools/client/debugger/src/utils/threads.js
@@ -1,16 +1,11 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
 // @flow
 
-import { basename } from "./path";
 import type { Thread } from "../types";
 
-export function getDisplayName(thread: Thread) {
-  return basename(thread.url);
+export function isWorker(thread: Thread) {
+  return thread.type == "worker";
 }
-
-export function isWorker(thread: Thread) {
-  return thread.actor.includes("workerTarget");
-}
--- a/devtools/server/actors/targets/content-process.js
+++ b/devtools/server/actors/targets/content-process.js
@@ -126,18 +126,16 @@ const ContentProcessTargetActor = ActorC
     // target actor. But this actor isn't being used outside of tests yet.
     if (!this._promisesActor) {
       this._promisesActor = new PromisesActor(this.conn, this);
       this.manage(this._promisesActor);
     }
 
     return {
       actor: this.actorID,
-      name: "Content process",
-
       consoleActor: this._consoleActor.actorID,
       threadActor: this.threadActor.actorID,
       memoryActor: this.memoryActor.actorID,
       promisesActor: this._promisesActor.actorID,
 
       traits: {
         networkMonitor: false,
       },
--- a/devtools/shared/fronts/descriptors/process.js
+++ b/devtools/shared/fronts/descriptors/process.js
@@ -44,16 +44,17 @@ class ProcessDescriptorFront extends Fro
       front = new BrowsingContextTargetFront(this._client);
     } else {
       front = new ContentProcessTargetFront(this._client);
     }
     // As these fronts aren't instantiated by protocol.js, we have to set their actor ID
     // manually like that:
     front.actorID = form.actor;
     front.form(form);
+    front.processID = this.id;
     this.manage(front);
     return front;
   }
 
   async getTarget() {
     if (this._processTargetFront && this._processTargetFront.actorID) {
       return this._processTargetFront;
     }
--- a/devtools/shared/fronts/targets/content-process.js
+++ b/devtools/shared/fronts/targets/content-process.js
@@ -28,16 +28,20 @@ class ContentProcessTargetFront extends 
     // Do not use `form` name to avoid colliding with protocol.js's `form` method
     this.targetForm = json;
     // FF69 chromeDebugger naming has been renamed into threadActor and could be removed when FF69
     // is no longer supported
     this._threadActor = json.threadActor || json.chromeDebugger;
     this.targetForm.contextActor = this._threadActor;
   }
 
+  get name() {
+    return `Content Process ${this.processID}`;
+  }
+
   attach() {
     // All target actors have a console actor to attach.
     // All but xpcshell test actors... which is using a ContentProcessTargetActor
     if (this.targetForm.consoleActor) {
       return this.attachConsole();
     }
     return Promise.resolve();
   }
--- a/devtools/shared/fronts/targets/target-mixin.js
+++ b/devtools/shared/fronts/targets/target-mixin.js
@@ -252,17 +252,17 @@ function TargetMixin(parentClass) {
     // Tells us if the related actor implements BrowsingContextTargetActor
     // interface and requires to call `attach` request before being used and
     // `detach` during cleanup.
     get isBrowsingContext() {
       return this.typeName === "browsingContextTarget";
     }
 
     get name() {
-      if (this.isAddon) {
+      if (this.isAddon || this.isContentProcess) {
         return this.targetForm.name;
       }
       return this.title;
     }
 
     get title() {
       return this._title || this.url;
     }
--- a/devtools/shared/fronts/targets/worker.js
+++ b/devtools/shared/fronts/targets/worker.js
@@ -38,16 +38,20 @@ class WorkerTargetFront extends TargetMi
     // Do not use `form` name to avoid colliding with protocol.js's `form` method
     this.targetForm = json;
     this._url = json.url;
     this.type = json.type;
     this.scope = json.scope;
     this.fetch = json.fetch;
   }
 
+  get name() {
+    return this.url.split("/").pop();
+  }
+
   async attach() {
     if (this._attach) {
       return this._attach;
     }
     this._attach = (async () => {
       const response = await super.attach();
 
       const isServiceWorker = this.type === Ci.nsIWorkerDebugger.TYPE_SERVICE;