Start fleshing out the commands that are currently stubbed.
<rdar://problem/24947489>
Created attachment 272728 [details] WIP Start making the protocol specification closer to what is needed. Add some TODOs with blind code in WebAutomationSession for what needs to be done
Created attachment 273087 [details] Patch 1/2
Created attachment 273088 [details] Patch 2/2
Comment on attachment 273087 [details] Patch 1/2 View in context: https://bugs.webkit.org/attachment.cgi?id=273087&action=review r=me > Source/WebKit2/ChangeLog:6 > + Please put the subtitle text after Reviewed By
Comment on attachment 273088 [details] Patch 2/2 View in context: https://bugs.webkit.org/attachment.cgi?id=273088&action=review Other than concern about page map getting out of sync it looks good. > Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp:105 > + auto iter = m_handleWebPageMap.find(handle); Nit: usually call this it or result > Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp:108 > + return WebProcessProxy::webPage(iter->value); Does this return null for pages that died? > Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp:117 > + String handle = WebCore::createCanonicalUUIDString().convertToASCIIUppercase(); Nice! > Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp:133 > + Usually I try to never assign the out parameter until the very end so it is mutually exclusive to errorString being set. Not super important here though since there is no error case. > Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp:159 > + FAIL_WITH_PREDEFINED_ERROR_MESSAGE(InternalError); You could add a more specific error code for this though its not required by spec. > Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp:175 > + page->tryClose(); There is likely some extra steps to take in case beforeunload dialog happens but we can punt for now. > Source/WebKit2/UIProcess/Automation/WebAutomationSession.h:99 > + HandleWebPageMap m_handleWebPageMap; I think these maps could get out of sync if a process crashes or page dies in some other way not through this API. Should we be more paranoid (I.e., recheck the process pool page list before handing out a WebPageProxy ref)?
Comment on attachment 273088 [details] Patch 2/2 View in context: https://bugs.webkit.org/attachment.cgi?id=273088&action=review >> Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp:105 >> + auto iter = m_handleWebPageMap.find(handle); > > Nit: usually call this it or result The other places I saw find being used in WebCore/WebKit mostly used iter. >> Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp:108 >> + return WebProcessProxy::webPage(iter->value); > > Does this return null for pages that died? Yep, it is a global map that is maintained by the lifetime of the page. >> Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp:175 >> + page->tryClose(); > > There is likely some extra steps to take in case beforeunload dialog happens but we can punt for now. I figured the beforeunload would be covered by the alert endpoints. >> Source/WebKit2/UIProcess/Automation/WebAutomationSession.h:99 >> + HandleWebPageMap m_handleWebPageMap; > > I think these maps could get out of sync if a process crashes or page dies in some other way not through this API. Should we be more paranoid (I.e., recheck the process pool page list before handing out a WebPageProxy ref)? The maps only store the uint64_t pageID for a WebPageProxy. We use WebProcessProxy::webPage() to look up the ID. It will not return a bad WebPageProxy ref.
Comment on attachment 273088 [details] Patch 2/2 r=me
https://trac.webkit.org/r197620 https://trac.webkit.org/r197621