[webkit-reviews] review granted: [Bug 174797] Web Automation: add support for uploading files : [Attachment 316320] Patch

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has granted Brian Burg <bburg at apple.com>'s
request for review:
Bug 174797: Web Automation: add support for uploading files
https://bugs.webkit.org/show_bug.cgi?id=174797

Attachment 316320: Patch

https://bugs.webkit.org/attachment.cgi?id=316320&action=review




--- Comment #3 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 316320
  --> https://bugs.webkit.org/attachment.cgi?id=316320
Patch

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

r=me

> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:507
> +    // Per §14.3.10.5 in the W3C spec, if at least one file no longer
exists, the command should fail.

"no longer exists" => "does not exist"

> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:510
> +	   if (!WebCore::fileExists(filename)) {

Did you also want to check that the file is readable by this process?

> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:517
> +    // FIXME: validate filenames against allowed MIME types before choosing
them.

Bugzilla bug before landing.

> Source/WebKit/UIProcess/WebPageProxy.cpp:4125
> +    if (m_controlledByAutomation) {
> +	   if (auto* automationSession =
process().processPool().automationSession()) {
> +	       automationSession->handleRunOpenPanel(*this, *frame,
parameters.get(), *m_openPanelResultListener);
> +	       return;
> +	   }
> +    }
> +
>      // Since runOpenPanel() can spin a nested run loop we need to turn off
the responsiveness timer.
>      m_process->responsivenessTimer().stop();

Should we always return while controlled by automation even if there was no
automation session?

Likewise we should probably let the responsiveness timer stop before returning
above.

> Source/WebKit/WebKit.xcodeproj/project.pbxproj:9981
> +				99249AD51F1F1E5600B62FBB /*
AutomationFrontendDispatchers.cpp in Sources */,

Nit: You could run `sort-Xcode-project-file WebKit.xcodeproj/project.pbxproj`
and only add your new bits, but then they would be in the correct locations.