Skip to content

Commit

Permalink
fix: regression in HTTP recovery of background tabs (#915)
Browse files Browse the repository at this point in the history
Co-authored-by: Emanuele D'Agostini <emanuele.d'agostini@assembly.it>
Co-authored-by: Marcin Rataj <lidel@lidel.org>
  • Loading branch information
3 people authored Aug 10, 2020
1 parent 832c602 commit a219d2b
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 12 deletions.
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

0 comments on commit a219d2b

Please sign in to comment.