Bug 1534085 - Avoid accidentally creating partial source objects. r=bhackett
authorLogan Smyth <loganfsmyth@gmail.com>
Wed, 13 Mar 2019 16:59:56 +0000
changeset 521761 48003b78de5a22362a6f869ddeb09741d2913576
parent 521760 f04779216e2b928707a1c848ccd135a7b9def3c2
child 521762 e7cd2f1e7f66665664eea0dde614a6ff6da35311
push id10867
push userdvarga@mozilla.com
push dateThu, 14 Mar 2019 15:20:45 +0000
treeherdermozilla-beta@abad13547875 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs1534085
milestone67.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 1534085 - Avoid accidentally creating partial source objects. r=bhackett Differential Revision: https://phabricator.services.mozilla.com/D23242
devtools/client/debugger/new/src/actions/sources/tests/loadSource.spec.js
devtools/client/debugger/new/src/reducers/sources.js
devtools/client/debugger/new/src/utils/test-head.js
--- a/devtools/client/debugger/new/src/actions/sources/tests/loadSource.spec.js
+++ b/devtools/client/debugger/new/src/actions/sources/tests/loadSource.spec.js
@@ -2,16 +2,17 @@
  * 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 {
   actions,
   selectors,
+  watchForState,
   createStore,
   makeSource
 } from "../../../utils/test-head";
 import { sourceThreadClient } from "../../tests/helpers/threadClient.js";
 
 describe("loadSourceText", () => {
   it("should load source text", async () => {
     const store = createStore(sourceThreadClient);
@@ -112,23 +113,30 @@ describe("loadSourceText", () => {
 
     await dispatch(actions.loadSourceText(prevSource));
     const curSource = selectors.getSource(getState(), "foo1");
 
     expect(prevSource === curSource).toBeTruthy();
   });
 
   it("should indicate a loading source", async () => {
-    const { dispatch, getState } = createStore(sourceThreadClient);
+    const store = createStore(sourceThreadClient);
+    const { dispatch } = store;
+
+    const source = makeSource("foo2");
+    await dispatch(actions.newSource(source));
 
-    // Don't block on this so we can check the loading state.
-    const source = makeSource("foo1");
-    dispatch(actions.loadSourceText(source));
-    const fooSource = selectors.getSource(getState(), "foo1");
-    expect(fooSource && fooSource.loadedState).toEqual("loading");
+    const wasLoading = watchForState(store, state => {
+      const fooSource = selectors.getSource(state, "foo2");
+      return fooSource && fooSource.loadedState === "loading";
+    });
+
+    await dispatch(actions.loadSourceText(source));
+
+    expect(wasLoading()).toBe(true);
   });
 
   it("should indicate an errored source text", async () => {
     const { dispatch, getState } = createStore(sourceThreadClient);
 
     const source = makeSource("bad-id");
     await dispatch(actions.newSource(source));
     await dispatch(actions.loadSourceText(source));
--- a/devtools/client/debugger/new/src/reducers/sources.js
+++ b/devtools/client/debugger/new/src/reducers/sources.js
@@ -174,16 +174,25 @@ function update(
 }
 
 /*
  * Update a source when its state changes
  * e.g. the text was loaded, it was blackboxed
  */
 function updateSource(state: SourcesState, source: Object) {
   const existingSource = state.sources[source.id];
+
+  // If there is no existing version of the source, it means that we probably
+  // ended up here as a result of an async action, and the sources were cleared
+  // between the action starting and the source being updated.
+  if (!existingSource) {
+    // TODO: We may want to consider throwing here once we have a better
+    // handle on async action flow control.
+    return state;
+  }
   return {
     ...state,
     sources: {
       ...state.sources,
       [source.id]: { ...existingSource, ...source }
     }
   };
 }
--- a/devtools/client/debugger/new/src/utils/test-head.js
+++ b/devtools/client/debugger/new/src/utils/test-head.js
@@ -98,17 +98,18 @@ function makeSource(name: string, props:
     ]
   };
   return (rv: any);
 }
 
 function makeOriginalSource(name: string, props?: Object): Source {
   const rv = {
     ...makeSourceRaw(name, props),
-    id: `${name}/originalSource`
+    id: `${name}/originalSource`,
+    actors: []
   };
   return (rv: any);
 }
 
 function makeFuncLocation(startLine, endLine) {
   if (!endLine) {
     endLine = startLine + 1;
   }
@@ -152,26 +153,54 @@ function waitForState(store: any, predic
       if (ret) {
         unsubscribe();
         resolve(ret);
       }
     });
   });
 }
 
+function watchForState(store: any, predicate: any): () => boolean {
+  let sawState = false;
+  const checkState = function() {
+    if (!sawState && predicate(store.getState())) {
+      sawState = true;
+    }
+    return sawState;
+  };
+
+  let unsubscribe;
+  if (!checkState()) {
+    unsubscribe = store.subscribe(() => {
+      if (checkState()) {
+        unsubscribe();
+      }
+    });
+  }
+
+  return function read() {
+    if (unsubscribe) {
+      unsubscribe();
+    }
+
+    return sawState;
+  };
+}
+
 function getTelemetryEvents(eventName: string) {
   return window.dbg._telemetry.events[eventName] || [];
 }
 
 export {
   actions,
   selectors,
   reducers,
   createStore,
   commonLog,
   getTelemetryEvents,
   makeFrame,
   makeSource,
   makeOriginalSource,
   makeSymbolDeclaration,
   waitForState,
+  watchForState,
   getHistory
 };