Ignore resize observation caused by server resizes

If we increase the remote screen size from the server in such a way that
it no longer fits the browser window, the browser will probably want to
show scrollbars. The same happens if you enable 'clipping' while the
remote is larger than the browser window. These scrollbars do, in turn,
decrease the available space in the browser window. This causes our
ResizeObserver to trigger.

If the resize observation triggers a requestRemoteResize() we will
overwrite the size and request a new one just because scrollbars have
appeared. We don't want that.

We can save the expected client size after resizing, and then compare
the current client size with the expected one. If there is no change
compared to the expected size, we shouldn't send the request.

Fixes issue #1616.
This commit is contained in:
Samuel Mannehed 2021-12-10 20:11:05 +01:00
parent 6cd69705d6
commit 0ff0844a14
2 changed files with 113 additions and 2 deletions

View File

@ -244,6 +244,8 @@ export default class RFB extends EventTargetMixin {
this._sock.on('message', this._handleMessage.bind(this));
this._sock.on('error', this._socketError.bind(this));
this._expectedClientWidth = null;
this._expectedClientHeight = null;
this._resizeObserver = new ResizeObserver(this._eventHandlers.handleResize);
// All prepared, kick off the connection
@ -623,7 +625,26 @@ export default class RFB extends EventTargetMixin {
{ detail: { name: this._fbName } }));
}
_saveExpectedClientSize() {
this._expectedClientWidth = this._screen.clientWidth;
this._expectedClientHeight = this._screen.clientHeight;
}
_currentClientSize() {
return [this._screen.clientWidth, this._screen.clientHeight];
}
_clientHasExpectedSize() {
const [currentWidth, currentHeight] = this._currentClientSize();
return currentWidth == this._expectedClientWidth &&
currentHeight == this._expectedClientHeight;
}
_handleResize() {
// Don't change anything if the client size is already as expected
if (this._clientHasExpectedSize()) {
return;
}
// If the window resized then our screen element might have
// as well. Update the viewport dimensions.
window.requestAnimationFrame(() => {
@ -664,6 +685,12 @@ export default class RFB extends EventTargetMixin {
this._display.viewportChangeSize(size.w, size.h);
this._fixScrollbars();
}
// When changing clipping we might show or hide scrollbars.
// This causes the expected client dimensions to change.
if (curClip !== newClip) {
this._saveExpectedClientSize();
}
}
_updateScale() {
@ -688,6 +715,7 @@ export default class RFB extends EventTargetMixin {
}
const size = this._screenSize();
RFB.messages.setDesktopSize(this._sock,
Math.floor(size.w), Math.floor(size.h),
this._screenID, this._screenFlags);
@ -2507,6 +2535,9 @@ export default class RFB extends EventTargetMixin {
this._updateScale();
this._updateContinuousUpdates();
// Keep this size until browser client size changes
this._saveExpectedClientSize();
}
_xvpOp(ver, op) {

View File

@ -540,6 +540,10 @@ describe('Remote Frame Buffer Protocol Client', function () {
sinon.spy(client._display, "viewportChangeSize");
client._sock._websocket._receiveData(new Uint8Array(incoming));
// The resize will cause scrollbars on the container, this causes a
// resize observation in the browsers
fakeResizeObserver.fire();
clock.tick(1000);
// FIXME: Display implicitly calls viewportChangeSize() when
// resizing the framebuffer, hence calledTwice.
@ -571,6 +575,32 @@ describe('Remote Frame Buffer Protocol Client', function () {
expect(client._display.viewportChangeSize).to.not.have.been.called;
});
describe('Clipping and remote resize', function () {
beforeEach(function () {
// Given a remote (100, 100) larger than the container (70x80),
client._resize(100, 100);
client._supportsSetDesktopSize = true;
client.resizeSession = true;
sinon.spy(RFB.messages, "setDesktopSize");
});
afterEach(function () {
RFB.messages.setDesktopSize.restore();
});
it('should not change remote size when changing clipping', function () {
// When changing clipping the scrollbars of the container
// will appear and disappear and thus trigger resize observations
client.clipViewport = false;
fakeResizeObserver.fire();
clock.tick(1000);
client.clipViewport = true;
fakeResizeObserver.fire();
clock.tick(1000);
// Then no resize requests should be sent
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
});
});
describe('Dragging', function () {
beforeEach(function () {
client.dragViewport = true;
@ -734,6 +764,10 @@ describe('Remote Frame Buffer Protocol Client', function () {
sinon.spy(client._display, "autoscale");
client._sock._websocket._receiveData(new Uint8Array(incoming));
// The resize will cause scrollbars on the container, this causes a
// resize observation in the browsers
fakeResizeObserver.fire();
clock.tick(1000);
expect(client._display.autoscale).to.have.been.calledOnce;
expect(client._display.autoscale).to.have.been.calledWith(70, 80);
@ -830,6 +864,35 @@ describe('Remote Frame Buffer Protocol Client', function () {
expect(RFB.messages.setDesktopSize).to.have.been.calledWith(sinon.match.object, 40, 50, 0, 0);
});
it('should not request the same size twice', function () {
container.style.width = '40px';
container.style.height = '50px';
fakeResizeObserver.fire();
clock.tick(1000);
expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
expect(RFB.messages.setDesktopSize).to.have.been.calledWith(
sinon.match.object, 40, 50, 0, 0);
// Server responds with the requested size 40x50
const incoming = [ 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
0x00, 0x28, 0x00, 0x32, 0xff, 0xff, 0xfe, 0xcc,
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x28, 0x00, 0x32,
0x00, 0x00, 0x00, 0x00];
client._sock._websocket._receiveData(new Uint8Array(incoming));
clock.tick(1000);
RFB.messages.setDesktopSize.resetHistory();
// size is still 40x50
fakeResizeObserver.fire();
clock.tick(1000);
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
});
it('should not resize until the container size is stable', function () {
container.style.width = '20px';
container.style.height = '30px';
@ -885,16 +948,33 @@ describe('Remote Frame Buffer Protocol Client', function () {
});
it('should not try to override a server resize', function () {
// Simple ExtendedDesktopSize FBU message
// Simple ExtendedDesktopSize FBU message, new size: 100x100
const incoming = [ 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00,
0x00, 0x04, 0x00, 0x04, 0xff, 0xff, 0xfe, 0xcc,
0x00, 0x64, 0x00, 0x64, 0xff, 0xff, 0xfe, 0xcc,
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x04,
0x00, 0x00, 0x00, 0x00 ];
// Note that this will cause the browser to display scrollbars
// since the framebuffer is 100x100 and the container is 70x80.
// The usable space (clientWidth/clientHeight) will be even smaller
// due to the scrollbars taking up space.
client._sock._websocket._receiveData(new Uint8Array(incoming));
// The scrollbars cause the ResizeObserver to fire
fakeResizeObserver.fire();
clock.tick(1000);
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
// An actual size change must not be ignored afterwards
container.style.width = '120px';
container.style.height = '130px';
fakeResizeObserver.fire();
clock.tick(1000);
expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
expect(RFB.messages.setDesktopSize.firstCall.args[1]).to.equal(120);
expect(RFB.messages.setDesktopSize.firstCall.args[2]).to.equal(130);
});
});