Bug 175184

Created attachment 317435 [details] Patch

Attachment 317435 [details] did not pass style-queue: ERROR: Source/WebDriver/Session.h:106: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebDriver/Session.h:107: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebDriver/Session.cpp:146: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebDriver/Session.cpp:197: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 4 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 317435 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=317435&action=review r=me > Source/WebDriver/ChangeLog:16 > + as part fo the result error message. Nit: of > Source/WebDriver/ChangeLog:19 > + (WebDriver::Session::handleUserPrompts): Check if therre's an active JavaScript dialog and deal with it depeding Nit: there's > Source/WebDriver/Session.cpp:167 > + bool shouldFailOnDismiss = false; I think it would be cleaner here to use a std::optional<UnhandledPromptBehavior>. The !value() case can go before the switch and you can move the most complicated part of the switch case (the if branch) out of the switch entirely. > Source/WebDriver/Session.cpp:210 > + errorResult.setAdditonalErrorData(WTFMove(additonalData)); Nit: setAdditionalErrorData > Source/WebDriver/Session.cpp:223 > + handleUserPrompts([this, url, completionHandler = WTFMove(completionHandler)](CommandResult&& result) mutable { This significantly indents most of the session code. Is there a way we can extract it into a separate method to cut down on async indent noise? (this situation always makes me miss JS promises..)

(In reply to Brian Burg from comment #3) > Comment on attachment 317435 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=317435&action=review > > r=me > > > Source/WebDriver/ChangeLog:16 > > + as part fo the result error message. > > Nit: of > > > Source/WebDriver/ChangeLog:19 > > + (WebDriver::Session::handleUserPrompts): Check if therre's an active JavaScript dialog and deal with it depeding > > Nit: there's > > > Source/WebDriver/Session.cpp:167 > > + bool shouldFailOnDismiss = false; > > I think it would be cleaner here to use a > std::optional<UnhandledPromptBehavior>. The !value() case can go before the > switch and you can move the most complicated part of the switch case (the if > branch) out of the switch entirely. > > > Source/WebDriver/Session.cpp:210 > > + errorResult.setAdditonalErrorData(WTFMove(additonalData)); > > Nit: setAdditionalErrorData Oops. > > Source/WebDriver/Session.cpp:223 > > + handleUserPrompts([this, url, completionHandler = WTFMove(completionHandler)](CommandResult&& result) mutable { > > This significantly indents most of the session code. Is there a way we can > extract it into a separate method to cut down on async indent noise? (this > situation always makes me miss JS promises..) That's the problem of lambdas :-/ things are even worse when we end up nesting several lambdas. Fortunately, I think waiting for navigation and handling prompts are the only global things we have to do before running commands, so this is the last time we have to reindent.

Committed r220388: <http://trac.webkit.org/changeset/220388>

<rdar://problem/33770251>