[webkit-reviews] review granted: [Bug 178616] [Cocoa] Web Automation: add SPI to tell whether the automation session is currently simulating user interactions : [Attachment 324488] 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 178616: [Cocoa] Web Automation: add SPI to tell whether the automation
session is currently simulating user interactions
https://bugs.webkit.org/show_bug.cgi?id=178616

Attachment 324488: Patch

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




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

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

> Source/WebKit/UIProcess/API/Cocoa/_WKAutomationSession.h:46
> + at property (nonatomic, readonly, getter=isSimulatingUserInteraction) BOOL
simulatingUserInteraction;

I think this needs:

    ... WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));

> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:668
> +	   m_simulatingUserInteraction = false;

It sounds like we should only be clearing the m_simulatingUserInteraction flag
when the m_pendingKeyboardEventsFlushedCallbacksPerPage list is empty.

Right now it sounds like we could be simulating events in multiple pages at the
same time and the first to finish will clear the global shared state.

> Source/WebKit/UIProcess/Automation/WebAutomationSession.h:264
> +    // True if a synthesized key or mouse event is still being processed.
> +    bool m_simulatingUserInteraction { false };

This says "or mouse event" but I don't see code below that sets this for mouse
events. The comment seems inaccurate.