Skip to content

Commit

Permalink
Partially bring suspendTabsUntilReady out of experimental status
Browse files Browse the repository at this point in the history
This commit will force-reload active tabs at launch for
environments not supporting suspend network request listeners,
or configured to not suspend network request listeners.
  • Loading branch information
gorhill committed Dec 18, 2021
1 parent edab87b commit a0a9497
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/js/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ const onHiddenSettingsReady = async function() {
});
}

// Matbe override default cache storage
// Maybe override default cache storage
const cacheBackend = await cacheStorage.select(
µb.hiddenSettings.cacheStorageAPI
);
Expand Down
18 changes: 16 additions & 2 deletions src/js/traffic.js
Original file line number Diff line number Diff line change
Expand Up @@ -1138,15 +1138,29 @@ const webRequest = {
vAPI.net = new vAPI.Net();
vAPI.net.suspend();

return ( ) => {
return async ( ) => {
vAPI.net.setSuspendableListener(onBeforeRequest);
vAPI.net.addListener(
'onHeadersReceived',
onHeadersReceived,
{ urls: [ 'http://*/*', 'https://*/*' ] },
[ 'blocking', 'responseHeaders' ]
);
vAPI.net.unsuspend(true);
vAPI.net.unsuspend({ force: true });

This comment has been minimized.

Copy link
@gwarser

gwarser Dec 24, 2021

Contributor

@gorhill where you get this force from? Should be all?

This comment has been minimized.

Copy link
@gorhill

gorhill Dec 24, 2021

Author Owner

Some changes I didn't unwind properly after I changed my mind in how I should approach this.

// Mitigation: force-reload active tabs for environments not
// supporting suspended network request listeners.
if (
vAPI.net.canSuspend() !== true ||
µb.hiddenSettings.suspendTabsUntilReady === 'no'
) {
const tabs = await vAPI.tabs.query({
active: true,
windowType: 'normal',
});
for ( const tab of tabs ) {
vAPI.tabs.reload(tab.id);
}
}
};
})(),

Expand Down

4 comments on commit a0a9497

@gwarser
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gorhill I think 80b758e broke suspending on Chrome. After this commit suspend() is never called with true to be forced

  • canSuspend() returns false by default and is not overridden in Chrome vapi-background-ext.js
  • suspendDepth
    suspend(force = false) {
    if ( this.canSuspend() || force ) {
    this.suspendDepth += 1;
    }
    is never increased
  • suspendOneRequest
    browser.webRequest.onBeforeRequest.addListener(
    details => {
    this.normalizeDetails(details);
    if ( this.suspendDepth !== 0 && details.tabId >= 0 ) {
    return this.suspendOneRequest(details);
    }
    is never called.

@gorhill
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, so this could explain your earlier observations.

Not sure setting suspendTabsUntilReady to yes is still worth it though, I think inactive tabs are no longer loaded by default in Chromium -- maybe this whole Chromium-specific code should go now and keep only the reloading of active tabs.

@gwarser
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find out that https://www.google.com/search?q=home&client=firefox-b-d&tbm=isch is a good page to test this. It clearly reloads on every browser restart. I think it may be annoying for some people. Allowing to turn reloading off can be a good idea.

@gorhill
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking a new checkbox in "Filter lists" pane for this might be the solution, something like:

  • Do not wait for filter lists to be fully loaded at launch

or whatever better formulation.

So un-chcked would mean current behavior, checked would mean unsuspended and not reloading active tab.

Please sign in to comment.