Closed Bug 1732958 Opened 3 years ago Closed 3 years ago

Sync vendored puppeteer to v13.0.1

Categories

(Remote Protocol :: CDP, task, P2)

Product:
Remote Protocol
Component:
CDP
Type:
task
Points:
8

Tracking

(firefox97 fixed)

Status:
RESOLVED FIXED
Milestone:
97 Branch
Tracking Flags:
Tracking Status
firefox97 --- fixed

People

(Reporter: whimboo, Assigned: jdescottes)

References

Details

(Whiteboard: [bidi-m3-mvp])

Attachments

(3 files)

Bug 1732958 - [puppeteer] Update hgignore to ignore new puppeteer folders
48 bytes, text/x-phabricator-request
Details | Review
Bug 1732958 - [puppeteer] Sync puppeteer v13.0.1
48 bytes, text/x-phabricator-request
Details | Review
Bug 1732958 - [puppeteer] Skip 2 new mandatory tests on Firefox
48 bytes, text/x-phabricator-request
Details | Review
Assignee: nobody → hskupin
Status: NEW → ASSIGNED

Turns out this is not necessary at the moment. As such we can delay for a bit. But note that a huge amount of test changes are coming in, which makes it hard to setup the right expectation data.

Assignee: hskupin → nobody
Status: ASSIGNED → NEW
No longer blocks: 1601245

Not needed for M2.

Whiteboard: [bidi-m2-mvp]

Puppeteer version 11.0.0 is out now:
https://github.com/puppeteer/puppeteer/releases/tag/v11.0.0

Summary: Sync vendored puppeteer to v10.4.0 → Sync vendored puppeteer to v11.0.0

Via bug 1713030 I landed some patches for better handling existing Firefox profiles. As best we should wait until that is included in the next release before vendoring a new version.

Puppeteer version 12.0.0 is out now:
https://github.com/puppeteer/puppeteer/releases/tag/v12.0.0

We should consider vendoring in the changes now to stay up-to-date.

Summary: Sync vendored puppeteer to v11.0.0 → Sync vendored puppeteer to v12.0.0
Whiteboard: [webdriver:triage]
Points: --- → 8
Priority: -- → P3
Whiteboard: [webdriver:triage] → [bidi-m2-mvp]

Puppeteer version 13.0.0 is out now:
https://github.com/puppeteer/puppeteer/releases/tag/v13.0.0

Summary: Sync vendored puppeteer to v12.0.0 → Sync vendored puppeteer to v13.0.0
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED

Sorry about the lack of updates here.

Summary of test changes for Puppeteer 13.0.0

Breakdown per test file

elementhandle.spec.ts

New tests: 4

PASS: ElementHandle specs Element.waitForSelector should wait correctly with waitForSelector on an element
PASS: ElementHandle specs ElementHandle.isIntersectingViewport should work with threshold
PASS: ElementHandle specs ElementHandle.isIntersectingViewport should work with threshold of 1
PASS: ElementHandle specs Custom queries should wait correctly with waitForSelector on an element

emulation.spec.ts

New tests: 1

FAIL: Emulation Page.emulateCPUThrottling should change the CPU throttling rate successfully

frame.spec.ts

New tests: 1

PASS: Frame specs Frame.evaluate allows readonly array to be an argument

Existing tests: 2

PASS -> TIMEOUT: Frame specs Frame.evaluate should throw for detached frames
PASS -> FAIL: Frame specs Frame Management should report different frame instance when frame re-attaches

jshandle.spec.ts

New tests: 5

PASS: JSHandle JSHandle.getProperty should work
PASS: JSHandle JSHandle.getProperty should return a JSHandle even if the property does not exist
PASS: JSHandle JSHandle.clickablePoint should work
PASS: JSHandle JSHandle.clickablePoint should work for iframes
FAIL: JSHandle JSHandle.click should work

Existing tests: 1

REMOVED: JSHandle Page.evaluateHandle should work with primitives

launcher.spec.ts

New tests: 4

PASS: Launcher specs Puppeteer Puppeteer.launch should pass the timeout parameter to browser.waitForTarget
PASS: Launcher specs Puppeteer Puppeteer.launch should set the debugging port
PASS: Launcher specs Puppeteer Puppeteer.launch should not allow setting debuggingPort and pipe
PASS: Launcher specs Puppeteer Puppeteer.executablePath returns executablePath for channel

Existing tests: 2

PASS -> FAIL: Launcher specs Puppeteer Puppeteer.connect should support targetFilter option
PASS -> FAIL: Launcher specs Puppeteer Puppeteer.connect should be able to reconnect to a disconnected browser

Note: Both failures started with https://github.com/puppeteer/puppeteer/pull/7722

navigation.spec.ts

Existing tests: 2

TIMEOUT -> FAIL: navigation Frame.goto should return matching responses
PASS -> TIMEOUT: navigation Frame.waitForNavigation should fail when frame detaches

network.spec.ts

New tests: 6

FAIL: network Request.initiator shoud return the initiator
TIMEOUT: network Response.buffer should throw if the response does not have a body
PASS: network Response.statusText handles missing status text
PASS: network raw network headers Same-origin set-cookie navigation
PASS: network raw network headers Same-origin set-cookie subresource
FAIL: network raw network headers Cross-origin set-cookie

page.spec.ts

New tests: 11

FAIL: Page removing and adding event handlers should correctly added and removed request events
FAIL: Page BrowserContext.overridePermissions should grant persistent-storage
PASS: Page Page.waitForNetworkIdle should work
PASS: Page Page.waitForNetworkIdle should respect timeout
PASS: Page Page.waitForNetworkIdle should respect idleTime
PASS: Page Page.waitForNetworkIdle should work with no timeout
FAIL: Page Page.exposeFunction should fallback to default export when passed a module object
FAIL: Page Page.setUserAgent should work with additional userAgentMetdata
PASS: Page Page.addScriptTag should add id when provided
PASS: Page printing to PDF can print to PDF and stream the result
PASS: Page printing to PDF should respect timeout

Existing tests: 1

TIMEOUT -> FAIL: Page Page.close should run beforeunload if asked for

Some tests failed to run because of an earlier test. Missing tests:

MISSING: Page Page.title should return the page title
MISSING: Page Page.select should select single option
MISSING: Page Page.select should select only first option
MISSING: Page Page.select should not throw when select causes navigation
MISSING: Page Page.select should select multiple options
MISSING: Page Page.select should respect event bubbling
MISSING: Page Page.select should throw when element is not a <select>
MISSING: Page Page.select should return [] on no matched values
MISSING: Page Page.select should return an array of matched values
MISSING: Page Page.select should return an array of one element when multiple is not set
MISSING: Page Page.select should return [] on no values
MISSING: Page Page.select should deselect all options when passed no values for a multiple select
MISSING: Page Page.select should deselect all options when passed no values for a select
MISSING: Page Page.select should throw if passed in non-strings
MISSING: Page Page.select should work when re-defining top-level Event class
MISSING: Page Page.Events.Close should work with window.close
MISSING: Page Page.Events.Close should work with page.close
MISSING: Page Page.browser should return the correct browser instance
MISSING: Page Page.browserContext should return the correct browser context instance

requestinterception-experimental.spec.ts

New tests: 49

FAIL: All 49 tests of the file

I suggest to keep the describeFailsFirefox for this one, we are not going to accidentally start supporting request interception.

screenshot.spec.ts

New tests: 1

PASS: Screenshots Page.screenshot should work with webp

waittask.spec.ts

New tests: 1

PASS: waittask specs Page.waitFor should allow you to select an element with parenthesis-starting xpath

Existing tests:

PASS -> TIMEOUT: waittask specs Frame.waitForSelector should throw when frame is detached
PASS -> TIMEOUT: waittask specs Frame.waitForXPath should throw when frame is detached

Analysis

There is a trend of issues in existing tests related to detached frames. Support for OOP iframes was added recently in Puppeteer, this might be related to the breakage. https://github.com/puppeteer/puppeteer/pull/7556

For the new failures in launcher.spec, a new assert was added in https://github.com/puppeteer/puppeteer/pull/7722. It relates to setting the context several times, which for some reason would only happen in Firefox? The following issue has been linked from the code: https://github.com/puppeteer/puppeteer/issues/4197

The situation with page.spec.ts is concerning since many tests seem to fail to even run. Will try a few other pushes to see if that's a consistent behavior or if it's intermittent. But we might have to skip another test in order to make sure all those existing tests can at least run.

Even when using the same itFailsFirefox/describeFailsFirefox, the tests are very often stopping in the middle of the suite.
I also started skipping the additional test from https://github.com/puppeteer/puppeteer/pull/7846 , doesn't help.
https://treeherder.mozilla.org/jobs?repo=try&revision=e59a5106aba10501612a6c157bd30728bc881207

We very often miss the results from page.spec.ts even though all the tests seem to pass when reading the logs.
There is an error thrown after the last test from the page.spec.ts file:

[task 2021-12-20T20:40:28.030Z] TEST-PASS | Page Page.client should return the client instance (page.spec.ts) | took 203ms
[task 2021-12-20T20:40:28.030Z] PID 407 | ["pass",{"title":"should return the client instance","fullTitle":"Page Page.client should return the client instance","file":"/builds/worker/checkouts/gecko/remote/test/puppeteer/test/page.spec.ts","duration":0,"currentRetry":0}]
[task 2021-12-20T20:40:28.030Z] PID 407 | 1640032828030	CDP	TRACE	CDPConnection 4eb8e1bc-2385-472b-bb73-509307c1187a -> {"method":"Target.disposeBrowserContext","params":{"browserContextId":85},"id":1602}
[task 2021-12-20T20:40:28.036Z] PID 407 | 1640032828035	CDP	TRACE	CDPConnection 4eb8e1bc-2385-472b-bb73-509307c1187a <- {"method":"Target.targetDestroyed","params":{"targetId":"bb9dd60d-4d88-4b62-9583-2359192b18a0"}}
[task 2021-12-20T20:40:28.039Z] PID 407 | 1640032828037	CDP	TRACE	CDPConnection 4eb8e1bc-2385-472b-bb73-509307c1187a <- {"id":1602,"result":{}}
[task 2021-12-20T20:40:28.074Z] PID 407 | 1640032828073	CDP	TRACE	CDPConnection 4eb8e1bc-2385-472b-bb73-509307c1187a -> {"method":"Browser.close","id":1603}
[task 2021-12-20T20:40:28.104Z] PID 407 | 1640032828104	CDP	TRACE	CDPConnection 4eb8e1bc-2385-472b-bb73-509307c1187a <- {"id":1603,"result":{}}
[task 2021-12-20T20:40:28.109Z] PID 407 | 1640032828108	CDP	TRACE	CDPConnection 4eb8e1bc-2385-472b-bb73-509307c1187a <- {"method":"Target.targetDestroyed","params":{"targetId":"4df0a88b-c5e6-4769-aabc-8f338d309ed4"}}
[task 2021-12-20T20:40:28.121Z] PID 407 | 1640032828119	RemoteAgent	DEBUG	Resetting recommended pref browser.contentblocking.introCount
[task 2021-12-20T20:40:28.121Z] PID 407 | 1640032828119	RemoteAgent	DEBUG	Resetting recommended pref browser.download.panel.shown
[task 2021-12-20T20:40:28.121Z] PID 407 | 1640032828119	RemoteAgent	DEBUG	Resetting recommended pref browser.tabs.closeWindowWithLastTab
[task 2021-12-20T20:40:28.121Z] PID 407 | 1640032828119	RemoteAgent	DEBUG	Resetting recommended pref browser.tabs.unloadOnLowMemory
[task 2021-12-20T20:40:28.121Z] PID 407 | 1640032828120	RemoteAgent	DEBUG	Resetting recommended pref browser.tabs.warnOnClose
[task 2021-12-20T20:40:28.121Z] PID 407 | 1640032828120	RemoteAgent	DEBUG	Resetting recommended pref browser.toolbars.bookmarks.visibility
[task 2021-12-20T20:40:28.121Z] PID 407 | 1640032828120	RemoteAgent	DEBUG	Resetting recommended pref browser.urlbar.merino.endpointURL
[task 2021-12-20T20:40:28.123Z] PID 407 | 1640032828121	RemoteAgent	DEBUG	Resetting recommended pref datareporting.policy.dataSubmissionPolicyAccepted
[task 2021-12-20T20:40:28.123Z] PID 407 | 1640032828121	RemoteAgent	DEBUG	Resetting recommended pref dom.push.connection.enabled
[task 2021-12-20T20:40:28.123Z] PID 407 | 1640032828121	RemoteAgent	DEBUG	Resetting recommended pref dom.successive_dialog_time_limit
[task 2021-12-20T20:40:28.123Z] PID 407 | 1640032828122	RemoteAgent	DEBUG	Resetting recommended pref extensions.getAddons.discovery.api_url
[task 2021-12-20T20:40:28.123Z] PID 407 | 1640032828122	RemoteAgent	DEBUG	Resetting recommended pref network.http.phishy-userpass-length
[task 2021-12-20T20:40:28.123Z] PID 407 | 1640032828123	RemoteAgent	DEBUG	Resetting recommended pref browser.topsites.contile.enabled
[task 2021-12-20T20:40:28.125Z] PID 407 | 1640032828125	RemoteAgent	TRACE	Received observer notification quit-application
[task 2021-12-20T20:40:28.136Z] PID 407 | 1640032828135	RemoteAgent	ERROR	unable to stop listener: TypeError: can't access property "sendAsyncMessage", this.mm is null(chrome://remote/content/cdp/sessions/TabSession.jsm:53:5) JS Stack trace: destructor@TabSession.jsm:53:5
[task 2021-12-20T20:40:28.136Z] PID 407 | onClosed@CDPConnection.jsm:193:15
[task 2021-12-20T20:40:28.136Z] PID 407 | close@WebSocketTransport.jsm:63:18
[task 2021-12-20T20:40:28.136Z] PID 407 | close@WebSocketConnection.jsm:50:20
[task 2021-12-20T20:40:28.136Z] PID 407 | destructor@Target.jsm:51:12
[task 2021-12-20T20:40:28.136Z] PID 407 | destroyTarget@TargetList.jsm:91:12
[task 2021-12-20T20:40:28.136Z] PID 407 | destructor@TargetList.jsm:102:12
[task 2021-12-20T20:40:28.136Z] PID 407 | stop@CDP.jsm:137:34
[task 2021-12-20T20:40:28.139Z] PID 407 | JavaScript error: chrome://remote/content/cdp/domains/parent/Emulation.jsm, line 154: TypeError: can't access property "customUserAgent", browsingContext is null
[task 2021-12-20T20:40:28.139Z] PID 407 | JavaScript error: chrome://remote/content/cdp/sessions/TabSession.jsm, line 78: TypeError: can't access property "sendAsyncMessage", this.mm is null

Maybe that prevents the test results from being reported successfully? Will try to skip this test as well.

Interesting. Would you mind filing a bug about the TypeError: can't access property "sendAsyncMessage", this.mm is null case? I've seen this in the past too but I cannot find any bug about it. It would be good to see this fixed given that it is unclear in how much it actually affects the test result. Btw is it always visible when running some particular tests?

Skipping more tests didn't help.

I also tried to vendor puppeteer 13.0.0 on top of an older central (96 NIGHTLY END), same issue.
And I tried to vendor older puppeteer versions: 12.0.0 and 11.0.0. Same issue with the page.spec.ts tests (see https://treeherder.mozilla.org/jobs?repo=try&revision=62fc341674843916db1dea8b2ad2be905c7bc768&selectedTaskRun=CbtYeasgT1eblPNAeuTIAg.0)

There have been several intermediary tags between 10.0.0 and 11.0.0, I can try to bisect even further.

TypeError: can't access property "sendAsyncMessage", this.mm is null

This error is very frequent, even today on central in successful runs. I think it happens at the end of a test "file" when closing the browser. I can be reproduced locally. Filed https://bugzilla.mozilla.org/show_bug.cgi?id=1747041

It seems the issues are starting with 10.4.0. 10.2.0 is the last tag for which I seem to get consistent try results for puppeteer.
Doing more retriggers to confirm.
Edit: Confirmed 10.2.0 is consistently green

https://github.com/puppeteer/puppeteer/compare/v10.2.0...v10.4.0

New tests added between 10.2 and 10.4:

ElementHandle specs ElementHandle.isIntersectingViewport should work with threshold (elementhandle.spec.ts)
ElementHandle specs ElementHandle.isIntersectingViewport should work with threshold of 1 (elementhandle.spec.ts)
Frame specs Frame.evaluate allows readonly array to be an argument (frame.spec.ts)
JSHandle JSHandle.clickablePoint should work (jshandle.spec.ts)
JSHandle JSHandle.clickablePoint should work for iframes (jshandle.spec.ts)
JSHandle JSHandle.click should work (jshandle.spec.ts)
Page BrowserContext.overridePermissions should grant persistent-storage (page.spec.ts)
Page Page.waitForNetworkIdle should work (page.spec.ts)
Page Page.waitForNetworkIdle should respect timeout (page.spec.ts)
Page Page.waitForNetworkIdle should respect idleTime (page.spec.ts)
Page Page.waitForNetworkIdle should work with no timeout (page.spec.ts)
Page Page.addScriptTag should add id when provided (page.spec.ts)
Page printing to PDF should respect timeout (page.spec.ts)
Page Page.client should return the client instance (page.spec.ts)
Screenshots Page.screenshot should work with webp (screenshot.spec.ts)
waittask specs Page.waitFor should allow you to select an element with parenthesis-starting xpath (waittask.spec.ts)

(excluding the experimental request interception ones which are all skipped on Firefox)

Trying to skip the additional page tests on the 10.4.0 vendored version: https://treeherder.mozilla.org/jobs?repo=try&revision=c429fb14a9d331bba2234fece052ce00cdb82414

Priority: P3 → P2
Whiteboard: [bidi-m2-mvp] → [bidi-m3-mvp]

I just saw that https://github.com/puppeteer/puppeteer/releases/tag/v13.0.1 has been released. Maybe it's worth integrating for this bug?

Depends on D134665

Summary: Sync vendored puppeteer to v13.0.0 → Sync vendored puppeteer to v13.0.1
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1478ef3f4795 [puppeteer] Update hgignore to ignore new puppeteer folders r=webdriver-reviewers,whimboo https://hg.mozilla.org/integration/autoland/rev/6668dbe63e5a [puppeteer] Sync puppeteer v13.0.1 r=webdriver-reviewers,whimboo https://hg.mozilla.org/integration/autoland/rev/357404b00c70 [puppeteer] Skip 2 new mandatory tests on Firefox r=webdriver-reviewers,whimboo
Blocks: 1748424

https://hg.mozilla.org/mozilla-central/rev/1478ef3f4795
https://hg.mozilla.org/mozilla-central/rev/6668dbe63e5a
https://hg.mozilla.org/mozilla-central/rev/357404b00c70

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
status-firefox97: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 97 Branch
Regressions: 1749266
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: