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 support for WebHID #30213

Merged
merged 13 commits into from Sep 23, 2021
Merged

feat: add support for WebHID #30213

merged 13 commits into from Sep 23, 2021

Conversation

jkleinsc
Copy link
Contributor

@jkleinsc jkleinsc commented Jul 20, 2021

Description of Change

This PR enables use of the WebHID API in Electron by exposing several new events on Session:

select-hid-device - fired when navigator.hid.requestDevice is called so that an Electron developer can programmatically select a HID device.
hid-device-added - fired when a new HID device is detected. Emitted after navigator.hid.requestDevice has been called and select-hid-device has fired if a new HID device becomes available. For example, this event will fire when a new USB device is plugged in.
hid-device-removed - fired when a HID device is removed. Emitted after navigator.hid.requestDevice has been called and select-hid-device has fired if a HID device has been removed. For example, this event will fire when a USB device is unplugged.

Additionally, there is a new handler, setDevicePermissionHandler(handler) on Session to be used by developers to provide default permissioning to devices without calling navigator.hid.requestDevice first.

Resolves #21311

Checklist

Release Notes

Notes: Added WebHID support

@jkleinsc jkleinsc added semver/minor backwards-compatible functionality target/14-x-y labels Jul 20, 2021
@electron-cation electron-cation bot added new-pr 🌱 PR opened in the last 24 hours api-review/requested 🗳 labels Jul 20, 2021
@jkleinsc jkleinsc force-pushed the enable-webhid branch 2 times, most recently from 1ae06ff to 32ac5c0 Compare July 21, 2021 14:44
docs/api/session.md Outdated Show resolved Hide resolved
docs/api/session.md Show resolved Hide resolved
docs/api/session.md Outdated Show resolved Hide resolved
docs/api/session.md Outdated Show resolved Hide resolved
shell/browser/electron_browser_context.cc Outdated Show resolved Hide resolved
shell/browser/hid/electron_hid_delegate.cc Show resolved Hide resolved
shell/browser/hid/electron_hid_delegate.cc Show resolved Hide resolved
@jkleinsc jkleinsc requested a review from nornagon July 26, 2021 18:54
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Jul 27, 2021
@jkleinsc jkleinsc force-pushed the enable-webhid branch 5 times, most recently from bb0498a to 8c77308 Compare August 16, 2021 14:45
shell/browser/hid/hid_chooser_controller.cc Show resolved Hide resolved
docs/api/session.md Outdated Show resolved Hide resolved
docs/api/session.md Outdated Show resolved Hide resolved
docs/api/session.md Outdated Show resolved Hide resolved
docs/api/session.md Outdated Show resolved Hide resolved
docs/api/session.md Show resolved Hide resolved
shell/browser/hid/hid_chooser_controller.h Show resolved Hide resolved
Copy link
Member

@nornagon nornagon left a comment

Choose a reason for hiding this comment

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

I wonder if we can potentially reduce the code volume here by simplifying our impl with respect to upstream. Since it seems we're quite far from the original impl, I'm not sure all the structures here are still useful...

docs/api/session.md Outdated Show resolved Hide resolved
docs/api/session.md Outdated Show resolved Hide resolved
lib/browser/api/web-contents.ts Outdated Show resolved Hide resolved
lib/browser/api/web-contents.ts Outdated Show resolved Hide resolved
docs/api/session.md Outdated Show resolved Hide resolved
shell/browser/hid/hid_chooser_context.h Outdated Show resolved Hide resolved
shell/browser/hid/hid_chooser_controller.cc Outdated Show resolved Hide resolved
shell/browser/hid/hid_chooser_controller.cc Outdated Show resolved Hide resolved
shell/browser/hid/hid_chooser_controller.cc Outdated Show resolved Hide resolved
spec-main/chromium-spec.ts Show resolved Hide resolved
@nornagon
Copy link
Member

re: PrimaryPageChanged, we do want to clear device permissions when navigation happens. Can we have a test for that perhaps?

docs/api/session.md Outdated Show resolved Hide resolved
docs/api/session.md Outdated Show resolved Hide resolved
docs/api/session.md Show resolved Hide resolved
docs/api/session.md Outdated Show resolved Hide resolved
docs/api/session.md Show resolved Hide resolved
shell/browser/hid/hid_chooser_controller.cc Show resolved Hide resolved
shell/browser/hid/hid_chooser_controller.cc Outdated Show resolved Hide resolved
shell/browser/hid/hid_chooser_controller.cc Outdated Show resolved Hide resolved
shell/browser/electron_permission_manager.h Outdated Show resolved Hide resolved
shell/browser/api/electron_api_web_contents.cc Outdated Show resolved Hide resolved
trop bot pushed a commit that referenced this pull request Sep 23, 2021
* feat: add support for WebHID

* Apply suggestions from code review

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

* Address review feedback

* Address review feedback

* chore: clear granted_devices on navigation

Also added test to verify devices get cleared

* fixup testing for device clear

* make sure navigator.hid.getDevices is run on correct frame

* clear granted devices on RenderFrameHost deletion/change

* manage device permissions per RenderFrameHost

This change makes sure we don't clear device permission prematurely due to child frame navigation

* Update shell/browser/api/electron_api_web_contents.cc

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

* apply review feedback from @zcbenz

* Match upstream ObjectMap

This change matches what ObjectPermissionContextBase uses to cache object permissions: https://source.chromium.org/chromium/chromium/src/+/main:components/permissions/object_permission_context_base.h;l=52;drc=8f95b5eab2797a3e26bba299f3b0df85bfc98bf5;bpv=1;bpt=0

The main reason for this was to resolve this crash on Win x64:
ok 2 WebContentsView doesn't crash when GCed during allocation
Received fatal exception EXCEPTION_ACCESS_VIOLATION
Backtrace:
        gin::WrappableBase::SecondWeakCallback [0x00007FF6F2AFA005+133] (o:\gin\wrappable.cc:53)
        v8::internal::GlobalHandles::InvokeSecondPassPhantomCallbacks [0x00007FF6F028F9AB+171] (o:\v8\src\handles\global-handles.cc:1400)
        v8::internal::GlobalHandles::InvokeSecondPassPhantomCallbacksFromTask [0x00007FF6F028F867+391] (o:\v8\src\handles\global-handles.cc:1387)
        node::PerIsolatePlatformData::RunForegroundTask [0x00007FF6F3B4D065+317] (o:\third_party\electron_node\src\node_platform.cc:415)
        node::PerIsolatePlatformData::FlushForegroundTasksInternal [0x00007FF6F3B4C424+776] (o:\third_party\electron_node\src\node_platform.cc:479)
        uv_run [0x00007FF6F2DDD07C+492] (o:\third_party\electron_node\deps\uv\src\win\core.c:609)
        electron::NodeBindings::UvRunOnce [0x00007FF6EEE1E036+294] (o:\electron\shell\common\node_bindings.cc:631)
        base::TaskAnnotator::RunTask [0x00007FF6F2318A19+457] (o:\base\task\common\task_annotator.cc:178)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl [0x00007FF6F2E6F553+963] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:361)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork [0x00007FF6F2E6EC69+137] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:266)
        base::MessagePumpForUI::DoRunLoop [0x00007FF6F235AA58+216] (o:\base\message_loop\message_pump_win.cc:221)
        base::MessagePumpWin::Run [0x00007FF6F235A01A+106] (o:\base\message_loop\message_pump_win.cc:79)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run [0x00007FF6F2E702DA+682] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:470)
        base::RunLoop::Run [0x00007FF6F22F95BA+842] (o:\base\run_loop.cc:136)
        content::BrowserMainLoop::RunMainMessageLoop [0x00007FF6F14423CC+208] (o:\content\browser\browser_main_loop.cc:990)
        content::BrowserMainRunnerImpl::Run [0x00007FF6F144402F+143] (o:\content\browser\browser_main_runner_impl.cc:153)
        content::BrowserMain [0x00007FF6F143F911+257] (o:\content\browser\browser_main.cc:49)
        content::RunBrowserProcessMain [0x00007FF6EFFA7D18+112] (o:\content\app\content_main_runner_impl.cc:608)
        content::ContentMainRunnerImpl::RunBrowser [0x00007FF6EFFA8CF4+1220] (o:\content\app\content_main_runner_impl.cc:1104)
        content::ContentMainRunnerImpl::Run [0x00007FF6EFFA87C9+393] (o:\content\app\content_main_runner_impl.cc:971)
        content::RunContentProcess [0x00007FF6EFFA73BD+733] (o:\content\app\content_main.cc:394)
        content::ContentMain [0x00007FF6EFFA79E1+54] (o:\content\app\content_main.cc:422)
        wWinMain [0x00007FF6EECA1535+889] (o:\electron\shell\app\electron_main.cc:291)
        __scrt_common_main_seh [0x00007FF6F6F88482+262] (d:\A01\_work\6\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
        BaseThreadInitThunk [0x00007FFEC0087034+20]
        RtlUserThreadStart [0x00007FFEC1F02651+33]
✗ Electron tests failed with code 0xc0000005.

Co-authored-by: Jeremy Rose <jeremya@chromium.org>
@trop
Copy link
Contributor

trop bot commented Sep 23, 2021

I was unable to backport this PR to "15-x-y" cleanly;
you will need to perform this backport manually.

@trop
Copy link
Contributor

trop bot commented Sep 23, 2021

I have automatically backported this PR to "16-x-y", please check out #31090

jkleinsc added a commit that referenced this pull request Sep 23, 2021
* feat: add support for WebHID

* Apply suggestions from code review

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

* Address review feedback

* Address review feedback

* chore: clear granted_devices on navigation

Also added test to verify devices get cleared

* fixup testing for device clear

* make sure navigator.hid.getDevices is run on correct frame

* clear granted devices on RenderFrameHost deletion/change

* manage device permissions per RenderFrameHost

This change makes sure we don't clear device permission prematurely due to child frame navigation

* Update shell/browser/api/electron_api_web_contents.cc

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

* apply review feedback from @zcbenz

* Match upstream ObjectMap

This change matches what ObjectPermissionContextBase uses to cache object permissions: https://source.chromium.org/chromium/chromium/src/+/main:components/permissions/object_permission_context_base.h;l=52;drc=8f95b5eab2797a3e26bba299f3b0df85bfc98bf5;bpv=1;bpt=0

The main reason for this was to resolve this crash on Win x64:
ok 2 WebContentsView doesn't crash when GCed during allocation
Received fatal exception EXCEPTION_ACCESS_VIOLATION
Backtrace:
        gin::WrappableBase::SecondWeakCallback [0x00007FF6F2AFA005+133] (o:\gin\wrappable.cc:53)
        v8::internal::GlobalHandles::InvokeSecondPassPhantomCallbacks [0x00007FF6F028F9AB+171] (o:\v8\src\handles\global-handles.cc:1400)
        v8::internal::GlobalHandles::InvokeSecondPassPhantomCallbacksFromTask [0x00007FF6F028F867+391] (o:\v8\src\handles\global-handles.cc:1387)
        node::PerIsolatePlatformData::RunForegroundTask [0x00007FF6F3B4D065+317] (o:\third_party\electron_node\src\node_platform.cc:415)
        node::PerIsolatePlatformData::FlushForegroundTasksInternal [0x00007FF6F3B4C424+776] (o:\third_party\electron_node\src\node_platform.cc:479)
        uv_run [0x00007FF6F2DDD07C+492] (o:\third_party\electron_node\deps\uv\src\win\core.c:609)
        electron::NodeBindings::UvRunOnce [0x00007FF6EEE1E036+294] (o:\electron\shell\common\node_bindings.cc:631)
        base::TaskAnnotator::RunTask [0x00007FF6F2318A19+457] (o:\base\task\common\task_annotator.cc:178)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl [0x00007FF6F2E6F553+963] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:361)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork [0x00007FF6F2E6EC69+137] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:266)
        base::MessagePumpForUI::DoRunLoop [0x00007FF6F235AA58+216] (o:\base\message_loop\message_pump_win.cc:221)
        base::MessagePumpWin::Run [0x00007FF6F235A01A+106] (o:\base\message_loop\message_pump_win.cc:79)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run [0x00007FF6F2E702DA+682] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:470)
        base::RunLoop::Run [0x00007FF6F22F95BA+842] (o:\base\run_loop.cc:136)
        content::BrowserMainLoop::RunMainMessageLoop [0x00007FF6F14423CC+208] (o:\content\browser\browser_main_loop.cc:990)
        content::BrowserMainRunnerImpl::Run [0x00007FF6F144402F+143] (o:\content\browser\browser_main_runner_impl.cc:153)
        content::BrowserMain [0x00007FF6F143F911+257] (o:\content\browser\browser_main.cc:49)
        content::RunBrowserProcessMain [0x00007FF6EFFA7D18+112] (o:\content\app\content_main_runner_impl.cc:608)
        content::ContentMainRunnerImpl::RunBrowser [0x00007FF6EFFA8CF4+1220] (o:\content\app\content_main_runner_impl.cc:1104)
        content::ContentMainRunnerImpl::Run [0x00007FF6EFFA87C9+393] (o:\content\app\content_main_runner_impl.cc:971)
        content::RunContentProcess [0x00007FF6EFFA73BD+733] (o:\content\app\content_main.cc:394)
        content::ContentMain [0x00007FF6EFFA79E1+54] (o:\content\app\content_main.cc:422)
        wWinMain [0x00007FF6EECA1535+889] (o:\electron\shell\app\electron_main.cc:291)
        __scrt_common_main_seh [0x00007FF6F6F88482+262] (d:\A01\_work\6\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
        BaseThreadInitThunk [0x00007FFEC0087034+20]
        RtlUserThreadStart [0x00007FFEC1F02651+33]
✗ Electron tests failed with code 0xc0000005.

Co-authored-by: Jeremy Rose <jeremya@chromium.org>
(cherry picked from commit 6aece4a)
@trop
Copy link
Contributor

trop bot commented Sep 23, 2021

@jkleinsc has manually backported this PR to "15-x-y", please check out #31095

jkleinsc added a commit that referenced this pull request Sep 29, 2021
* feat: add support for WebHID

* Apply suggestions from code review

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

* Address review feedback

* Address review feedback

* chore: clear granted_devices on navigation

Also added test to verify devices get cleared

* fixup testing for device clear

* make sure navigator.hid.getDevices is run on correct frame

* clear granted devices on RenderFrameHost deletion/change

* manage device permissions per RenderFrameHost

This change makes sure we don't clear device permission prematurely due to child frame navigation

* Update shell/browser/api/electron_api_web_contents.cc

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

* apply review feedback from @zcbenz

* Match upstream ObjectMap

This change matches what ObjectPermissionContextBase uses to cache object permissions: https://source.chromium.org/chromium/chromium/src/+/main:components/permissions/object_permission_context_base.h;l=52;drc=8f95b5eab2797a3e26bba299f3b0df85bfc98bf5;bpv=1;bpt=0

The main reason for this was to resolve this crash on Win x64:
ok 2 WebContentsView doesn't crash when GCed during allocation
Received fatal exception EXCEPTION_ACCESS_VIOLATION
Backtrace:
        gin::WrappableBase::SecondWeakCallback [0x00007FF6F2AFA005+133] (o:\gin\wrappable.cc:53)
        v8::internal::GlobalHandles::InvokeSecondPassPhantomCallbacks [0x00007FF6F028F9AB+171] (o:\v8\src\handles\global-handles.cc:1400)
        v8::internal::GlobalHandles::InvokeSecondPassPhantomCallbacksFromTask [0x00007FF6F028F867+391] (o:\v8\src\handles\global-handles.cc:1387)
        node::PerIsolatePlatformData::RunForegroundTask [0x00007FF6F3B4D065+317] (o:\third_party\electron_node\src\node_platform.cc:415)
        node::PerIsolatePlatformData::FlushForegroundTasksInternal [0x00007FF6F3B4C424+776] (o:\third_party\electron_node\src\node_platform.cc:479)
        uv_run [0x00007FF6F2DDD07C+492] (o:\third_party\electron_node\deps\uv\src\win\core.c:609)
        electron::NodeBindings::UvRunOnce [0x00007FF6EEE1E036+294] (o:\electron\shell\common\node_bindings.cc:631)
        base::TaskAnnotator::RunTask [0x00007FF6F2318A19+457] (o:\base\task\common\task_annotator.cc:178)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl [0x00007FF6F2E6F553+963] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:361)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork [0x00007FF6F2E6EC69+137] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:266)
        base::MessagePumpForUI::DoRunLoop [0x00007FF6F235AA58+216] (o:\base\message_loop\message_pump_win.cc:221)
        base::MessagePumpWin::Run [0x00007FF6F235A01A+106] (o:\base\message_loop\message_pump_win.cc:79)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run [0x00007FF6F2E702DA+682] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:470)
        base::RunLoop::Run [0x00007FF6F22F95BA+842] (o:\base\run_loop.cc:136)
        content::BrowserMainLoop::RunMainMessageLoop [0x00007FF6F14423CC+208] (o:\content\browser\browser_main_loop.cc:990)
        content::BrowserMainRunnerImpl::Run [0x00007FF6F144402F+143] (o:\content\browser\browser_main_runner_impl.cc:153)
        content::BrowserMain [0x00007FF6F143F911+257] (o:\content\browser\browser_main.cc:49)
        content::RunBrowserProcessMain [0x00007FF6EFFA7D18+112] (o:\content\app\content_main_runner_impl.cc:608)
        content::ContentMainRunnerImpl::RunBrowser [0x00007FF6EFFA8CF4+1220] (o:\content\app\content_main_runner_impl.cc:1104)
        content::ContentMainRunnerImpl::Run [0x00007FF6EFFA87C9+393] (o:\content\app\content_main_runner_impl.cc:971)
        content::RunContentProcess [0x00007FF6EFFA73BD+733] (o:\content\app\content_main.cc:394)
        content::ContentMain [0x00007FF6EFFA79E1+54] (o:\content\app\content_main.cc:422)
        wWinMain [0x00007FF6EECA1535+889] (o:\electron\shell\app\electron_main.cc:291)
        __scrt_common_main_seh [0x00007FF6F6F88482+262] (d:\A01\_work\6\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
        BaseThreadInitThunk [0x00007FFEC0087034+20]
        RtlUserThreadStart [0x00007FFEC1F02651+33]
✗ Electron tests failed with code 0xc0000005.

Co-authored-by: Jeremy Rose <jeremya@chromium.org>
(cherry picked from commit 6aece4a)
jkleinsc added a commit that referenced this pull request Sep 29, 2021
* feat: add support for WebHID

* Apply suggestions from code review

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

* Address review feedback

* Address review feedback

* chore: clear granted_devices on navigation

Also added test to verify devices get cleared

* fixup testing for device clear

* make sure navigator.hid.getDevices is run on correct frame

* clear granted devices on RenderFrameHost deletion/change

* manage device permissions per RenderFrameHost

This change makes sure we don't clear device permission prematurely due to child frame navigation

* Update shell/browser/api/electron_api_web_contents.cc

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

* apply review feedback from @zcbenz

* Match upstream ObjectMap

This change matches what ObjectPermissionContextBase uses to cache object permissions: https://source.chromium.org/chromium/chromium/src/+/main:components/permissions/object_permission_context_base.h;l=52;drc=8f95b5eab2797a3e26bba299f3b0df85bfc98bf5;bpv=1;bpt=0

The main reason for this was to resolve this crash on Win x64:
ok 2 WebContentsView doesn't crash when GCed during allocation
Received fatal exception EXCEPTION_ACCESS_VIOLATION
Backtrace:
        gin::WrappableBase::SecondWeakCallback [0x00007FF6F2AFA005+133] (o:\gin\wrappable.cc:53)
        v8::internal::GlobalHandles::InvokeSecondPassPhantomCallbacks [0x00007FF6F028F9AB+171] (o:\v8\src\handles\global-handles.cc:1400)
        v8::internal::GlobalHandles::InvokeSecondPassPhantomCallbacksFromTask [0x00007FF6F028F867+391] (o:\v8\src\handles\global-handles.cc:1387)
        node::PerIsolatePlatformData::RunForegroundTask [0x00007FF6F3B4D065+317] (o:\third_party\electron_node\src\node_platform.cc:415)
        node::PerIsolatePlatformData::FlushForegroundTasksInternal [0x00007FF6F3B4C424+776] (o:\third_party\electron_node\src\node_platform.cc:479)
        uv_run [0x00007FF6F2DDD07C+492] (o:\third_party\electron_node\deps\uv\src\win\core.c:609)
        electron::NodeBindings::UvRunOnce [0x00007FF6EEE1E036+294] (o:\electron\shell\common\node_bindings.cc:631)
        base::TaskAnnotator::RunTask [0x00007FF6F2318A19+457] (o:\base\task\common\task_annotator.cc:178)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl [0x00007FF6F2E6F553+963] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:361)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork [0x00007FF6F2E6EC69+137] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:266)
        base::MessagePumpForUI::DoRunLoop [0x00007FF6F235AA58+216] (o:\base\message_loop\message_pump_win.cc:221)
        base::MessagePumpWin::Run [0x00007FF6F235A01A+106] (o:\base\message_loop\message_pump_win.cc:79)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run [0x00007FF6F2E702DA+682] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:470)
        base::RunLoop::Run [0x00007FF6F22F95BA+842] (o:\base\run_loop.cc:136)
        content::BrowserMainLoop::RunMainMessageLoop [0x00007FF6F14423CC+208] (o:\content\browser\browser_main_loop.cc:990)
        content::BrowserMainRunnerImpl::Run [0x00007FF6F144402F+143] (o:\content\browser\browser_main_runner_impl.cc:153)
        content::BrowserMain [0x00007FF6F143F911+257] (o:\content\browser\browser_main.cc:49)
        content::RunBrowserProcessMain [0x00007FF6EFFA7D18+112] (o:\content\app\content_main_runner_impl.cc:608)
        content::ContentMainRunnerImpl::RunBrowser [0x00007FF6EFFA8CF4+1220] (o:\content\app\content_main_runner_impl.cc:1104)
        content::ContentMainRunnerImpl::Run [0x00007FF6EFFA87C9+393] (o:\content\app\content_main_runner_impl.cc:971)
        content::RunContentProcess [0x00007FF6EFFA73BD+733] (o:\content\app\content_main.cc:394)
        content::ContentMain [0x00007FF6EFFA79E1+54] (o:\content\app\content_main.cc:422)
        wWinMain [0x00007FF6EECA1535+889] (o:\electron\shell\app\electron_main.cc:291)
        __scrt_common_main_seh [0x00007FF6F6F88482+262] (d:\A01\_work\6\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
        BaseThreadInitThunk [0x00007FFEC0087034+20]
        RtlUserThreadStart [0x00007FFEC1F02651+33]
✗ Electron tests failed with code 0xc0000005.

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
Co-authored-by: Jeremy Rose <jeremya@chromium.org>
@trop trop bot removed the in-flight/16-x-y label Sep 29, 2021
@trop trop bot mentioned this pull request Sep 29, 2021
trop bot pushed a commit that referenced this pull request Sep 29, 2021
* feat: add support for WebHID

* Apply suggestions from code review

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

* Address review feedback

* Address review feedback

* chore: clear granted_devices on navigation

Also added test to verify devices get cleared

* fixup testing for device clear

* make sure navigator.hid.getDevices is run on correct frame

* clear granted devices on RenderFrameHost deletion/change

* manage device permissions per RenderFrameHost

This change makes sure we don't clear device permission prematurely due to child frame navigation

* Update shell/browser/api/electron_api_web_contents.cc

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

* apply review feedback from @zcbenz

* Match upstream ObjectMap

This change matches what ObjectPermissionContextBase uses to cache object permissions: https://source.chromium.org/chromium/chromium/src/+/main:components/permissions/object_permission_context_base.h;l=52;drc=8f95b5eab2797a3e26bba299f3b0df85bfc98bf5;bpv=1;bpt=0

The main reason for this was to resolve this crash on Win x64:
ok 2 WebContentsView doesn't crash when GCed during allocation
Received fatal exception EXCEPTION_ACCESS_VIOLATION
Backtrace:
        gin::WrappableBase::SecondWeakCallback [0x00007FF6F2AFA005+133] (o:\gin\wrappable.cc:53)
        v8::internal::GlobalHandles::InvokeSecondPassPhantomCallbacks [0x00007FF6F028F9AB+171] (o:\v8\src\handles\global-handles.cc:1400)
        v8::internal::GlobalHandles::InvokeSecondPassPhantomCallbacksFromTask [0x00007FF6F028F867+391] (o:\v8\src\handles\global-handles.cc:1387)
        node::PerIsolatePlatformData::RunForegroundTask [0x00007FF6F3B4D065+317] (o:\third_party\electron_node\src\node_platform.cc:415)
        node::PerIsolatePlatformData::FlushForegroundTasksInternal [0x00007FF6F3B4C424+776] (o:\third_party\electron_node\src\node_platform.cc:479)
        uv_run [0x00007FF6F2DDD07C+492] (o:\third_party\electron_node\deps\uv\src\win\core.c:609)
        electron::NodeBindings::UvRunOnce [0x00007FF6EEE1E036+294] (o:\electron\shell\common\node_bindings.cc:631)
        base::TaskAnnotator::RunTask [0x00007FF6F2318A19+457] (o:\base\task\common\task_annotator.cc:178)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl [0x00007FF6F2E6F553+963] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:361)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork [0x00007FF6F2E6EC69+137] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:266)
        base::MessagePumpForUI::DoRunLoop [0x00007FF6F235AA58+216] (o:\base\message_loop\message_pump_win.cc:221)
        base::MessagePumpWin::Run [0x00007FF6F235A01A+106] (o:\base\message_loop\message_pump_win.cc:79)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run [0x00007FF6F2E702DA+682] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:470)
        base::RunLoop::Run [0x00007FF6F22F95BA+842] (o:\base\run_loop.cc:136)
        content::BrowserMainLoop::RunMainMessageLoop [0x00007FF6F14423CC+208] (o:\content\browser\browser_main_loop.cc:990)
        content::BrowserMainRunnerImpl::Run [0x00007FF6F144402F+143] (o:\content\browser\browser_main_runner_impl.cc:153)
        content::BrowserMain [0x00007FF6F143F911+257] (o:\content\browser\browser_main.cc:49)
        content::RunBrowserProcessMain [0x00007FF6EFFA7D18+112] (o:\content\app\content_main_runner_impl.cc:608)
        content::ContentMainRunnerImpl::RunBrowser [0x00007FF6EFFA8CF4+1220] (o:\content\app\content_main_runner_impl.cc:1104)
        content::ContentMainRunnerImpl::Run [0x00007FF6EFFA87C9+393] (o:\content\app\content_main_runner_impl.cc:971)
        content::RunContentProcess [0x00007FF6EFFA73BD+733] (o:\content\app\content_main.cc:394)
        content::ContentMain [0x00007FF6EFFA79E1+54] (o:\content\app\content_main.cc:422)
        wWinMain [0x00007FF6EECA1535+889] (o:\electron\shell\app\electron_main.cc:291)
        __scrt_common_main_seh [0x00007FF6F6F88482+262] (d:\A01\_work\6\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
        BaseThreadInitThunk [0x00007FFEC0087034+20]
        RtlUserThreadStart [0x00007FFEC1F02651+33]
✗ Electron tests failed with code 0xc0000005.

Co-authored-by: Jeremy Rose <jeremya@chromium.org>
(cherry picked from commit 6aece4a)
jkleinsc added a commit that referenced this pull request Sep 30, 2021
* feat: add support for WebHID (#30213)

* feat: add support for WebHID

* Apply suggestions from code review

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

* Address review feedback

* Address review feedback

* chore: clear granted_devices on navigation

Also added test to verify devices get cleared

* fixup testing for device clear

* make sure navigator.hid.getDevices is run on correct frame

* clear granted devices on RenderFrameHost deletion/change

* manage device permissions per RenderFrameHost

This change makes sure we don't clear device permission prematurely due to child frame navigation

* Update shell/browser/api/electron_api_web_contents.cc

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

* apply review feedback from @zcbenz

* Match upstream ObjectMap

This change matches what ObjectPermissionContextBase uses to cache object permissions: https://source.chromium.org/chromium/chromium/src/+/main:components/permissions/object_permission_context_base.h;l=52;drc=8f95b5eab2797a3e26bba299f3b0df85bfc98bf5;bpv=1;bpt=0

The main reason for this was to resolve this crash on Win x64:
ok 2 WebContentsView doesn't crash when GCed during allocation
Received fatal exception EXCEPTION_ACCESS_VIOLATION
Backtrace:
        gin::WrappableBase::SecondWeakCallback [0x00007FF6F2AFA005+133] (o:\gin\wrappable.cc:53)
        v8::internal::GlobalHandles::InvokeSecondPassPhantomCallbacks [0x00007FF6F028F9AB+171] (o:\v8\src\handles\global-handles.cc:1400)
        v8::internal::GlobalHandles::InvokeSecondPassPhantomCallbacksFromTask [0x00007FF6F028F867+391] (o:\v8\src\handles\global-handles.cc:1387)
        node::PerIsolatePlatformData::RunForegroundTask [0x00007FF6F3B4D065+317] (o:\third_party\electron_node\src\node_platform.cc:415)
        node::PerIsolatePlatformData::FlushForegroundTasksInternal [0x00007FF6F3B4C424+776] (o:\third_party\electron_node\src\node_platform.cc:479)
        uv_run [0x00007FF6F2DDD07C+492] (o:\third_party\electron_node\deps\uv\src\win\core.c:609)
        electron::NodeBindings::UvRunOnce [0x00007FF6EEE1E036+294] (o:\electron\shell\common\node_bindings.cc:631)
        base::TaskAnnotator::RunTask [0x00007FF6F2318A19+457] (o:\base\task\common\task_annotator.cc:178)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl [0x00007FF6F2E6F553+963] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:361)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork [0x00007FF6F2E6EC69+137] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:266)
        base::MessagePumpForUI::DoRunLoop [0x00007FF6F235AA58+216] (o:\base\message_loop\message_pump_win.cc:221)
        base::MessagePumpWin::Run [0x00007FF6F235A01A+106] (o:\base\message_loop\message_pump_win.cc:79)
        base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run [0x00007FF6F2E702DA+682] (o:\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:470)
        base::RunLoop::Run [0x00007FF6F22F95BA+842] (o:\base\run_loop.cc:136)
        content::BrowserMainLoop::RunMainMessageLoop [0x00007FF6F14423CC+208] (o:\content\browser\browser_main_loop.cc:990)
        content::BrowserMainRunnerImpl::Run [0x00007FF6F144402F+143] (o:\content\browser\browser_main_runner_impl.cc:153)
        content::BrowserMain [0x00007FF6F143F911+257] (o:\content\browser\browser_main.cc:49)
        content::RunBrowserProcessMain [0x00007FF6EFFA7D18+112] (o:\content\app\content_main_runner_impl.cc:608)
        content::ContentMainRunnerImpl::RunBrowser [0x00007FF6EFFA8CF4+1220] (o:\content\app\content_main_runner_impl.cc:1104)
        content::ContentMainRunnerImpl::Run [0x00007FF6EFFA87C9+393] (o:\content\app\content_main_runner_impl.cc:971)
        content::RunContentProcess [0x00007FF6EFFA73BD+733] (o:\content\app\content_main.cc:394)
        content::ContentMain [0x00007FF6EFFA79E1+54] (o:\content\app\content_main.cc:422)
        wWinMain [0x00007FF6EECA1535+889] (o:\electron\shell\app\electron_main.cc:291)
        __scrt_common_main_seh [0x00007FF6F6F88482+262] (d:\A01\_work\6\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
        BaseThreadInitThunk [0x00007FFEC0087034+20]
        RtlUserThreadStart [0x00007FFEC1F02651+33]
✗ Electron tests failed with code 0xc0000005.

Co-authored-by: Jeremy Rose <jeremya@chromium.org>
(cherry picked from commit 6aece4a)

* fixup for 14-x-y

Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
@trop trop bot removed the in-flight/14-x-y label Sep 30, 2021
@ghost ghost mentioned this pull request Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-review/requested 🗳 semver/minor backwards-compatible functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WebHID API support
5 participants