Bug 177405 - Web Automation: add commands to get and set mock user permissions for pages in an automation session
Summary: Web Automation: add commands to get and set mock user permissions for pages i...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebDriver (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: BJ Burg
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-09-22 23:05 PDT by BJ Burg
Modified: 2017-09-26 10:12 PDT (History)
5 users (show)

See Also:


Attachments
Patch (11.20 KB, patch)
2017-09-22 23:25 PDT, BJ Burg
no flags Details | Formatted Diff | Diff
Patch (11.27 KB, patch)
2017-09-26 09:01 PDT, BJ Burg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2017-09-22 23:05:18 PDT
This patch also implements the first permission, getUserMedia.
Comment 1 BJ Burg 2017-09-22 23:19:15 PDT
<rdar://problem/34493846>
Comment 2 Radar WebKit Bug Importer 2017-09-22 23:19:44 PDT
<rdar://problem/34609571>
Comment 3 BJ Burg 2017-09-22 23:25:34 PDT
Created attachment 321617 [details]
Patch
Comment 4 Carlos Garcia Campos 2017-09-22 23:32:04 PDT
Comment on attachment 321617 [details]
Patch

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

> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:1259
> +        case Inspector::Protocol::Automation::MockPermission::GetUserMedia:
> +            m_mockPermissionForGetUserMedia = permissionValue;
> +        }

I would add a break here, even if there aren't more options yet.
Comment 5 Joseph Pecoraro 2017-09-25 16:45:08 PDT
Comment on attachment 321617 [details]
Patch

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

r=me. You might need a WK2 Owner to approve the UserMediaPermissionRequestManagerProxy change, but the rest looks good to me.

> Source/WebKit/UIProcess/Automation/Automation.json:222
> +            "id": "MockPermission",

I'm not loving the Mock part of this name. Effectively you are setting the permission for the session, so I don't think Mock is necessary. If the intent is to portray that this is unique from the real system / user permissions then replacing Mock with Session would be event clearer. `getMockPermissions` => `getSessionPermissions`. Whether or not Mock stays in the commands below, it doesn't need to be included in these type names.

>> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:1259
>> +        }
> 
> I would add a break here, even if there aren't more options yet.

+1. I'm surprised this didn't get a compiler warning. Maybe we haven't enabled -Wimplicit-fallthrough in all files? I think C++ is supposed to have it thought.

> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:1268
> +

Nit: Extra newline.
Comment 6 BJ Burg 2017-09-26 09:01:00 PDT
Created attachment 321821 [details]
Patch
Comment 7 BJ Burg 2017-09-26 09:12:22 PDT
<rdar://problem/34493846>
Comment 8 WebKit Commit Bot 2017-09-26 10:12:25 PDT
Comment on attachment 321821 [details]
Patch

Clearing flags on attachment: 321821

Committed r222503: <http://trac.webkit.org/changeset/222503>
Comment 9 WebKit Commit Bot 2017-09-26 10:12:26 PDT
All reviewed patches have been landed.  Closing bug.