fix(events): avoid firing lifecycle events twice (#16055)

Previously, when some iframe started/finished a new navigation, we
could have removed and then re-addded load/domcontentloaded on the
main frame.

Drive-by: unflake wheel test in Firefox.
This commit is contained in:
Dmitry Gozman 2022-07-29 12:44:04 -07:00 committed by GitHub
parent af8e3e7afa
commit 809002df60
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 13 additions and 4 deletions

View File

@ -530,7 +530,7 @@ export class Frame extends SdkObject {
_onClearLifecycle() {
this._firedLifecycleEvents.clear();
// Recalculate subtree lifecycle for the whole tree - it should not be that big.
this._page.mainFrame()._recalculateLifecycle();
this._page.mainFrame()._recalculateLifecycle(this);
// Keep the current navigation request if any.
this._inflightRequests = new Set(Array.from(this._inflightRequests).filter(request => request === this._currentDocument.request));
this._stopNetworkIdleTimer();
@ -593,10 +593,10 @@ export class Frame extends SdkObject {
});
}
_recalculateLifecycle() {
_recalculateLifecycle(frameThatAllowsRemovingLifecycleEvents?: Frame) {
const events = new Set<types.LifecycleEvent>(this._firedLifecycleEvents);
for (const child of this._childFrames) {
child._recalculateLifecycle();
child._recalculateLifecycle(frameThatAllowsRemovingLifecycleEvents);
// We require a particular lifecycle event to be fired in the whole
// frame subtree, and then consider it done.
for (const event of events) {
@ -604,6 +604,12 @@ export class Frame extends SdkObject {
events.delete(event);
}
}
if (frameThatAllowsRemovingLifecycleEvents !== this) {
// Usually, lifecycle events are fired once and not removed after that, so we keep existing ones.
// However, when we clear them right before a new commit, this is allowed for a particular frame.
for (const event of this._subtreeLifecycleEvents)
events.add(event);
}
const mainFrame = this._page.mainFrame();
for (const event of events) {
// Checking whether we have already notified about this event.

View File

@ -30,7 +30,7 @@ it('should fire once', async ({ page, server, browserName }) => {
it('should fire once with iframe navigation', async ({ page, server, browserName }) => {
it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/15086' });
it.fail();
it.fixme(browserName === 'firefox', 'Firefox sometimes double fires.');
let requestCount = 0;
server.setRoute('/tracker', (_, res) => {

View File

@ -148,6 +148,9 @@ it('should work when the event is canceled', async ({ page }) => {
await page.evaluate(() => {
document.querySelector('div').addEventListener('wheel', e => e.preventDefault());
});
// Give wheel listener a chance to propagate through all the layers in Firefox.
for (let i = 0; i < 10; i++)
await page.evaluate(() => new Promise(x => requestAnimationFrame(() => requestAnimationFrame(x))));
await page.mouse.wheel(0, 100);
await expectEvent(page, {
deltaX: 0,