diff --git a/shell/browser/net/node_stream_loader.cc b/shell/browser/net/node_stream_loader.cc index 11621eb7fe1d6..d4fcaafdf943c 100644 --- a/shell/browser/net/node_stream_loader.cc +++ b/shell/browser/net/node_stream_loader.cc @@ -68,6 +68,8 @@ void NodeStreamLoader::Start(network::ResourceResponseHead head) { void NodeStreamLoader::NotifyReadable() { if (!readable_) ReadMore(); + else if (is_reading_) + has_read_waiting_ = true; readable_ = true; } @@ -100,8 +102,16 @@ void NodeStreamLoader::ReadMore() { // If there is no buffer read, wait until |readable| is emitted again. v8::Local buffer; if (!ret.ToLocal(&buffer) || !node::Buffer::HasInstance(buffer)) { - readable_ = false; is_reading_ = false; + + // If 'readable' was called after 'read()', try again + if (has_read_waiting_) { + has_read_waiting_ = false; + ReadMore(); + return; + } + + readable_ = false; if (ended_) { NotifyComplete(result_); } diff --git a/shell/browser/net/node_stream_loader.h b/shell/browser/net/node_stream_loader.h index 55a529eacd123..d27499212c0fe 100644 --- a/shell/browser/net/node_stream_loader.h +++ b/shell/browser/net/node_stream_loader.h @@ -85,6 +85,11 @@ class NodeStreamLoader : public network::mojom::URLLoader { // flag. bool readable_ = false; + // It's possible for reads to be queued using nextTick() during read() + // which will cause 'readable' to emit during ReadMore, so we track if + // that occurred in a flag. + bool has_read_waiting_ = false; + // Store the V8 callbacks to unsubscribe them later. std::map> handlers_; diff --git a/spec-main/api-protocol-spec.ts b/spec-main/api-protocol-spec.ts index 754410940f393..1e93f14a5fd3f 100644 --- a/spec-main/api-protocol-spec.ts +++ b/spec-main/api-protocol-spec.ts @@ -26,6 +26,38 @@ const uninterceptProtocol = promisify(protocol.uninterceptProtocol) const text = 'valar morghulis' const protocolName = 'sp' +======= +import { expect } from 'chai'; +import { protocol, webContents, WebContents, session, BrowserWindow, ipcMain } from 'electron/main'; +import { AddressInfo } from 'net'; +import * as ChildProcess from 'child_process'; +import * as path from 'path'; +import * as http from 'http'; +import * as fs from 'fs'; +import * as qs from 'querystring'; +import * as stream from 'stream'; +import { EventEmitter } from 'events'; +import { closeWindow } from './window-helpers'; +import { emittedOnce } from './events-helpers'; +import { WebmGenerator } from './video-helpers'; + +const fixturesPath = path.resolve(__dirname, '..', 'spec', 'fixtures'); + +const registerStringProtocol = protocol.registerStringProtocol; +const registerBufferProtocol = protocol.registerBufferProtocol; +const registerFileProtocol = protocol.registerFileProtocol; +const registerHttpProtocol = protocol.registerHttpProtocol; +const registerStreamProtocol = protocol.registerStreamProtocol; +const interceptStringProtocol = protocol.interceptStringProtocol; +const interceptBufferProtocol = protocol.interceptBufferProtocol; +const interceptHttpProtocol = protocol.interceptHttpProtocol; +const interceptStreamProtocol = protocol.interceptStreamProtocol; +const unregisterProtocol = protocol.unregisterProtocol; +const uninterceptProtocol = protocol.uninterceptProtocol; + +const text = 'valar morghulis'; +const protocolName = 'sp'; +>>>>>>> 81d09bea4... fix: correctly handle nexttick scheduling in stream reads (#24022) const postData = { name: 'post test', type: 'string' @@ -406,38 +438,94 @@ describe('protocol module', () => { statusCode: 200, headers: { 'Content-Type': 'text/plain' }, data: getStream(1024 * 1024, Buffer.alloc(1024 * 1024 * 2)).pipe(dumbPassthrough()) - }) - }) - const r = await ajax(protocolName + '://fake-host') - expect(r.data).to.have.lengthOf(1024 * 1024 * 2) - }) - }) - - describe('protocol.isProtocolHandled', () => { - it('returns true for built-in protocols', async () => { - for (const p of ['about', 'file', 'http', 'https']) { - const handled = await protocol.isProtocolHandled(p) - expect(handled).to.be.true(`${p}: is handled`) + }); + }); + const r = await ajax(protocolName + '://fake-host'); + expect(r.data).to.have.lengthOf(1024 * 1024 * 2); + }); + + it('can handle next-tick scheduling during read calls', async () => { + const events = new EventEmitter(); + function createStream () { + const buffers = [ + Buffer.alloc(65536), + Buffer.alloc(65537), + Buffer.alloc(39156) + ]; + const e = new stream.Readable({ highWaterMark: 0 }); + e.push(buffers.shift()); + e._read = function () { + process.nextTick(() => this.push(buffers.shift() || null)); + }; + e.on('end', function () { + events.emit('end'); + }); + return e; } - }) - - it('returns false when scheme is not registered', async () => { - const result = await protocol.isProtocolHandled('no-exist') - expect(result).to.be.false('no-exist: is handled') - }) - - it('returns true for custom protocol', async () => { - await registerStringProtocol(protocolName, (request, callback) => callback()) - const result = await protocol.isProtocolHandled(protocolName) - expect(result).to.be.true('custom protocol is handled') - }) - - it('returns true for intercepted protocol', async () => { - await interceptStringProtocol('http', (request, callback) => callback()) - const result = await protocol.isProtocolHandled('http') - expect(result).to.be.true('intercepted protocol is handled') - }) - }) + registerStreamProtocol(protocolName, (request, callback) => { + callback({ + statusCode: 200, + headers: { 'Content-Type': 'text/plain' }, + data: createStream() + }); + }); + const hasEndedPromise = emittedOnce(events, 'end'); + ajax(protocolName + '://fake-host'); + await hasEndedPromise; + }); + + it('can handle next-tick scheduling during read calls', async () => { + const events = new EventEmitter(); + function createStream () { + const buffers = [ + Buffer.alloc(65536), + Buffer.alloc(65537), + Buffer.alloc(39156) + ]; + const e = new stream.Readable({ highWaterMark: 0 }); + e.push(buffers.shift()); + e._read = function () { + process.nextTick(() => this.push(buffers.shift() || null)); + }; + e.on('end', function () { + events.emit('end'); + }); + return e; + } + registerStreamProtocol(protocolName, (request, callback) => { + callback({ + statusCode: 200, + headers: { 'Content-Type': 'text/plain' }, + data: createStream() + }); + }); + const hasEndedPromise = emittedOnce(events, 'end'); + ajax(protocolName + '://fake-host'); + await hasEndedPromise; + }); + }); + + describe('protocol.isProtocolRegistered', () => { + it('returns false when scheme is not registered', () => { + const result = protocol.isProtocolRegistered('no-exist'); + expect(result).to.be.false('no-exist: is handled'); + }); + + it('returns true for custom protocol', () => { + registerStringProtocol(protocolName, (request, callback) => callback('')); + const result = protocol.isProtocolRegistered(protocolName); + expect(result).to.be.true('custom protocol is handled'); + }); + }); + + describe('protocol.isProtocolIntercepted', () => { + it('returns true for intercepted protocol', () => { + interceptStringProtocol('http', (request, callback) => callback('')); + const result = protocol.isProtocolIntercepted('http'); + expect(result).to.be.true('intercepted protocol is handled'); + }); + }); +>>>>>>> 81d09bea4... fix: correctly handle nexttick scheduling in stream reads (#24022) describe('protocol.intercept(Any)Protocol', () => { it('throws error when scheme is already intercepted', (done) => {