Change screenshot markers into TracingMarkers

src/components/timeline/TrackScreenshots.js

@@ -14,12 +14,9 @@ import {
 } from '../../reducers/profile-view';
 import { withSize, type SizeProps } from '../shared/WithSize';
 
-import type {
-  ThreadIndex,
-  Thread,
-  MarkersTableWithPayload,
-} from '../../types/profile';
+import type { ThreadIndex, Thread } from '../../types/profile';
 import type { ScreenshotPayload } from '../../types/markers';
+import type { TracingMarker } from '../../types/profile-derived';
 import type { Milliseconds } from '../../types/units';
 import type {
   ExplicitConnectOptions,
@@ -38,7 +35,7 @@ type StateProps = {|
   +thread: Thread,
   +rangeStart: Milliseconds,
   +rangeEnd: Milliseconds,
-  +screenshots: MarkersTableWithPayload<ScreenshotPayload>,
+  +screenshots: TracingMarker[],
   +threadName: string,
   +isMakingPreviewSelection: boolean,
 |};
@@ -75,7 +72,7 @@ class Screenshots extends PureComponent<Props, State> {
     // Loop backwards to find the latest screenshot that has a time less
     // than the current time at the mouse position.
     for (let i = screenshots.length - 1; i >= 0; i--) {
-      const screenshotTime = screenshots.time[i];
+      const screenshotTime = screenshots[i].start;
       if (mouseTime >= screenshotTime) {
         return i;
       }
@@ -109,23 +106,22 @@ class Screenshots extends PureComponent<Props, State> {
 
     let screenshotIndex = 0;
     for (
-      let left = timeToPixel(screenshots.time[0]);
+      let left = timeToPixel(screenshots[0].start);
       left < outerContainerWidth;
       left += imageContainerWidth
     ) {
       // Try to find the next screenshot to fit in, or re-use the existing one.
       for (let i = screenshotIndex; i < screenshots.length; i++) {
-        if (timeToPixel(screenshots.time[i]) <= left) {
+        if (timeToPixel(screenshots[i].start) <= left) {
           screenshotIndex = i;
         } else {
           break;
         }
       }
-      const {
-        url: urlStringIndex,
-        windowWidth,
-        windowHeight,
-      } = screenshots.data[screenshotIndex];
+      // Coerce the payload into a screenshot one.
+      const payload: ScreenshotPayload = (screenshots[screenshotIndex]
+        .data: any);
+      const { url: urlStringIndex, windowWidth, windowHeight } = payload;
       const scaledImageWidth = TRACK_HEIGHT * windowWidth / windowHeight;
       images.push(
         <div
@@ -159,9 +155,9 @@ class Screenshots extends PureComponent<Props, State> {
     if (screenshotIndex === null) {
       return null;
     }
-    const { url, windowWidth, windowHeight } = screenshots.data[
-      screenshotIndex
-    ];
+    // Coerce the payload into a screenshot one.
+    const payload: ScreenshotPayload = (screenshots[screenshotIndex].data: any);
+    const { url, windowWidth, windowHeight } = payload;
 
     // Compute the hover image's thumbnail size.
     let hoverHeight = HOVER_HEIGHT;
@@ -238,7 +234,7 @@ const options: ExplicitConnectOptions<OwnProps, StateProps, DispatchProps> = {
     return {
       thread: selectors.getRangeFilteredThread(state),
       screenshots: ensureExists(
-        selectors.getScreenshotMarkersById(state).get(screenshotId),
+        selectors.getRangeFilteredScreenshotsById(state).get(screenshotId),
         'Expected to find screenshots for the given pid'
       ),
       threadName: selectors.getFriendlyThreadName(state),

src/reducers/profile-view.js

@@ -31,7 +31,6 @@ import type {
   ThreadIndex,
   SamplesTable,
   MarkersTable,
-  MarkersTableWithPayload,
   Pid,
 } from '../types/profile';
 import type {
@@ -754,10 +753,7 @@ export type SelectorsForThread = {
   getCommittedRangeFilteredTracingMarkersForHeader: State => TracingMarker[],
   getNetworkTracingMarkers: State => TracingMarker[],
   getNetworkTiming: State => MarkerTimingRows,
-  getScreenshotMarkersById: State => Map<
-    string,
-    MarkersTableWithPayload<ScreenshotPayload>
-  >,
+  getRangeFilteredScreenshotsById: State => Map<string, TracingMarker[]>,
   getFilteredThread: State => Thread,
   getPreviewFilteredThread: State => Thread,
   getCallNodeInfo: State => CallNodeInfo,
@@ -782,19 +778,22 @@ export const selectorsForThread = (
   threadIndex: ThreadIndex
 ): SelectorsForThread => {
   if (!(threadIndex in selectorsForThreads)) {
+    const getThread = (state: State): Thread =>
+      getProfile(state).threads[threadIndex];
+    const _getMarkersTable = (state: State) => getThread(state).markers;
+    const _getStringTable = (state: State) => getThread(state).stringTable;
+
     /**
      * The first per-thread selectors filter out and transform a thread based on user's
      * interactions. The transforms are order dependendent.
      *
-     * 1. Unfiltered - The first selector gets the unmodified original thread.
+     * 1. Unfiltered getThread - The first selector gets the unmodified original thread.
      * 2. Range - New samples table with only samples in the committed range.
      * 3. Transform - Apply the transform stack that modifies the stacks and samples.
      * 4. Implementation - Modify stacks and samples to only show a single implementation.
      * 5. Search - Exclude samples that don't include some text in the stack.
      * 6. Preview - Only include samples that are within a user's preview range selection.
      */
-    const getThread = (state: State): Thread =>
-      getProfile(state).threads[threadIndex];
     const getRangeFilteredThread = createSelector(
       getThread,
       getCommittedRange,
@@ -1001,38 +1000,65 @@ export const selectorsForThread = (
       getNetworkTracingMarkers,
       MarkerTiming.getMarkerTiming
     );
-    const getScreenshotMarkersById = createSelector(
-      getRangeFilteredThread,
-      thread => {
-        const { markers, stringTable } = thread;
+    const getScreenshotsById = createSelector(
+      _getMarkersTable,
+      _getStringTable,
+      getProfileRootRange,
+      (markers, stringTable, rootRange) => {
         const idToScreenshotMarkers = new Map();
-        const nameIndex = stringTable.indexForString('CompositorScreenshot');
+        const name = 'CompositorScreenshot';
+        const nameIndex = stringTable.indexForString(name);
         for (let markerIndex = 0; markerIndex < markers.length; markerIndex++) {
           if (markers.name[markerIndex] === nameIndex) {
             // Coerce the payload to a screenshot one. Don't do a runtime check that
             // this is correct.
             const data: ScreenshotPayload = (markers.data[markerIndex]: any);
-            let screenshotMarkerTable = idToScreenshotMarkers.get(
-              data.windowID
-            );
-            if (screenshotMarkerTable === undefined) {
-              screenshotMarkerTable = {
-                time: [],
-                data: [],
-                name: [],
-                length: 0,
-              };
-              idToScreenshotMarkers.set(data.windowID, screenshotMarkerTable);
+
+            let tracingMarkers = idToScreenshotMarkers.get(data.windowID);
+            if (tracingMarkers === undefined) {
+              tracingMarkers = [];
+              idToScreenshotMarkers.set(data.windowID, tracingMarkers);
+            }
+
+            tracingMarkers.push({
+              start: markers.time[markerIndex],
+              dur: 0,
+              title: null,
+              name,
+              data,
+            });
+
+            if (tracingMarkers.length > 1) {
+              // Set the duration
+              const prevMarker = tracingMarkers[tracingMarkers.length - 2];
+              const nextMarker = tracingMarkers[tracingMarkers.length - 1];
+              prevMarker.dur = nextMarker.start - prevMarker.start;
             }
-            screenshotMarkerTable.time.push(markers.time[markerIndex]);
-            screenshotMarkerTable.data.push(data);
-            screenshotMarkerTable.name.push(nameIndex);
-            screenshotMarkerTable.length++;
           }
         }
+        for (const [, tracingMarkers] of idToScreenshotMarkers) {
+          // This last marker must exist.
+          const lastMarker = tracingMarkers[tracingMarkers.length - 1];
+          lastMarker.dur = rootRange.end - lastMarker.start;
+        }
         return idToScreenshotMarkers;
       }
     );
+    const getRangeFilteredScreenshotsById = createSelector(
+      getScreenshotsById,
+      getCommittedRange,
+      (screenshotsById, { start, end }) => {
+        const newMap = new Map();
+        for (const [id, screenshots] of screenshotsById) {
+          newMap.set(
+            id,
+            ProfileData.filterTracingMarkersToRange(screenshots, start, end)
+          );
+        }
+        return newMap;
+      }
+    );
+
     const getCallNodeInfo = createSelector(
       getFilteredThread,
       getDefaultCategory,
@@ -1149,7 +1175,7 @@ export const selectorsForThread = (
       getCommittedRangeFilteredTracingMarkersForHeader,
       getNetworkTracingMarkers,
       getNetworkTiming,
-      getScreenshotMarkersById,
+      getRangeFilteredScreenshotsById,
       getFilteredThread,
       getPreviewFilteredThread,
       getCallNodeInfo,

src/test/components/TrackScreenshots.test.js

@@ -3,10 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 // @flow
+import type {
+  Profile,
+  Thread,
+  IndexIntoMarkersTable,
+} from '../../types/profile';
+
 import * as React from 'react';
 import { Provider } from 'react-redux';
 import { mount } from 'enzyme';
 
+import { commitRange } from '../../actions/profile-view';
 import TrackScreenshots, {
   TRACK_HEIGHT,
 } from '../../components/timeline/TrackScreenshots';
@@ -85,10 +92,48 @@ describe('timeline/TrackScreenshots', function() {
     const pageX = LEFT + TRACK_WIDTH - 1;
     expect(pageX > moveMouseAndGetLeft(pageX)).toBe(true);
   });
+
+  it('renders a screenshot images when zooming into a range without a screenshot start time actually in the range', () => {
+    const profile = getScreenshotTrackProfile();
+    const [thread] = profile.threads;
+    const markerIndexA = thread.markers.length - 2;
+    const markerIndexB = thread.markers.length - 1;
+
+    _setScreenshotMarkersToUnknown(thread, markerIndexA, markerIndexB);
+
+    const { dispatch, view } = setup(profile);
+    dispatch(
+      commitRange(
+        thread.markers.time[markerIndexA],
+        thread.markers.time[markerIndexB]
+      )
+    );
+    view.update();
+    expect(view.find('.timelineTrackScreenshotImg').length).toBeGreaterThan(0);
+  });
+
+  it('renders a no images when zooming into a range before screenshots', () => {
+    const profile = getScreenshotTrackProfile();
+    const [thread] = profile.threads;
+
+    const markerIndexA = 0;
+    const markerIndexB = 1;
+
+    _setScreenshotMarkersToUnknown(thread, markerIndexA, markerIndexB);
+
+    const { dispatch, view } = setup(profile);
+    dispatch(
+      commitRange(
+        thread.markers.time[markerIndexA],
+        thread.markers.time[markerIndexB]
+      )
+    );
+    view.update();
+    expect(view.find('.timelineTrackScreenshotImg').length).toBe(0);
+  });
 });
 
-function setup() {
-  const profile = getScreenshotTrackProfile();
+function setup(profile: Profile = getScreenshotTrackProfile()) {
   const store = storeWithProfile(profile);
   const { getState, dispatch } = store;
   const flushRafCalls = mockRaf();
@@ -131,3 +176,22 @@ function setup() {
     moveMouseAndGetLeft,
   };
 }
+
+function _setScreenshotMarkersToUnknown(
+  thread: Thread,
+  ...markerIndexes: IndexIntoMarkersTable[]
+) {
+  // Remove off the last few screenshot markers
+  const unknownStringIndex = thread.stringTable.indexForString('Unknown');
+  const screenshotStringIndex = thread.stringTable.indexForString(
+    'CompositorScreenshot'
+  );
+  for (const markerIndex of markerIndexes) {
+    // Double check that we've actually got screenshot markers:
+    if (thread.markers.name[markerIndex] !== screenshotStringIndex) {
+      throw new Error('This is not a screenshot marker.');
+    }
+    thread.markers.name[markerIndex] = unknownStringIndex;
+    thread.markers.data[markerIndex] = null;
+  }
+}