Bug 1541631 - Part 4: Use resource utility for SourceWithContent caching. r=jlast

Differential Revision: https://phabricator.services.mozilla.com/D42029

--HG--
extra : moz-landing-system : lando

devtools/client/debugger/src/components/SecondaryPanes/Frames/tests/Frames.spec.js

@@ -206,8 +206,8 @@ describe("Frames", () => {
       ];
 
       const sources: SourceResourceState = insertResources(createInitial(), [
-        source1,
+        { ...source1, content: null },
-        source2,
+        { ...source2, content: null },
       ]);
 
       const processedFrames = formatCallStackFrames(frames, sources, source1);

devtools/client/debugger/src/reducers/sources.js

@@ -24,7 +24,9 @@ import {
   updateResources,
   hasResource,
   getResource,
+  getMappedResource,
   getResourceIds,
+  memoizeResourceShallow,
   makeReduceQuery,
   makeReduceAllQuery,
   makeMapWithArgs,
@@ -93,6 +95,7 @@ export type SourceBase = {|
 
 type SourceResource = Resource<{
   ...SourceBase,
+  content: AsyncValue<SourceContent> | null,
 }>;
 export type SourceResourceState = ResourceState<SourceResource>;
 
@@ -102,8 +105,6 @@ export type SourcesState = {
   // All known sources.
   sources: SourceResourceState,
 
-  content: SourcesContentMap,
-
   breakpointPositions: BreakpointPositionsMap,
   breakableLines: { [SourceId]: Array<number> },
 
@@ -254,9 +255,16 @@ function update(
   return state;
 }
 
-function resourceAsSource(r: SourceResource): Source {
+const resourceAsSourceBase = memoizeResourceShallow(
-  return { ...r };
+  ({ content, ...source }: SourceResource): SourceBase => source
-}
+);
+
+const resourceAsSourceWithContent = memoizeResourceShallow(
+  ({ content, ...source }: SourceResource): SourceWithContent => ({
+    ...source,
+    content: content && content.state !== "pending" ? content : null,
+  })
+);
 
 /*
  * Add sources to the sources store
@@ -266,24 +274,26 @@ function resourceAsSource(r: SourceResource): Source {
 function addSources(state: SourcesState, sources: SourceBase[]): SourcesState {
   state = {
     ...state,
-    content: { ...state.content },
     urls: { ...state.urls },
     plainUrls: { ...state.plainUrls },
   };
 
-  state.sources = insertResources(state.sources, sources);
+  state.sources = insertResources(
+    state.sources,
+    sources.map(source => ({
+      ...source,
+      content: null,
+    }))
+  );
 
   for (const source of sources) {
-    // 1. Add the source to the sources map
+    // 1. Update the source url map
-    state.content[source.id] = null;
-
-    // 2. Update the source url map
     const existing = state.urls[source.url] || [];
     if (!existing.includes(source.id)) {
       state.urls[source.url] = [...existing, source.id];
     }
 
-    // 3. Update the plain url map
+    // 2. Update the plain url map
     if (source.url) {
       const plainUrl = getPlainUrl(source.url);
       const existingPlainUrls = state.plainUrls[plainUrl] || [];
@@ -403,7 +413,7 @@ function updateLoadedState(
 
   // If there was a navigation between the time the action was started and
   // completed, we don't want to update the store.
-  if (action.epoch !== state.epoch || !(sourceId in state.content)) {
+  if (action.epoch !== state.epoch || !hasResource(state.sources, sourceId)) {
     return state;
   }
 
@@ -427,10 +437,12 @@ function updateLoadedState(
 
   return {
     ...state,
-    content: {
+    sources: updateResources(state.sources, [
-      ...state.content,
+      {
-      [sourceId]: content,
+        id: sourceId,
-    },
+        content,
+      },
+    ]),
   };
 }
 
@@ -528,7 +540,9 @@ export function getSourceInSources(
   sources: SourceResourceState,
   id: string
 ): ?Source {
-  return hasResource(sources, id) ? getResource(sources, id) : null;
+  return hasResource(sources, id)
+    ? getMappedResource(sources, id, resourceAsSourceBase)
+    : null;
 }
 
 export function getSource(state: OuterState, id: SourceId): ?Source {
@@ -562,7 +576,9 @@ export function getSourcesByURLInSources(
   if (!url || !urls[url]) {
     return [];
   }
-  return urls[url].map(id => getResource(sources, id));
+  return urls[url].map(id =>
+    getMappedResource(sources, id, resourceAsSourceBase)
+  );
 }
 
 export function getSourcesByURL(state: OuterState, url: string): Source[] {
@@ -681,7 +697,7 @@ export function getHasSiblingOfSameName(state: OuterState, source: ?Source) {
 const querySourceList: ReduceAllQuery<
   SourceResource,
   Array<Source>
-> = makeReduceAllQuery(resourceAsSource, sources => sources.slice());
+> = makeReduceAllQuery(resourceAsSourceBase, sources => sources.slice());
 
 export function getSources(state: OuterState): SourceResourceState {
   return state.sources.sources;
@@ -748,17 +764,15 @@ type GSSWC = Selector<?SourceWithContent>;
 export const getSelectedSourceWithContent: GSSWC = createSelector(
   getSelectedLocation,
   getSources,
-  state => state.sources.content,
   (
     selectedLocation: ?SourceLocation,
-    sources: SourceResourceState,
+    sources: SourceResourceState
-    content: SourcesContentMap
   ): SourceWithContent | null => {
     const source =
       selectedLocation &&
       getSourceInSources(sources, selectedLocation.sourceId);
     return source
-      ? getSourceWithContentInner(sources, content, source.id)
+      ? getMappedResource(sources, source.id, resourceAsSourceWithContent)
       : null;
   }
 );
@@ -766,19 +780,17 @@ export function getSourceWithContent(
   state: OuterState,
   id: SourceId
 ): SourceWithContent {
-  return getSourceWithContentInner(
+  return getMappedResource(
     state.sources.sources,
-    state.sources.content,
+    id,
-    id
+    resourceAsSourceWithContent
   );
 }
 export function getSourceContent(
   state: OuterState,
   id: SourceId
 ): SettledValue<SourceContent> | null {
-  // Assert the resource exists.
+  const { content } = getResource(state.sources.sources, id);
-  getResource(state.sources.sources, id);
-  const content = state.sources.content[id];
 
   if (!content || content.state === "pending") {
     return null;
@@ -787,30 +799,6 @@ export function getSourceContent(
   return content;
 }
 
-const contentLookup: WeakMap<Source, SourceWithContent> = new WeakMap();
-function getSourceWithContentInner(
-  sources: SourceResourceState,
-  content: SourcesContentMap,
-  id: SourceId
-): SourceWithContent {
-  const source = getResource(sources, id);
-  let contentValue = content[source.id];
-
-  let result = contentLookup.get(source);
-  if (!result || result.content !== contentValue) {
-    if (contentValue && contentValue.state === "pending") {
-      contentValue = null;
-    }
-    result = {
-      ...source,
-      content: contentValue,
-    };
-    contentLookup.set(source, result);
-  }
-
-  return result;
-}
-
 export function getSelectedSourceId(state: OuterState) {
   const source = getSelectedSource((state: any));
   return source && source.id;
@@ -1001,7 +989,7 @@ export const getSelectedBreakableLines: Selector<Set<number>> = createSelector(
 );
 
 export function isSourceLoadingOrLoaded(state: OuterState, sourceId: string) {
-  const content = state.sources.content[sourceId];
+  const { content } = getResource(state.sources.sources, sourceId);
   return content !== null;
 }