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

fix: regression in HTTP recovery of background tabs #915

Merged
merged 3 commits into from
Aug 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions add-on/src/lib/ipfs-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -414,9 +414,9 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru

// Chromium bug: sometimes tabs.update does not work from onCompleted,
// we run additional update after 1s just to be sure
setTimeout(() => browser.tabs.update({ url: fixedUrl }), 1000)
setTimeout(() => browser.tabs.update(request.tabId, { url: fixedUrl }), 1000)

return browser.tabs.update({ url: fixedUrl })
return browser.tabs.update(request.tabId, { url: fixedUrl })
}

if (isRecoverable(request, state, ipfsPathValidator)) {
Expand Down Expand Up @@ -616,7 +616,7 @@ async function updateTabWithURL (request, redirectUrl, browser) {
// Do nothing if the URL remains the same
if (request.url === redirectUrl) return

return browser.tabs.update({
return browser.tabs.update(request.tabId, {
active: true,
url: redirectUrl
})
Expand Down
16 changes: 8 additions & 8 deletions test/functional/lib/ipfs-request-gateway-recover.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ const createDnslinkResolver = require('../../../add-on/src/lib/dnslink')
const { createIpfsPathValidator } = require('../../../add-on/src/lib/ipfs-path')
const { optionDefaults } = require('../../../add-on/src/lib/options')

const url2request = (url, type = 'main_frame') => {
return { url, type }
const url2request = (url, type = 'main_frame', tabId = new Date().valueOf()) => {
return { url, type, tabId }
}
const urlRequestWithStatus = (url, statusCode = 200, type = 'main_frame') => {
return { ...url2request(url, type), statusCode }
Expand Down Expand Up @@ -54,7 +54,7 @@ describe('requestHandler.onCompleted:', function () { // HTTP-level errors
it('should redirect to default subdomain gateway on broken subdomain gateway request', async function () {
const request = urlRequestWithStatus('http://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.brokenexample.com/wiki/', 500)
await requestHandler.onCompleted(request)
assert.ok(browser.tabs.update.withArgs({ url: 'https://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/', active: true }).calledOnce, 'tabs.update should be called with default subdomain gateway URL')
assert.ok(browser.tabs.update.withArgs(request.tabId, { url: 'https://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/', active: true }).calledOnce, 'tabs.update should be called with default subdomain gateway URL')
})
it('should do nothing if broken request is a non-IPFS request', async function () {
const request = urlRequestWithStatus('https://wikipedia.org', 500)
Expand Down Expand Up @@ -89,7 +89,7 @@ describe('requestHandler.onCompleted:', function () { // HTTP-level errors
it('should recover from unreachable third party public gateway by reopening on the public gateway', async function () {
const request = urlRequestWithStatus('https://nondefaultipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', 500)
await requestHandler.onCompleted(request)
assert.ok(browser.tabs.update.withArgs({ url: 'https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', active: true }).calledOnce, 'tabs.update should be called with IPFS default public gateway URL')
assert.ok(browser.tabs.update.withArgs(request.tabId, { url: 'https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', active: true }).calledOnce, 'tabs.update should be called with IPFS default public gateway URL')
})
})

Expand Down Expand Up @@ -153,7 +153,7 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors
it('should redirect to default subdomain gateway on failed subdomain gateway request', async function () {
const request = urlRequestWithStatus('http://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.brokenexample.com/wiki/', 500)
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.update.withArgs({ url: 'https://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/', active: true }).calledOnce, 'tabs.update should be called with default subdomain gateway URL')
assert.ok(browser.tabs.update.withArgs(request.tabId, { url: 'https://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/', active: true }).calledOnce, 'tabs.update should be called with default subdomain gateway URL')
})
it('should do nothing if failed request is a non-IPFS request', async function () {
const request = urlRequestWithNetworkError('https://wikipedia.org')
Expand All @@ -179,7 +179,7 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors
it('should recover from unreachable third party public gateway by reopening on the public gateway', async function () {
const request = urlRequestWithNetworkError('https://nondefaultipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h')
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.update.withArgs({ url: 'https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', active: true }).calledOnce, 'tabs.update should be called with IPFS default public gateway URL')
assert.ok(browser.tabs.update.withArgs(request.tabId, { url: 'https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', active: true }).calledOnce, 'tabs.update should be called with IPFS default public gateway URL')
})
it('should recover from unreachable HTTP server by reopening DNSLink on the active gateway', async function () {
state.dnslinkPolicy = 'best-effort'
Expand All @@ -188,7 +188,7 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors
const expectedUrl = 'http://localhost:8080/ipns/en.wikipedia-on-ipfs.org/'
const request = urlRequestWithNetworkError('https://en.wikipedia-on-ipfs.org/')
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.update.withArgs({ url: expectedUrl, active: true }).calledOnce, 'tabs.update should be called with DNSLink on local gateway URL')
assert.ok(browser.tabs.update.withArgs(request.tabId, { url: expectedUrl, active: true }).calledOnce, 'tabs.update should be called with DNSLink on local gateway URL')
dnslinkResolver.clearCache()
})
it('should recover from failed DNS for .eth opening it on EthDNS gateway at .eth.link', async function () {
Expand All @@ -199,7 +199,7 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors
const expectedUrl = 'https://almonit.eth.link/'
const request = urlRequestWithNetworkError('https://almonit.eth', dnsFailure)
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.update.withArgs({ url: expectedUrl, active: true }).calledOnce, 'tabs.update should be called with ENS resource on local gateway URL')
assert.ok(browser.tabs.update.withArgs(request.tabId, { url: expectedUrl, active: true }).calledOnce, 'tabs.update should be called with ENS resource on local gateway URL')
dnslinkResolver.clearCache()
})
})
Expand Down
2 changes: 1 addition & 1 deletion test/functional/lib/ipfs-request-workarounds.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ describe('modifyRequest processing', function () {
browser.tabs.update.flush()
assert.ok(browser.tabs.update.notCalled)
modifyRequest.onCompleted(request)
assert.ok(browser.tabs.update.withArgs({ url: fixedDNSLinkUrl }).calledOnce)
assert.ok(browser.tabs.update.withArgs(request.tabId, { url: fixedDNSLinkUrl }).calledOnce)
browser.tabs.update.flush()
})
})
Expand Down