Skip to content
This repository has been archived by the owner on Mar 16, 2023. It is now read-only.

Commit

Permalink
fix(sdk): fix race condition when opening multiple ui popups
Browse files Browse the repository at this point in the history
  • Loading branch information
LeilaWang committed Feb 25, 2020
1 parent b5d0c62 commit e9aa14d
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 112 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,16 @@ import {
} from '~/config/event';
import filterStream from '~/utils/filterStream';
import userPermissionQuery from '~/background/services/GraphQLService/Queries/userPermissionQuery';
import AuthService from '~/background/services/AuthService';
import query from '../utils/query';

const registerExtensionUi = async (request, connection) => {
const {
requestId,
domain,
data: {
args,
...rest
},
} = request;
const registeredDomain = await AuthService.getRegisteredDomain(domain);

connection.UiActionSubject.next({
...request,
type: 'ui.register.extension',
data: {
...rest,
args: {
...args,
domainRegistered: !!registeredDomain,
},
},
});

return filterStream(
Expand Down
105 changes: 87 additions & 18 deletions packages/extension/src/background/utils/connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
import Iframe from '~/utils/Iframe';
import {
warnLog,
errorLog,
} from '~/utils/log';
import {
permissionError,
Expand Down Expand Up @@ -76,6 +77,8 @@ class Connection {
containerId: this.containerId,
});

this.uiRequestQueue = [];

// send the messages to the client
merge(this.clientAction$, this.clientResponse$).pipe(
tap(({ webClientId, ...rest }) => {
Expand Down Expand Up @@ -112,6 +115,8 @@ class Connection {
},
} = action;

await this.ensureUiRequestOrder(requestId);

const loadingElem = document.getElementById('aztec-popup-placeholder');
loadingElem.style.display = 'block';

Expand All @@ -128,6 +133,8 @@ class Connection {
webClientId,
});

// wait until the iframe is ready before opening the ui
// because creating iframe will block other processes and make the loading animation look aweful
const frame = await this.uiFrame.ensureCreated();

this.openUi({
Expand Down Expand Up @@ -185,12 +192,7 @@ class Connection {
webClientId,
} = this.requests[requestId]);
}
this.closeUi();
this.ClientResponseSubject.next({
type: uiCloseEvent,
requestId,
webClientId,
});
this.closeUi(data);

const {
abort,
Expand Down Expand Up @@ -267,11 +269,9 @@ class Connection {
type: clientResponseEvent,
});

this.closeUi();

this.ClientResponseSubject.next({
...data,
type: uiCloseEvent,
const ids = this.uiRequestQueue.map(({ requestId }) => requestId);
ids.forEach((requestId) => {
this.closeUi({ requestId }, false);
});

const {
Expand Down Expand Up @@ -438,9 +438,7 @@ class Connection {
requestId,
webClientId,
}) {
this.closeUi();
this.ClientResponseSubject.next({
type: uiCloseEvent,
this.closeUi({
requestId,
webClientId,
});
Expand All @@ -455,7 +453,31 @@ class Connection {
this.removeRequest(requestId);
}

closeUi = () => {
closeUi = ({
requestId,
webClientId,
}, closeBackground = true) => {
if (!this.uiRequestQueue.length) {
return false;
}
const request = this.uiRequestQueue.shift();
if (request.requestId !== requestId) {
errorLog('Request not run in order.');
this.uiRequestQueue.unshift(request);
return false;
}
if (this.uiRequestQueue.length) {
const {
requestId: nextRequestId,
resolve,
} = this.uiRequestQueue.shift();
this.uiRequestQueue.unshift({
requestId: nextRequestId,
});
resolve();
return false;
}

const event = new CustomEvent('closeAztec');
window.dispatchEvent(event);

Expand All @@ -465,13 +487,60 @@ class Connection {
type: 'closed',
},
}, '*');

if (closeBackground) {
// wait for the fade-out animation to finish before closing the backgroundFrame
setTimeout(() => {
if (!this.uiRequestQueue.length) {
this.ClientResponseSubject.next({
type: uiCloseEvent,
requestId,
webClientId,
});
}
}, 500);
}

return true;
}

openUi = (detail) => {
const event = new CustomEvent('openAztec', {
detail,
if (!this.uiRequestQueue.length) {
return false;
}
if (this.uiRequestQueue[0].requestId !== detail.requestId) {
errorLog('Request not run in order.');
return false;
}
if (this.uiRequestQueue[0].resolve) {
errorLog('UI open request not resolved.');
return false;
}
if (this.uiRequestQueue.length === 1) {
const event = new CustomEvent('openAztec', {
detail,
});
window.dispatchEvent(event);
return true;
}

return false;
}

async ensureUiRequestOrder(requestId) {
if (!this.uiRequestQueue.length) {
this.uiRequestQueue.push({
requestId,
});
return null;
}

return new Promise((resolve) => {
this.uiRequestQueue.push({
requestId,
resolve,
});
});
window.dispatchEvent(event);
}

removeRequest(requestId) {
Expand Down
24 changes: 12 additions & 12 deletions packages/extension/src/client/services/ConnectionService/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,36 +71,36 @@ class ConnectionService {
if (!this.port) return;

let requestId = this.disconnectRequestId;
let callbackKey = `${requestId}-disconnect`;
if (requestId) {
await new Promise((resolve) => {
if (!this.callbackMapping[requestId]) {
this.callbackMapping[requestId] = {
[uiCloseEvent]: [],
};
if (!this.callbackMapping[callbackKey]) {
this.callbackMapping[callbackKey] = [];
}
this.callbackMapping[requestId][uiCloseEvent].push(resolve);
this.callbackMapping[callbackKey].push(resolve);
});
return;
}

requestId = randomId();
this.disconnectRequestId = requestId;
callbackKey = `${requestId}-disconnect`;

const uiDisconnected = new Promise((resolve) => {
this.callbackMapping[requestId] = {
[uiCloseEvent]: [resolve],
};
});
backgroundFrame.close();

await this.postToBackground({
type: clientDisconnectEvent,
requestId,
});

await uiDisconnected;

this.disconnectRequestId = null;
this.setInitialVars();

if (this.callbackMapping[callbackKey]) {
this.callbackMapping[callbackKey].forEach((cb) => {
cb();
});
}
}

async openConnection(clientProfile) {
Expand Down
5 changes: 4 additions & 1 deletion packages/extension/src/ui/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,10 @@ class App extends PureComponent {
}, minDelayTime);
} else {
this.setState(
nextState,
{
...nextState,
loading: true,
},
ensureMinPendingTime(() => this.goToPage(route), minDelayTime),
);
}
Expand Down
16 changes: 0 additions & 16 deletions packages/extension/src/ui/pages/Register.jsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import React from 'react';
import PropTypes from 'prop-types';
import ConnectionService from '~uiModules/services/ConnectionService';
import returnAndClose from '~/ui/helpers/returnAndClose';
import getGSNConfig from '~/ui/helpers/getGSNConfig';
import StepsHandler from '~/ui/views/handlers/StepsHandler';
import RegisterContent from '~/ui/views/RegisterContent';
import registerSteps from '~/ui/steps/register';

const Register = ({
currentAccount,
domainRegistered,
goToPage,
}) => {
const fetchInitialData = async () => {
const gsnConfig = await getGSNConfig();
Expand All @@ -30,21 +26,11 @@ const Register = ({
};
};

const handleClose = async (data) => {
if (domainRegistered) {
returnAndClose(data);
} else {
goToPage('loading');
ConnectionService.returnToClient(data);
}
};

return (
<StepsHandler
testId="steps-register"
fetchInitialData={fetchInitialData}
Content={RegisterContent}
onExit={handleClose}
/>
);
};
Expand All @@ -53,8 +39,6 @@ Register.propTypes = {
currentAccount: PropTypes.shape({
address: PropTypes.string.isRequired,
}).isRequired,
domainRegistered: PropTypes.bool.isRequired,
goToPage: PropTypes.func.isRequired,
};

export default Register;
16 changes: 0 additions & 16 deletions packages/extension/src/ui/pages/RegisterAddress.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import React from 'react';
import PropTypes from 'prop-types';
import ConnectionService from '~/ui/services/ConnectionService';
import returnAndClose from '~/ui/helpers/returnAndClose';
import getGSNConfig from '~/ui/helpers/getGSNConfig';
import StepsHandler from '~/ui/views/handlers/StepsHandler';
import RegisterContent from '~/ui/views/RegisterContent';
Expand All @@ -10,8 +8,6 @@ import apis from '~uiModules/apis';

const RegisterAddress = ({
currentAccount,
domainRegistered,
goToPage,
}) => {
const fetchInitialData = async () => {
const gsnConfig = await getGSNConfig();
Expand All @@ -37,21 +33,11 @@ const RegisterAddress = ({
};
};

const handleClose = async (data) => {
if (domainRegistered) {
returnAndClose(data);
} else {
goToPage('loading');
ConnectionService.returnToClient(data);
}
};

return (
<StepsHandler
testId="steps-register-address"
fetchInitialData={fetchInitialData}
Content={RegisterContent}
onExit={handleClose}
/>
);
};
Expand All @@ -61,8 +47,6 @@ RegisterAddress.propTypes = {
address: PropTypes.string.isRequired,
linkedPublicKey: PropTypes.string.isRequired,
}).isRequired,
domainRegistered: PropTypes.bool.isRequired,
goToPage: PropTypes.func.isRequired,
};

export default RegisterAddress;
Loading

0 comments on commit e9aa14d

Please sign in to comment.