Skip to content

Commit

Permalink
switch to isolating change within LocalStorage processor
Browse files Browse the repository at this point in the history
  • Loading branch information
samouri committed Feb 24, 2022
1 parent 603b270 commit a58f111
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 37 deletions.
8 changes: 4 additions & 4 deletions src/main-thread/commands/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ export const StorageProcessor: CommandExecutorInterface = (strings, nodeContext,

// TODO(choumx): Clean up key/value strings (or don't store them in the first place)
// to avoid leaking memory.
const key = keyIndex > 0 ? strings.get(keyIndex) : '';
const value = valueIndex > 0 ? strings.get(valueIndex) : null;
const key = keyIndex > 0 ? strings.get(keyIndex - 1) : '';
const value = valueIndex > 0 ? strings.get(valueIndex - 1) : null;

if (operation === GetOrSet.GET) {
get(location, key);
Expand All @@ -82,8 +82,8 @@ export const StorageProcessor: CommandExecutorInterface = (strings, nodeContext,
const keyIndex = mutations[startPosition + StorageMutationIndex.Key];
const valueIndex = mutations[startPosition + StorageMutationIndex.Value];

const key = keyIndex > 0 ? strings.get(keyIndex) : null;
const value = valueIndex > 0 ? strings.get(valueIndex) : null;
const key = keyIndex > 0 ? strings.get(keyIndex - 1) : null;
const value = valueIndex > 0 ? strings.get(valueIndex - 1) : null;

return {
type: 'STORAGE',
Expand Down
8 changes: 3 additions & 5 deletions src/main-thread/strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,7 @@ export class StringContext {
private strings: Array<string>;

constructor() {
this.strings = [
// The 0th index is reserved for meaning no value. Or delete.
// For example, the storage.removeItem code path.
null as any,
];
this.strings = [];
}

/**
Expand All @@ -27,6 +23,8 @@ export class StringContext {

/**
* Stores a string in mapping and returns the index of the location.
* @param value string to store
* @return {number}
*/
store(value: string): number {
this.strings.push(value);
Expand Down
5 changes: 1 addition & 4 deletions src/test/Emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@ export interface Emitter {
* mutation transfer state is also stored at the module-level.
* TODO: Remove this and replace with strings.getForTesting() at call sites.
*/
const strings: Array<string> = [
// First index reserved to signify deletion.
null as any,
];
const strings: Array<string> = [];

/**
* Stubs `document.postMessage` to invoke callbacks passed to `once()`.
Expand Down
6 changes: 3 additions & 3 deletions src/test/main-thread/commands/function.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ test('Returns the value of a resolved function', async (t) => {
const mutations: number[] = [];
mutations[FunctionMutationIndex.Status] = ResolveOrReject.RESOLVE;
mutations[FunctionMutationIndex.Index] = index;
mutations[FunctionMutationIndex.Value] = 1;
mutations[FunctionMutationIndex.Value] = 0;

processor.execute(new Uint16Array(mutations), 0, true);
t.deepEqual(await promise, { val: true });
Expand All @@ -41,7 +41,7 @@ test('Is able to return undefined', async (t) => {
const mutations: number[] = [];
mutations[FunctionMutationIndex.Status] = ResolveOrReject.RESOLVE;
mutations[FunctionMutationIndex.Index] = index;
mutations[FunctionMutationIndex.Value] = 1;
mutations[FunctionMutationIndex.Value] = 0;

processor.execute(new Uint16Array(mutations), 0, true);
t.deepEqual(await promise, undefined);
Expand All @@ -53,7 +53,7 @@ test('Returns the value of a rejected value', (t) => {
const mutations: number[] = [];
mutations[FunctionMutationIndex.Status] = ResolveOrReject.REJECT;
mutations[FunctionMutationIndex.Index] = index;
mutations[FunctionMutationIndex.Value] = 1;
mutations[FunctionMutationIndex.Value] = 0;

processor.execute(new Uint16Array(mutations), 0, true);
return promise.then(
Expand Down
36 changes: 18 additions & 18 deletions src/test/main-thread/mutator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@ test.serial('batch mutations', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
1,
0,
1 + 1,
0,
0 + 1,
]),
);
mutator.mutate(
Expand All @@ -82,9 +82,9 @@ test.serial('batch mutations', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
2,
1,
0,
2 + 1,
1 + 1,
]),
);

Expand All @@ -102,9 +102,9 @@ test.serial('batch mutations', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
3,
2,
0,
3 + 1,
2 + 1,
]),
);

Expand Down Expand Up @@ -140,9 +140,9 @@ test.serial('batch mutations with custom pump', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
1,
0,
1 + 1,
0,
0 + 1,
]),
);
mutator.mutate(
Expand All @@ -152,9 +152,9 @@ test.serial('batch mutations with custom pump', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
2,
1,
0,
2 + 1,
1 + 1,
]),
);

Expand All @@ -174,9 +174,9 @@ test.serial('batch mutations with custom pump', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
3,
2,
0,
3 + 1,
2 + 1,
]),
);

Expand Down Expand Up @@ -209,9 +209,9 @@ test.serial('leverage allowlist to exclude mutation type', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
1,
0,
1 + 1,
0,
0 + 1,
]),
);
mutator.mutate(
Expand All @@ -221,9 +221,9 @@ test.serial('leverage allowlist to exclude mutation type', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
2,
1,
0,
2 + 1,
1 + 1,
]),
);

Expand Down Expand Up @@ -257,9 +257,9 @@ test.serial('split strings from mutations', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
1,
0,
1 + 1,
0,
0 + 1,
]),
);

Expand Down
2 changes: 1 addition & 1 deletion src/test/mutation-transfer/storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ test.serial.cb('Storage.removeItem', (t) => {
const { document, storage } = t.context;

expectMutations(document, (mutations) => {
t.deepEqual(mutations, [TransferrableMutationType.STORAGE, GetOrSet.SET, StorageLocation.Local, getForTesting('foo'), 0]);
t.deepEqual(mutations, [TransferrableMutationType.STORAGE, GetOrSet.SET, StorageLocation.Local, getForTesting('foo')! + 1, 0]);
t.end();
});

Expand Down
2 changes: 1 addition & 1 deletion src/worker-thread/Storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export function createStorage(document: Document | DocumentStub, location: Stora
TransferrableMutationType.STORAGE,
GetOrSet.SET,
location,
store(key),
store(key) + 1,
0, // value == 0 represents deletion.
]);
},
Expand Down
2 changes: 1 addition & 1 deletion src/worker-thread/strings.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
let count: number = 1; // count starts from 1 since 0th index is reserved to mean "no value" or "delete".
let count: number = 0;
let transfer: Array<string> = [];
const mapping: Map<string, number> = new Map();

Expand Down

0 comments on commit a58f111

Please sign in to comment.