10.2 Close Window https://w3c.github.io/webdriver/webdriver-spec.html#close-window Return the result of running the remote end steps for the Get Window Handles command. We are currently returning null. We should also try to close the session if there aren't more browsing contexts.
Created attachment 316445 [details] Patch
Attachment 316445 [details] did not pass style-queue: ERROR: Source/WebDriver/Session.h:110: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebDriver/Session.h:111: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebDriver/Session.h:112: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebDriver/Session.cpp:60: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebDriver/Session.cpp:308: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebDriver/Session.cpp:336: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 6 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 316445 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316445&action=review Absent a good justification, I don't want to add a redundant command. The web service parts look good. > Source/WebDriver/Session.cpp:308 > +void Session::continueGetWindowHandles(const InspectorObject& result, Function<void (CommandResult&&)>&& completionHandler) Nit: I prefer "fetchWindowHandles" or just "getWindowHandles". > Source/WebKit/UIProcess/Automation/Automation.json:250 > + "returns": [ I don't like this. It's redundant with another command. Can you just compose the two Automation commands in the service implementation? That's what safaridriver does.
(In reply to Brian Burg from comment #3) > Comment on attachment 316445 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=316445&action=review > > Absent a good justification, I don't want to add a redundant command. The > web service parts look good. > > > Source/WebDriver/Session.cpp:308 > > +void Session::continueGetWindowHandles(const InspectorObject& result, Function<void (CommandResult&&)>&& completionHandler) > > Nit: I prefer "fetchWindowHandles" or just "getWindowHandles". There's already getWindowHandles, this is what we call when we get the response of getWindowHandles. > > Source/WebKit/UIProcess/Automation/Automation.json:250 > > + "returns": [ > > I don't like this. It's redundant with another command. Can you just compose > the two Automation commands in the service implementation? That's what > safaridriver does. Yes, I just thought it was more convenient, and it simplifies the implementation in the driver.
Created attachment 316630 [details] Patch
Attachment 316630 [details] did not pass style-queue: ERROR: Source/WebDriver/Session.h:110: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebDriver/Session.h:111: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebDriver/Session.cpp:60: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebDriver/Session.cpp:308: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 4 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 316630 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316630&action=review r=me with naming change > Source/WebDriver/ChangeLog:24 > + (WebDriver::Session::closeWindow): Call closeTopLevelBrowsingContext() passing the current toplvel browsing context. Nit: toplvel > Source/WebDriver/Session.cpp:60 > +void Session::closeNextToplevelBrowsingContext(const String& toplevelBrowsingContext, Function<void (CommandResult&&)>&& completionHandler) Shouldn't this entry point be closeAllToplevelBrowsingContexts? It closes more than just the next one.
Committed r220794: <http://trac.webkit.org/changeset/220794>
<rdar://problem/33916648>