From c5b85aabd83b389bca52ed483c93eb1c5507738b Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Tue, 3 Dec 2019 02:57:58 -0400 Subject: [PATCH] Avoid race condtion that could prevent contextual account switching (#7623) There was a race condition in the logic responsible for switching the selected account based upon the active tab. It was asynchronously querying the active tab, then assuming it had been retrieved later. The active tab info itself was already in the redux store in another spot, one that is guaranteed to be set before the UI renders. The race condition was avoided by deleting the duplicate state, and using the other active tab state. --- .../connected-sites-list.container.js | 4 ++-- ui/app/ducks/app/app.js | 6 ------ ui/app/pages/routes/index.js | 17 +++++++++++---- ui/app/selectors/selectors.js | 4 ++-- ui/app/store/actions.js | 21 ------------------- 5 files changed, 17 insertions(+), 35 deletions(-) diff --git a/ui/app/components/app/connected-sites-list/connected-sites-list.container.js b/ui/app/components/app/connected-sites-list/connected-sites-list.container.js index 253a1f2701e4..de45831a9df7 100644 --- a/ui/app/components/app/connected-sites-list/connected-sites-list.container.js +++ b/ui/app/components/app/connected-sites-list/connected-sites-list.container.js @@ -17,8 +17,8 @@ import { getOriginFromUrl } from '../../../helpers/utils/util' const mapStateToProps = state => { const addressConnectedToCurrentTab = getAddressConnectedToCurrentTab(state) - const { openMetaMaskTabs, currentActiveTab = {} } = state.appState - const { title, url, id } = currentActiveTab + const { openMetaMaskTabs } = state.appState + const { title, url, id } = state.activeTab let tabToConnect diff --git a/ui/app/ducks/app/app.js b/ui/app/ducks/app/app.js index 72aa94ddcddd..8ac34a2c829f 100644 --- a/ui/app/ducks/app/app.js +++ b/ui/app/ducks/app/app.js @@ -78,7 +78,6 @@ export default function reduceApp (state, action) { requestAccountTabs: {}, openMetaMaskTabs: {}, currentWindowTab: {}, - currentActiveTab: {}, }, state.appState) switch (action.type) { @@ -782,11 +781,6 @@ export default function reduceApp (state, action) { currentWindowTab: action.value, }) - case actions.SET_ACTIVE_TAB: - return extend(appState, { - currentActiveTab: action.value, - }) - default: return appState } diff --git a/ui/app/pages/routes/index.js b/ui/app/pages/routes/index.js index 4c795baa6274..13d14fde57b0 100644 --- a/ui/app/pages/routes/index.js +++ b/ui/app/pages/routes/index.js @@ -86,7 +86,7 @@ import { class Routes extends Component { componentWillMount () { - const { currentCurrency, setCurrentCurrencyToUSD, getActiveTab } = this.props + const { currentCurrency, setCurrentCurrencyToUSD } = this.props if (!currentCurrency) { setCurrentCurrencyToUSD() @@ -105,8 +105,13 @@ class Routes extends Component { }) } }) + } - getActiveTab() + componentDidMount () { + const { addressConnectedToCurrentTab, showAccountDetail, selectedAddress } = this.props + if (addressConnectedToCurrentTab && addressConnectedToCurrentTab !== selectedAddress) { + showAccountDetail(addressConnectedToCurrentTab) + } } componentDidUpdate (prevProps) { @@ -357,6 +362,7 @@ Routes.propTypes = { textDirection: PropTypes.string, network: PropTypes.string, provider: PropTypes.object, + selectedAddress: PropTypes.string, frequentRpcListDetail: PropTypes.array, currentView: PropTypes.object, sidebar: PropTypes.object, @@ -373,11 +379,14 @@ Routes.propTypes = { providerId: PropTypes.string, hasPermissionsRequests: PropTypes.bool, autoLogoutTimeLimit: PropTypes.number, - getActiveTab: PropTypes.func, addressConnectedToCurrentTab: PropTypes.string, showAccountDetail: PropTypes.func, } +Routes.defaultProps = { + selectedAddress: undefined, +} + function mapStateToProps (state) { const { appState } = state const { @@ -403,6 +412,7 @@ function mapStateToProps (state) { submittedPendingTransactions: submittedPendingTransactionsSelector(state), network: state.metamask.network, provider: state.metamask.provider, + selectedAddress: state.metamask.selectedAddress, frequentRpcListDetail: state.metamask.frequentRpcListDetail || [], currentCurrency: state.metamask.currentCurrency, isMouseUser: state.appState.isMouseUser, @@ -420,7 +430,6 @@ function mapDispatchToProps (dispatch) { setCurrentCurrencyToUSD: () => dispatch(actions.setCurrentCurrency('usd')), setMouseUserState: (isMouseUser) => dispatch(actions.setMouseUserState(isMouseUser)), setLastActiveTime: () => dispatch(actions.setLastActiveTime()), - getActiveTab: () => dispatch(actions.getActiveTab()), showAccountDetail: address => dispatch(actions.showAccountDetail(address)), } } diff --git a/ui/app/selectors/selectors.js b/ui/app/selectors/selectors.js index c29dcb35f27d..fe8c108b44e7 100644 --- a/ui/app/selectors/selectors.js +++ b/ui/app/selectors/selectors.js @@ -560,8 +560,8 @@ function getRenderablePermissionsDomains (state) { } function getOriginOfCurrentTab (state) { - const { appState: { currentActiveTab = {} } } = state - return currentActiveTab.url && getOriginFromUrl(currentActiveTab.url) + const { activeTab } = state + return activeTab && activeTab.url && getOriginFromUrl(activeTab.url) } function getLastConnectedInfo (state) { diff --git a/ui/app/store/actions.js b/ui/app/store/actions.js index 3d04b09efe10..72ef1c9a46ed 100644 --- a/ui/app/store/actions.js +++ b/ui/app/store/actions.js @@ -402,9 +402,6 @@ var actions = { getCurrentWindowTab, SET_REQUEST_ACCOUNT_TABS: 'SET_REQUEST_ACCOUNT_TABS', SET_CURRENT_WINDOW_TAB: 'SET_CURRENT_WINDOW_TAB', - setActiveTab, - SET_ACTIVE_TAB: 'SET_ACTIVE_TAB', - getActiveTab, getOpenMetamaskTabsIds, SET_OPEN_METAMASK_TAB_IDS: 'SET_OPEN_METAMASK_TAB_IDS', } @@ -3074,21 +3071,3 @@ function getCurrentWindowTab () { dispatch(setCurrentWindowTab(currentWindowTab)) } } - -function setActiveTab (activeTab) { - return { - type: actions.SET_ACTIVE_TAB, - value: activeTab, - } -} - -function getActiveTab () { - return (dispatch) => { - return global.platform.queryTabs() - .then(tabs => { - const activeTab = tabs.find(tab => tab.active) - dispatch(setActiveTab(activeTab)) - return activeTab - }) - } -}