[wip] API review 1.11

[pavelfeldman]

Browser

• [ ] Document newContext
  • our playwright.dev/dotnet does not show newContext(options) method
• [x] Add newPage(options) => #1358

BrowserContext

• [x] GetCookiesAsync has two versions with enumerable and array. Should we auto-generate things like that? => #1343

BrowserType

• [ ] Launch/LaunchPersistentContext - document (options) variants => #1331

Devices

• [x] There are playwright.Devices in some examples, but I don't see them in the code. => #1359

FileChooser

• [ ] IsMultiple getter vs method. Is implies a check, As implies action, so it is unusual to see those as properties.

JSHandle

• [x] AsElement getter vs method => https://github.com/microsoft/playwright-sharp/pull/1374

Page

• [x] SetDefaultNavigationTimeout, SetDefaultTimeout (same for context) it is a get/set in the API, should not be => #1367
• [x] GoToAsync should be GotoAsync => #1360
• [x] Task OpenerAsync() should be sync

Request

• [ ] IsNavigationRequest getter vs method
• [x] GetPayloadAsJson - we don't have this method, why adding? => #1361

Response

• [x] Response.statusCode - let's remove this

Route

• [x] Can we await in route handler? I did not find a test for this.

WebSocket

• [ ] Duplicated WaitForEventAsync - why? https://github.com/microsoft/playwright-sharp/blob/main/src/Playwright/WebSocket.cs#L62-L113

Generic questions

• [x] Tests use TaskUtils.WhenAll (similar to Promise.all) - do we want java-style callbacks instead?
• [x] Threading model. We use ConfigureAwait(false) everywhere which means our code runs on unpredictable threads. Therefore we use ConcurrentDictionary in many places. Should we also use concurrent lists instead of arrays? Is there any way to protect against concurrent use of our APIs? What are our threading assumptions/guarantees?
• [ ] On which thread do we call event listeners? We would like to be single-threaded "like UI", so we have to ensure we call the listeners on some predefined thread.

[kblok]

Task OpenerAsync() should be sync

@pavelfeldman do you want resume the conversation here https://github.com/microsoft/playwright/pull/6408 ?

[kblok]

@pavelfeldman description updated based on issues that were already created or created after going through your comments. A few questions:

Document newContext

What do you mean by that?

• [ ] [nit, open] Do we want Accept/Dismiss to be Async?

They involve communication with the driver. They should be async.

• [ ] Response.statusCode - what is that?

It's a map of the int code to httpstatuscode https://docs.microsoft.com/en-us/dotnet/api/system.net.httpstatuscode?view=net-5.0. Users will prefer this over an int. We could either leave it like this or change the docs upstream to generate only the httpstatuscode version.

• [ ] Tests use TaskUtils.WhenAll (similar to Promise.all) - do we want java-style callbacks instead?

I would like you to show me some examples there. But my feeling is that a C# is way more closer to a TypeScript than a Java coding style.

• [ ] Threading model. We use ConfigureAwait(false) everywhere which means our code runs on unpredictable threads. Therefore we use ConcurrentDictionary in many places. Should we also use concurrent lists instead of arrays? Is there any way to protect against concurrent use of our APIs? What are our threading assumptions/guarantees?

I think that all our arrays comes already "cooked" from the driver. If you saw any array that we are building from several driver messages we DO need to take a look at it.

getter vs method

I think that if it sounds like a property, and we can generate a property, why not?

Can we await in route handler? I did not find a test for this.

We can't. We are doing fire and forgets on those callbacks now. I would wait for some specific requests here. That would be an overload.

[pavelfeldman]

@pavelfeldman do you want resume the conversation here microsoft/playwright#6408 ?

No, why?

[kblok]

Can we close this @pavelfeldman ?