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

[API Improvement] Wait for Tree Style Tab shutdown #2128

Closed
Lej77 opened this issue Jan 13, 2019 · 7 comments
Closed

[API Improvement] Wait for Tree Style Tab shutdown #2128

Lej77 opened this issue Jan 13, 2019 · 7 comments

Comments

@Lej77
Copy link
Contributor

Lej77 commented Jan 13, 2019

Short description

I have looked into ways that other extension could detect when Tree Style Tab is disabled. Currently the proposed way in the wiki is to use ping with setInterval.

I have found another way that can be used to detect when Tree Style Tab is disabeld. If the response to a message is a promise that is only resolved when the extension is disabled then that message can be used to do something after Tree Style Tab is disabled.

Code somewhere in Tree Style Tab:

const onClosePromise = new Promise((resolve, reject) => {
    // If this promise doesn't do anything then there seems to be a timeout so it only works if the tracked extension (this extension) is disabled within about 10 seconds
    // after this promise is used as a response to a message. After that it will not throw an error for the waiting extension.

    // If we use the following then the returned promise will be rejected when the extension is disabled even for longer times:
    window.addEventListener('beforeunload', () => resolve());
});

browser.runtime.onMessageExternal.addListener((message, sender) => {
    switch (message.type) {
        case 'wait-for-shutdown': {
            return onClosePromise;
        } break;
    }
});

Code in an extension that wants to be notified when Tree Style Tab is disabled:

async function waitForTSTShutdown() {
    try {
        // This message should always throw an error:
        await browser.runtime.sendMessage(kTST_ID, { type: 'wait-for-shutdown' });
        return false;
    } catch (error) {
        // Extension was disabled before message was sent:
        if (error.message.startsWith("Could not establish connection. Receiving end does not exist.")) {
            return false;
        }
        // Extension was disabled while we waited:
        if (error.message.startsWith("Message manager disconnected")) {
            return false;
        }
    }
    // Error probably thrown by the tracked extension's event listener:
    return true;
}
waitForTSTShutdown().then(uninitFeaturesForTST);

or simpler:

browser.runtime.sendMessage(kTST_ID, { type: 'wait-for-shutdown' }).finally(uninitFeaturesForTST);
@Lej77 Lej77 changed the title [API Improvement] Wait for Tree Style Tab exit [API Improvement] Wait for Tree Style Tab shutdown Jan 13, 2019
@piroor
Copy link
Owner

piroor commented Jan 16, 2019

I agree that TST should have something API to send message from TST to others on TST's shutdown. And your proposal looks really effective. Thank you for the idea!

Hmm, however it looks odd a little. The message type wait-for-shutdown looks that it should be resolved after shutdown, but rejected and notified to the receiver as an error actually.

Then, how about another way: runtime.connect() with extensionId? It looks effective like as your proposal. Moreover, the connection may be used for more applications also.
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/runtime/connect

@Lej77
Copy link
Contributor Author

Lej77 commented Jan 16, 2019

@piroor I have tested runtime.connect() before with some simple test extensions but the runtime.Port.onDisconnect event was never triggered when the extension was disabled. Perhaps it would work if a message was sent just as the extension is disabled with something like: window.addEventListener('beforeunload', () => port.postMessage(true)); but I haven't tested that.

Even if a Port could be used to detect when the extension is disabled it seems like it would be easier for developers to just use a message since that is what is used for the rest of the API. Also it seems like a Port should use more resources then even a long running message.

@Lej77
Copy link
Contributor Author

Lej77 commented Jan 16, 2019

@piroor I tested using browser.runtime.connect() to detect when another extension is disabled. It doesn't seem to work:

  • The runtime.Port.onDisconnect event is not called when the other extension is disabled.
    • Using something like window.addEventListener('beforeunload', () => port.disconnect()); doesn't help.
  • The port.onMessage event is not called if a message is sent with something like window.addEventListener('beforeunload', () => port.postMessage(true));

So a Port isn't really a better option then just using the ping method that is currently suggested in the wiki since it seems you need to interact with the Port in order to determine if it is still connected.

@piroor
Copy link
Owner

piroor commented Jan 16, 2019

Thank you for detailed research! Then, your initial proposal looks the only one available method. I've implemented the API by faa22f6.

@Lej77
Copy link
Contributor Author

Lej77 commented Jan 16, 2019

@piroor Happy to help!

@Lej77 Lej77 closed this as completed Jan 16, 2019
@piroor
Copy link
Owner

piroor commented Jan 16, 2019

Document: https://github.com/piroor/treestyletab/wiki/API-for-other-addons#wait-for-shutdown-type-message

@Lej77
Copy link
Contributor Author

Lej77 commented Jan 16, 2019

@piroor I fixed some things in the rich example code for wait-for-shutdown in the documention, but otherwise it looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants