Fix the layout of the global process track with no thread

m1rp · Jul 30, 2019 · e2dc859 · e2dc859
1 parent 1fb607c
commit e2dc859
Show file tree

Hide file tree

Showing 3 changed files with 119 additions and 16 deletions.
diff --git a/src/components/timeline/GlobalTrack.js b/src/components/timeline/GlobalTrack.js
@@ -164,6 +164,7 @@ class GlobalTrackComponent extends PureComponent<Props> {
       style,
       localTracks,
       pid,
+      globalTrack,
     } = this.props;
 
     if (isHidden) {
@@ -192,11 +193,14 @@ class GlobalTrackComponent extends PureComponent<Props> {
             <button type="button" className="timelineTrackNameButton">
               {trackName}
               {
-                // Only show the PID if it is a real number. A string PID is an
-                // artificially generated value that is not useful, and a null
-                // value does not exist. */
+                // Only show the PID if:
+                //   1. It is a real number. A string PID is an artificially generated
+                //      value that is not useful, and a null value does not exist.
+                //   2. The global track actually points to a real thread. A stub
+                //      process track is created
               }
-              {typeof pid === 'number' ? (
+              {typeof pid === 'number' &&
+              globalTrack.mainThreadIndex !== null ? (
                 <div className="timelineTrackNameButtonAdditionalDetails">
                   PID: {pid}
                 </div>

diff --git a/src/test/components/GlobalTrack.test.js b/src/test/components/GlobalTrack.test.js
@@ -18,6 +18,7 @@ import { getSelectedThreadIndex } from '../../selectors/url-state';
 import { ensureExists } from '../../utils/flow';
 import mockCanvasContext from '../fixtures/mocks/canvas-context';
 import { getProfileWithNiceTracks } from '../fixtures/profiles/tracks';
+import { getProfileFromTextSamples } from '../fixtures/profiles/processed-profile';
 import { storeWithProfile } from '../fixtures/stores';
 import { getBoundingBox } from '../fixtures/utils';
 
@@ -27,15 +28,29 @@ const RIGHT_CLICK = 2;
 describe('timeline/GlobalTrack', function() {
   /**
    *  getProfileWithNiceTracks() looks like: [
-   *    'show [thread GeckoMain process]',
-   *    'show [thread GeckoMain tab]',       <- use this global track.
+   *    'show [thread GeckoMain process]',   // Track index 0
+   *    'show [thread GeckoMain tab]',       // Track index 1 (default)
    *    '  - show [thread DOM Worker]',
    *    '  - show [thread Style]',
+   *    'show [process]',                    // Track index 2
+   *    '  - show [thread NoMain]'
    *  ]
    */
-  function setup() {
-    const trackIndex = 1;
+  const GECKOMAIN_TAB_TRACK_INDEX = 1;
+  const NO_THREAD_TRACK_INDEX = 2;
+  function setup(trackIndex = GECKOMAIN_TAB_TRACK_INDEX) {
     const profile = getProfileWithNiceTracks();
+    {
+      // Add another thread to highlight a thread-less global process track.
+      const {
+        profile: {
+          threads: [thread],
+        },
+      } = getProfileFromTextSamples('A');
+      thread.name = 'NoMain';
+      thread.pid = 5555;
+      profile.threads.push(thread);
+    }
     const store = storeWithProfile(profile);
     const { getState, dispatch } = store;
     const trackReference = { type: 'global', trackIndex };
@@ -45,9 +60,6 @@ describe('timeline/GlobalTrack', function() {
       throw new Error('Expected a process track.');
     }
     const threadIndex = track.mainThreadIndex;
-    if (threadIndex === null) {
-      throw new Error('Expected the track to have a thread index.');
-    }
 
     // Some child components render to canvas.
     jest
@@ -57,8 +69,10 @@ describe('timeline/GlobalTrack', function() {
       .spyOn(HTMLElement.prototype, 'getBoundingClientRect')
       .mockImplementation(() => getBoundingBox(400, 400));
 
-    // The assertions are simpler if this thread is not already selected.
-    dispatch(changeSelectedThread(threadIndex + 1));
+    if (threadIndex !== null) {
+      // The assertions are simpler if the GeckoMain tab thread is not already selected.
+      dispatch(changeSelectedThread(threadIndex + 1));
+    }
 
     const renderResult = render(
       <Provider store={store}>
@@ -92,8 +106,13 @@ describe('timeline/GlobalTrack', function() {
     };
   }
 
-  it('matches the snapshot of a global track', () => {
-    const { container } = setup();
+  it('matches the snapshot of a global process track', () => {
+    const { container } = setup(GECKOMAIN_TAB_TRACK_INDEX);
+    expect(container.firstChild).toMatchSnapshot();
+  });
+
+  it('matches the snapshot of a global process track without a thread', () => {
+    const { container } = setup(NO_THREAD_TRACK_INDEX);
     expect(container.firstChild).toMatchSnapshot();
   });
 

diff --git a/src/test/components/__snapshots__/GlobalTrack.test.js.snap b/src/test/components/__snapshots__/GlobalTrack.test.js.snap
@@ -1,6 +1,6 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`timeline/GlobalTrack matches the snapshot of a global track 1`] = `
+exports[`timeline/GlobalTrack matches the snapshot of a global process track 1`] = `
 <li
   class="timelineTrack"
 >
@@ -172,3 +172,83 @@ process: \\"tab\\" (222)"
   </ol>
 </li>
 `;
+
+exports[`timeline/GlobalTrack matches the snapshot of a global process track without a thread 1`] = `
+<li
+  class="timelineTrack"
+>
+  <div
+    class="timelineTrackRow timelineTrackGlobalRow"
+  >
+    <div
+      class="react-contextmenu-wrapper timelineTrackLabel timelineTrackGlobalGrippy"
+    >
+      <button
+        class="timelineTrackNameButton"
+        type="button"
+      >
+        Process 5555
+      </button>
+    </div>
+    <div
+      class="timelineTrackTrack"
+    >
+      <div
+        class="timelineTrackThreadBlank"
+      />
+    </div>
+  </div>
+  <ol
+    class="timelineTrackLocalTracks"
+  >
+    <li
+      class="timelineTrack timelineTrackLocal"
+    >
+      <div
+        class="timelineTrackRow timelineTrackLocalRow"
+      >
+        <div
+          class="react-contextmenu-wrapper timelineTrackLabel timelineTrackLocalLabel timelineTrackLocalGrippy"
+          title="thread: \\"NoMain\\" (0)
+process: \\"default\\" (5555)"
+        >
+          <button
+            class="timelineTrackNameButton"
+            type="button"
+          >
+            NoMain
+          </button>
+        </div>
+        <div
+          class="timelineTrackTrack"
+        >
+          <div
+            class="timelineTrackThread"
+          >
+            <div
+              class="timelineMarkers"
+              data-testid="TimelineMarkersJank"
+            >
+              <canvas
+                class="timelineMarkersCanvas"
+              />
+            </div>
+            <div
+              class="threadActivityGraph"
+            >
+              <canvas
+                class="threadActivityGraphCanvas threadActivityGraphCanvas"
+                height="400"
+                width="400"
+              />
+            </div>
+            <div
+              class="timelineEmptyThreadIndicator"
+            />
+          </div>
+        </div>
+      </div>
+    </li>
+  </ol>
+</li>
+`;