Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GIS-866: Search Communication #647

Merged
merged 17 commits into from Jan 7, 2021
Next

add search messager class and allow calling of refreshToken

  • Loading branch information
fcjr committed Dec 22, 2020
commit 653632e2458511bcf6fb746e4adac17135bb844e
@@ -40,6 +40,7 @@ import tabInfo from './classes/TabInfo';
import metrics from './classes/Metrics';
import account from './classes/Account';
import promoModals from './classes/PromoModals';
import searchMessager from './classes/SearchMessager';
// utilities
import { allowAllwaysC2P } from './utils/click2play';
import * as common from './utils/common';
@@ -1648,6 +1649,15 @@ function initializeGhosteryModules() {
});
}

/**
* Initialize Search Messaging.
* @memberOf Background
*/
function initializeSearchMessaging() {
const sm = new searchMessager();
sm.init()
}

/**
* Application Initializer
* Called whenever the browser starts or the extension is
@@ -1659,6 +1669,7 @@ function init() {
initializePopup();
initializeEventListeners();
initializeVersioning();
initializeSearchMessaging();
return metrics.init(globals.JUST_INSTALLED).then(() => initializeGhosteryModules().then(() => {
account.migrate()
.then(() => {
@@ -142,6 +142,8 @@ class Account {
})
)

refreshToken = () => api.refreshToken()

// @TODO a 404 here should trigger a logout
getUser = () => (
this._getUserID()
@@ -0,0 +1,51 @@
/**
* Search Messager
*
* Ghostery Browser Extension
* https://www.ghostery.com/
*
* Copyright 2019 Ghostery, Inc. All rights reserved.
*
* This Source Code Form is subject to the terms of the Mozilla Public
* 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
*/

import ExtMessenger from './ExtMessenger';
import account from './Account';
import { log } from '../utils/common';

/**
* Class for handling cross-extension messaging.
This conversation was marked as resolved by christophertino

This comment has been minimized.

@wlycdgr

wlycdgr Dec 23, 2020
Member

Can you throw a @since into the JSDoc comment?

This comment has been minimized.

@wlycdgr

wlycdgr Dec 23, 2020
Member

....What the heck is going on with the @ since formatting, ?lol?

This comment has been minimized.

@christophertino

christophertino Dec 24, 2020
Member

You tagged that github user. I updated the comment.

* @memberOf BackgroundClasses
*/
export default class SearchMessager {
constructor() {
this.extensionId = 'search@ghostery.com';
this._messageHandler = this._messageHandler.bind(this);
}

init() {
ExtMessenger.addListener(this._messageHandler);
}

unload() {
ExtMessenger.removeListener(this._messageHandler);
}

_messageHandler(message, sender, sendResponse) {
if (sender.id !== this.extensionId) {
return;
}

// allow search extension to refresh token
if (message === "refreshToken") {
account.refreshToken()
.then(() => sendResponse({ success: true }))
.catch(error => sendResponse({ success: false, error }));
return true
} else {
log('SearchMessager error: Unhandled message', message);
}
}
}
@@ -27,6 +27,10 @@ class Api {
}
}

refreshToken() {
This conversation was marked as resolved by jsignanini

This comment has been minimized.

@jsignanini

jsignanini Jan 4, 2021
Member

@fcjr I'm thinking renaming this to something like refreshTokenAndHandleIsRefreshing to make it super clear what the purpose of this new function is

This comment has been minimized.

@fcjr

fcjr Jan 4, 2021
Author Member

That sounds reasonable, I'll make the change. My thought with just naming it refreshToken was that as an external user of the api class the inner "isRefreshing" state is an implementation detail that external users of the function don't really need to know about, but since this should only be used in special cases anyways there shouldn't be a harm in making it explicit

This comment has been minimized.

@jsignanini

jsignanini Jan 4, 2021
Member

@fcjr actually you do make a good point as it’s only used internally in the other “private” function. Ok you can just leave it as is then. Thanks.

return this._refreshToken().then(this.isRefreshing = false);
}

_refreshToken() {
if (this.isRefreshing) {
let bindedResolve;
ProTip! Use n and p to navigate between commits in a pull request.