Skip to content

Disable auto reload if page is not visible #3483

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.

Sign up for GitHub

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

Merged
bmd3k merged 2 commits into from
Apr 7, 2020
Merged

Disable auto reload if page is not visible #3483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1ee1405
Do not auto reload if document is not visible.
bmd3k Apr 6, 2020
9f9ef9a
Fix removal of visibilitychange event listener
bmd3k Apr 6, 2020
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
47 changes: 44 additions & 3 deletions tensorboard/components/tf_tensorboard/autoReloadBehavior.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,34 @@ namespace tf_tensorboard {
_autoReloadId: {
type: Number,
},
// Tracks whethere an auto reload was missed because the document was not visible.
_missedAutoReload: {
type: Boolean,
value: false,
},
_boundHandleVisibilityChange: {
type: Object,
},
autoReloadIntervalSecs: {
type: Number,
value: 30,
},
},
attached: function() {
this._boundHandleVisibilityChange_ = this._handleVisibilityChange.bind(
this
);
document.addEventListener(
'visibilitychange',
this._boundHandleVisibilityChange
);
},
detached: function() {
window.clearTimeout(this._autoReloadId);
document.removeEventListener(
'visibilitychange',
this._boundHandleVisibilityChange
);
},
_autoReloadObserver: function(autoReload) {
window.localStorage.setItem(AUTORELOAD_LOCALSTORAGE_KEY, autoReload);
Expand All @@ -59,14 +80,34 @@ namespace tf_tensorboard {
}
},
_doAutoReload: function() {
if (this.reload == null) {
throw new Error('AutoReloadBehavior requires a reload method');
if (this._isDocumentVisible()) {
this._doReload();
} else {
this._missedAutoReload = true;
}
this.reload();
this._autoReloadId = window.setTimeout(
() => this._doAutoReload(),
this.autoReloadIntervalSecs * 1000
);
},
_doReload: function() {
if (this.reload == null) {
Copy link
Contributor

@stephanwlee stephanwlee Apr 6, 2020

Choose a reason for hiding this comment

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

Prefer triple equals.

Copy link
Contributor Author

@bmd3k bmd3k Apr 6, 2020

Choose a reason for hiding this comment

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

I didn't write this originally so can't say for sure: But I wouldn't be surprised if the check is purposefully "==" to also catch when this.reload is undefined.

Copy link
Contributor

@stephanwlee stephanwlee Apr 6, 2020

Choose a reason for hiding this comment

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

ACK

throw new Error('AutoReloadBehavior requires a reload method');
}
this.reload();
},
_handleVisibilityChange: function() {
if (this._isDocumentVisible() && this._missedAutoReload) {
this._missedAutoReload = false;
this._doReload();
}
},
/**
* Wraps Page Visibility API call to determine if document is visible.
* Can be overriden for testing purposes.
*/
_isDocumentVisible: function() {
return document.visibilityState === 'visible';
},
};
} // namespace tf_tensorboard
129 changes: 129 additions & 0 deletions tensorboard/components/tf_tensorboard/test/autoReloadTests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ namespace tf_tensorboard {
const key = AUTORELOAD_LOCALSTORAGE_KEY;
let clock;
let callCount: number;
let sandbox: any;
let isDocumentVisible = true;

beforeEach(function() {
ls.setItem(key, 'false'); // start it turned off so we can mutate fns
Expand All @@ -35,6 +37,16 @@ namespace tf_tensorboard {
testElement.reload = function() {
callCount++;
};

sandbox = sinon.sandbox.create();
sandbox.stub(testElement, '_isDocumentVisible', function() {
return isDocumentVisible;
});
});

afterEach(function() {
isDocumentVisible = true;
sandbox.restore();
});

before(function() {
Expand All @@ -45,6 +57,11 @@ namespace tf_tensorboard {
clock.restore();
});

function simulateVisibilityChange(visibility) {
isDocumentVisible = visibility;
testElement._handleVisibilityChange();
}

it('reads and writes autoReload state from localStorage', function() {
ls.removeItem(key);
testElement = fixture('autoReloadFixture');
Expand Down Expand Up @@ -99,6 +116,118 @@ namespace tf_tensorboard {
clock.tick(5000);
});
});

it('does not reload if document is not visible', function() {
testElement.autoReloadIntervalSecs = 1;
testElement.autoReloadEnabled = true;

clock.tick(1000);
chai.assert.equal(callCount, 1, 'ticking clock triggered call');

simulateVisibilityChange(false);
clock.tick(1000);
chai.assert.equal(
callCount,
1,
'ticking clock while not visible did not trigger call'
);
});

it('reloads when document becomes visible if missed reload', function() {
testElement.autoReloadIntervalSecs = 1;
testElement.autoReloadEnabled = true;

simulateVisibilityChange(false);
clock.tick(1000);
chai.assert.equal(
callCount,
0,
'ticking clock while not visible did not trigger call'
);

simulateVisibilityChange(true);
chai.assert.equal(callCount, 1, 'visibility change triggered call');
});

it('reloads when document becomes visible if missed reload, regardless of how long not visible', function() {
testElement.autoReloadIntervalSecs = 1;
testElement.autoReloadEnabled = true;
clock.tick(1000);
chai.assert.equal(callCount, 1, 'ticking clock triggered call');

// Document is not visible during time period that includes missed auto reload but is less than
// autoReloadIntervalSecs
clock.tick(300);
simulateVisibilityChange(false);
clock.tick(800);
chai.assert.equal(
callCount,
1,
'ticking clock while not visible did not trigger call'
);

simulateVisibilityChange(true);
chai.assert.equal(callCount, 2, 'visibility change triggered call');
});

it('does not reload when document becomes visible if there was not a missed reload', function() {
testElement.autoReloadIntervalSecs = 1;
testElement.autoReloadEnabled = true;

clock.tick(1000);
chai.assert.equal(callCount, 1, 'ticking clock triggered call');

// Document is not visible during time period that does not include missed auto reload.
simulateVisibilityChange(false);
clock.tick(500);
simulateVisibilityChange(true);
chai.assert.equal(
callCount,
1,
'visibility change did not trigger call'
);
});

it('does not reload when document becomes visible if missed reload was already handled', function() {
testElement.autoReloadIntervalSecs = 1;
testElement.autoReloadEnabled = true;

simulateVisibilityChange(false);
clock.tick(1200);
chai.assert.equal(
callCount,
0,
'ticking clock while not visible did not trigger call'
);

simulateVisibilityChange(true);
chai.assert.equal(callCount, 1, 'visibility change triggered call');

// Document is not visible during time period that does not include another missed reload.
simulateVisibilityChange(false);
clock.tick(200);
simulateVisibilityChange(true);
chai.assert.equal(
callCount,
1,
'visibility change did not trigger call'
);
});

it('does not reload when document becomes visible if auto reload is off', function() {
testElement.autoReloadIntervalSecs = 1;
testElement.autoReloadEnabled = false;

simulateVisibilityChange(false);
clock.tick(5000);

simulateVisibilityChange(true);
chai.assert.equal(
callCount,
0,
'visibility change did not trigger call'
);
});
});
});
} // namespace tf_tensorboard