Skip to content
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.

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

feat: add Page.waitForDevicePrompt #9299

Merged
merged 17 commits into from
Mar 21, 2023

Conversation

mzgoddard
Copy link
Contributor

What kind of change does this PR introduce?

Add a feature through Page.waitForDevicePrompt() and DeviceRequestPrompt to respond to DeviceAccess.deviceRequestPrompted protocol events.

Did you add tests for your changes?

This change adds tests for the interface that simulate interaction with a mock session object.

These tests cover the added code except for Page.waitForDevicePrompt itself.

If relevant, did you update the documentation?

Page.waitForDevicePrompt includes a jsdoc with a comment with a likely common use case:

const [deviceRequest] = Promise.all([
  page.waitForDevicePrompt(),
  page.click('#connect-bluetooth'),
]);
await devicePrompt.select(
  await devicePrompt.waitForDevice(({name}) => name.includes('My Device'))
);

Summary

Page.waitForDevicePrompt resolves to a DeviceRequestPrompt when the next DeviceAccess.deviceRequestPrompted events are emitted by the session. DeviceRequestPrompt.select and DeviceRequestPrompt.cancel can be be called to respond to the prompt. DeviceRequestPrompt.waitForDevice resolves to the first device in the prompt that matches a given filter. That device can be listed in DeviceRequestPrompt.devices when waitForDevice is called or from future DeviceAccess.deviceRequestPrompted events.

This wraps devtools protocol events and methods added in
https://chromium-review.googlesource.com/c/chromium/src/+/3933844. That commit log integrates those events with WebBluetooth. When WebBluetooth's requestDevice is called, after some steps are performed, if they succeed, a prompt for the user is opened to select a device. This interface and the protocol additions it depends on add a way to automate responding to that prompt.

@mzgoddard
Copy link
Contributor Author

To further test this interface with the devtools protocol additions, without mocking them, a test environment with a specific hardware setup like bluetooth hardware with nearby reliable devices or some degree of detailed virtualization. Chromium uses Mojo services to fake parts of its WebBluetooth implementation in tests. Tests here could use that as well but these features are not normally shipped, to my knowledge, because the size of the released build they add. But I am thinking that would be "too much" here to automate testting this code, so I've used a mock session object here to test it.

The commit log https://chromium-review.googlesource.com/c/chromium/src/+/3933844 isn't merged yet, so I used a npm-linked version of devtools-protocol I've pushed to https://github.com/mzgoddard/devtools-protocol/tree/device-access-device-request-prompted while writing this change and testing it.

@OrKoN OrKoN self-requested a review November 22, 2022 08:14
@OrKoN
Copy link
Collaborator

OrKoN commented Nov 22, 2022

@mzgoddard thanks for the PR. Would you mind rebasing?

Michael "Z" Goddard added 5 commits February 13, 2023 17:21
Add Page.waitForDevicePrompt() and DeviceRequestPrompt.

Page.waitForDevicePrompt resolves to a DeviceRequestPrompt when the next
DeviceAccess.deviceRequestPrompted events are emitted by the session.
DeviceRequestPrompt.select and DeviceRequestPrompt.cancel can be be
called to respond to the prompt.
@mzgoddard
Copy link
Contributor Author

I rebased onto main. The CL this depends on has merged and devtools-bot pushed to devtools-protocol a commit including the change ChromeDevTools/devtools-protocol@97f8fcb.

As I understand it, puppeteer tracks Chrome's Beta release. So, this PR will wait until then when the dependency on devtools-protocol will be bumped to include that. Is that right?

@OrKoN
Copy link
Collaborator

OrKoN commented Feb 14, 2023

@mzgoddard yes, that's right. We can land it once we rolled the Chromium binary version that supports this feature.

@@ -0,0 +1,77 @@
// AUTOGENERATED - Use `npm run generate:sources` to regenerate.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should not be committed. I think the file has been removed from the tree but accidentally pushed here.

@OrKoN
Copy link
Collaborator

OrKoN commented Mar 16, 2023

@mzgoddard let me know if need help rebasing this. We can land it now.

@OrKoN
Copy link
Collaborator

OrKoN commented Mar 17, 2023

@mzgoddard could you please run npm run format to update the formatting based on the latest rules?

@OrKoN OrKoN enabled auto-merge (squash) March 21, 2023 06:18
@OrKoN
Copy link
Collaborator

OrKoN commented Mar 21, 2023

Also, the docs need to be re-generated npm run docs. The current comments seem to differ from the generated markdown.

@OrKoN OrKoN disabled auto-merge March 21, 2023 09:21
@OrKoN OrKoN merged commit a5149d5 into puppeteer:main Mar 21, 2023
@release-please release-please bot mentioned this pull request Mar 21, 2023
thiagowfx added a commit that referenced this pull request Oct 13, 2023
Only call `deviceRequestPromptManager()` if the parent frame is not
null.

Follow-up-of: #9299
thiagowfx added a commit that referenced this pull request Oct 13, 2023
Only call `deviceRequestPromptManager()` if the parent frame is not
null.

Bug: #11072
Follow-up-of: #9299
thiagowfx added a commit that referenced this pull request Oct 13, 2023
Only call `deviceRequestPromptManager()` if the parent frame is not
null.

Bug: #11072
Follow-up-of: #9299
thiagowfx added a commit that referenced this pull request Oct 13, 2023
Only call `deviceRequestPromptManager()` if the parent frame is not
null.

Bug: #11072
Follow-up-of: #9299
thiagowfx added a commit that referenced this pull request Oct 13, 2023
Only call `deviceRequestPromptManager()` if the parent frame is not
null.

Bug: #11072
Follow-up-of: #9299
thiagowfx added a commit that referenced this pull request Oct 13, 2023
Only call `deviceRequestPromptManager()` if the parent frame is not
null.

Bug: #11072
Follow-up-of: #9299
thiagowfx added a commit that referenced this pull request Oct 15, 2023
Only call `deviceRequestPromptManager()` if the parent frame is not
null.

Bug: #11072
Follow-up-of: #9299
devices: [],
});

expect(manager).toBeTruthy();
Copy link
Contributor

Choose a reason for hiding this comment

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

@mzgoddard, sorry for quite a late comment. But what's the purpose of this expect?
How the manager won't be truthy?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants