Bug 1534085 - Avoid accidentally creating partial source objects. r=bhackett
☠☠ backed out by ca44b5f0bb41 ☠ ☠
authorLogan Smyth <loganfsmyth@gmail.com>
Wed, 13 Mar 2019 00:27:00 +0000
changeset 521624 fae9b5118064af3356e0a534b1852e9b37bc73b8
parent 521623 c1b92a031a981b3c65ef7c73d287f653dcbc1f15
child 521625 bb3918322c3c0f903e9a11065e56b921fe11bff0
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 => selectors.getSource(state, "foo2").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
 };