Bug 217559 - [WebAuthn] Implement a dummy WebAuthnProcess
Summary: [WebAuthn] Implement a dummy WebAuthnProcess
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jiewen Tan
URL:
Keywords: InRadar
Depends on:
Blocks: 181943
  Show dependency treegraph
 
Reported: 2020-10-10 02:32 PDT by Jiewen Tan
Modified: 2020-10-16 12:38 PDT (History)
14 users (show)

See Also:


Attachments
Patch (11.55 KB, patch)
2020-10-10 03:12 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (173.56 KB, patch)
2020-10-14 15:25 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (174.21 KB, patch)
2020-10-14 19:45 PDT, Jiewen Tan
bfulgham: review+
Details | Formatted Diff | Diff
Patch for landing (174.23 KB, patch)
2020-10-16 11:45 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jiewen Tan 2020-10-10 02:32:26 PDT
Implement a dummy WebAuthenticationAgent Part 2.
Comment 1 Radar WebKit Bug Importer 2020-10-10 02:32:57 PDT
<rdar://problem/70168749>
Comment 2 Jiewen Tan 2020-10-10 03:12:10 PDT
Created attachment 411008 [details]
Patch
Comment 3 Jiewen Tan 2020-10-14 15:25:33 PDT
Created attachment 411378 [details]
Patch
Comment 4 Jiewen Tan 2020-10-14 19:45:30 PDT
Created attachment 411396 [details]
Patch
Comment 5 Brent Fulgham 2020-10-16 10:02:48 PDT
Comment on attachment 411396 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411396&action=review

You should have Chris Dumez take a look at the process assertion bits, but this seems to be correct as far as I can tell. You should also file a bug to add relevant entitlements to the new service once you start moving calls to USB/NFC things into this new code path.

> Source/WebKit/ChangeLog:25
> +        This patch therefore focues on making the WebAuthn process happen and removes the WebAuthn daemon.

focus

> Source/WebKit/ChangeLog:45
> +        Paperwork for introudcing the new WebAuthn process. Mostly copied from GPU process.

introducing

> Source/WebKit/ChangeLog:125
> +        Paperwork for introudcing the new WebAuthn process. Mostly copied from GPU process.

introducing

> Source/WebKit/ChangeLog:128
> +        The sandbox profile is originally from the GPU Process with IOKit related rules removed. Will tighten it again after the process is functional properly.

"process is fully functional"

> Source/WebKit/ChangeLog:153
> +        Paperwork for introudcing the new WebAuthn process. Mostly copied from GPU process.

introducing

> Source/WebCore/en.lproj/Localizable.strings:49
> +/* visible name of the GPU process. The argument is the application name. */

This isn't the GPU Process. This is the Web Authentication process!

> Source/WebKit/UIProcess/Launcher/mac/ProcessLauncherMac.mm:81
> +    // On Mac, leak a boost onto the NetworkProcess, GPUProcess and WebAuthnProcess.

Please use the "Oxford Comma":  "NetworkProcess, GPUProcess, and WebAuthnProcess."
Comment 6 Jiewen Tan 2020-10-16 11:44:11 PDT
Comment on attachment 411396 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411396&action=review

Thanks Brent for r+ this patch.

>> Source/WebKit/ChangeLog:25
>> +        This patch therefore focues on making the WebAuthn process happen and removes the WebAuthn daemon.
> 
> focus

Fixed.

>> Source/WebKit/ChangeLog:45
>> +        Paperwork for introudcing the new WebAuthn process. Mostly copied from GPU process.
> 
> introducing

Fixed.

>> Source/WebKit/ChangeLog:125
>> +        Paperwork for introudcing the new WebAuthn process. Mostly copied from GPU process.
> 
> introducing

Fixed.

>> Source/WebKit/ChangeLog:128
>> +        The sandbox profile is originally from the GPU Process with IOKit related rules removed. Will tighten it again after the process is functional properly.
> 
> "process is fully functional"

Fixed.

>> Source/WebKit/ChangeLog:153
>> +        Paperwork for introudcing the new WebAuthn process. Mostly copied from GPU process.
> 
> introducing

Fixed.

>> Source/WebCore/en.lproj/Localizable.strings:49
>> +/* visible name of the GPU process. The argument is the application name. */
> 
> This isn't the GPU Process. This is the Web Authentication process!

Oops. Fixed.

>> Source/WebKit/UIProcess/Launcher/mac/ProcessLauncherMac.mm:81
>> +    // On Mac, leak a boost onto the NetworkProcess, GPUProcess and WebAuthnProcess.
> 
> Please use the "Oxford Comma":  "NetworkProcess, GPUProcess, and WebAuthnProcess."

Fixed.
Comment 7 Jiewen Tan 2020-10-16 11:45:38 PDT
Created attachment 411602 [details]
Patch for landing
Comment 8 EWS 2020-10-16 12:38:53 PDT
Committed r268605: <https://trac.webkit.org/changeset/268605>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 411602 [details].