[Webkit-unassigned] [Bug 166682] Add initial implementation of WebDriver process to run the HTTP server

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=166682

--- Comment #31 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 315417
  --> https://bugs.webkit.org/attachment.cgi?id=315417
Patch

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

>>> Source/WebDriver/WebDriverService.cpp:334
>>> +    if (!desiredCapabilities.getString(ASCIILiteral("browserName"), capabilities.browserName)) {
>> 
>> I found it weird, and probably wrong, that this requires these three capabilities to be specified by the REST API client in order to do anything. Typically no capabilities are required by other drivers.
>> 
>> The semantics of capabilities are a bit strange, especially now in W3C spec, but basically an endpoint should ignore capabilities it does not understand or that don't control anything. If the server can interpret the capability and do something with it, then it is possible for it to reject a capability value as an invalid argument. Since these capabilities aren't being used to match a particular browser, it seems like a mistake to make them required. If you start rejecting the session when these capabilities don't match the current system, then it makes more sense to validate them here or elsewhere.
> 
> The idea is that webkitgtk:browserOptions is optional, but if present it should contain all the webkitgtk specific capabilities, so webkitgtk based drivers should include those. I'll read again the capabilities mess in the spec in any case.

I've read the spec again and now I remember why I didn't pay much attention to capabilities, not only because it's indeed a mess, but because selenium doesn't seem to use the returned capabilities at all, they are just stored in the driver. There aren't tests checking capabilities either.

>> Source/WebDriver/WebDriverService.cpp:398
>> +            capabilities->setString(ASCIILiteral("browserName"), session->capabilities().browserName);
> 
> I think this is where you would use the resolved capabilities (from requested capabilities), and also populate the capabilities from the session host directly, as it can query these platform things.

These are supposed to be the resolved (or merged) capabilities, the idea is that the session host receives the desired capabilities, but changes them to the actual ones. It just happens that for now they are the same ones, since I'm only using the capabilities as a way to pass the browser binary path and arguments. What do you mean by populate the capabilities from the host? I understood that you preferred that session (and session host I guess) don't have to deal with the inspector objects, that's why I added this here. I added Session::capabilities  as a wrapper of SessionHost::capabilities just for convenience.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170717/d909748c/attachment.html>