Bug 177405

Summary: Web Automation: add commands to get and set mock user permissions for pages in an automation session
Product: WebKit Reporter: BJ Burg <bburg>
Component: WebDriverAssignee: BJ Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, cgarcia, commit-queue, joepeck, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

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.