Currently, DRT only supports WebKit full screen APIs. Now that Bug #56264 has landed, DRT should be able to run the WebKit2 full screen layout tests.
And by DRT, I mean WebKitTestRunner.
Created attachment 88338 [details] Patch
Whoops, missed the WebKit2/ChangeLog. But that's ok, as it gives the a chance to rebase and re-upload anyway.
Created attachment 88344 [details] Patch
Attachment 88344 [details] did not build on win: Build output: http://queues.webkit.org/results/8341043
Created attachment 88362 [details] Patch Forgot one #if ENABLE(FULLSCREEN_API) in WebPage.h.
Attachment 88362 [details] did not build on qt: Build output: http://queues.webkit.org/results/8341071
Created attachment 88363 [details] Patch Corrected the order of #endifs and end-braces in InjectedBundlePageFullScreenClient.h.
Attachment 88362 [details] did not build on win: Build output: http://queues.webkit.org/results/8345052
Attachment 88363 [details] did not build on qt: Build output: http://queues.webkit.org/results/8341073
Created attachment 88366 [details] Patch Made the #if in WKBundlePage.cpp more inclusive.
Attachment 88363 [details] did not build on win: Build output: http://queues.webkit.org/results/8342047
Comment on attachment 88366 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=88366&action=review > Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:143 > +void WKBundlePageWillEnterFullScreenForElement(WKBundlePageRef pageRef, WKBundleNodeHandleRef elementRef) > +{ > + WebCore::Node* coreNode = toImpl(elementRef)->coreNode(); > + if (!coreNode || !coreNode->isElementNode()) > + return; > + > + WebCore::Element* element = static_cast<WebCore::Element*>(coreNode); > + element->document()->webkitWillEnterFullScreenForElement(element); > +} > + > +void WKBundlePageDidEnterFullScreenForElement(WKBundlePageRef pageRef, WKBundleNodeHandleRef elementRef) > +{ > + WebCore::Node* coreNode = toImpl(elementRef)->coreNode(); > + if (!coreNode || !coreNode->isElementNode()) > + return; > + > + WebCore::Element* element = static_cast<WebCore::Element*>(coreNode); > + element->document()->webkitDidEnterFullScreenForElement(element); > +} > + > +void WKBundlePageWillExitFullScreenForElement(WKBundlePageRef pageRef, WKBundleNodeHandleRef elementRef) > +{ > + WebCore::Node* coreNode = toImpl(elementRef)->coreNode(); > + if (!coreNode || !coreNode->isElementNode()) > + return; > + > + WebCore::Element* element = static_cast<WebCore::Element*>(coreNode); > + element->document()->webkitWillExitFullScreenForElement(element); > +} > + > +void WKBundlePageDidExitFullScreenForElement(WKBundlePageRef pageRef, WKBundleNodeHandleRef elementRef) > +{ > + WebCore::Node* coreNode = toImpl(elementRef)->coreNode(); > + if (!coreNode || !coreNode->isElementNode()) > + return; > + > + WebCore::Element* element = static_cast<WebCore::Element*>(coreNode); > + element->document()->webkitDidExitFullScreenForElement(element); > +} All the logic should be in WebPage, not in the API layer. > Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.h:247 > +#if defined(ENABLE_FULLSCREEN_API) && ENABLE_FULLSCREEN_API > +// Full Screen client Please don't put #ifdefs like these in API headers. Clients will not not know the correct value to set. Instead, the functions should just be no-ops if the feature is not supported. > Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.h:257 > + WKBundlePageEnterFullScreenForElement enterFullScreenForElement; > + WKBundlePageExitFullScreenForElement exitFullScreenForElement; We usually prefix the set type of clients with Will or Did. > Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageFullScreenClient.h:46 > + bool supportsFullScreen(WebPage*, bool withKeyboard); Instead of a bool, this should probably be an enum.
(In reply to comment #13) > (From update of attachment 88366 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=88366&action=review > > > +void WKBundlePageDidExitFullScreenForElement(WKBundlePageRef pageRef, WKBundleNodeHandleRef elementRef) > > +{ > > + WebCore::Node* coreNode = toImpl(elementRef)->coreNode(); > > + if (!coreNode || !coreNode->isElementNode()) > > + return; > > + > > + WebCore::Element* element = static_cast<WebCore::Element*>(coreNode); > > + element->document()->webkitDidExitFullScreenForElement(element); > > +} > > All the logic should be in WebPage, not in the API layer. Done. (Well, in the page->fullScreenController()) > > Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.h:247 > > +#if defined(ENABLE_FULLSCREEN_API) && ENABLE_FULLSCREEN_API > > +// Full Screen client > > Please don't put #ifdefs like these in API headers. Clients will not not know the correct value to set. Instead, the functions should just be no-ops if the feature is not supported. Ah, I saw ENABLE_INSPECTOR in there and thought it was kosher. Unifdefd. > > Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.h:257 > > + WKBundlePageEnterFullScreenForElement enterFullScreenForElement; > > + WKBundlePageExitFullScreenForElement exitFullScreenForElement; > > We usually prefix the set type of clients with Will or Did. These are commands, not notifications. The will and did notifications come the other direction, and are also defined in this header. Or maybe I'm misunderstanding what "the set type of clients" means? > > Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageFullScreenClient.h:46 > > + bool supportsFullScreen(WebPage*, bool withKeyboard); > > Instead of a bool, this should probably be an enum. Sure thing boss.
Created attachment 89339 [details] Patch
Comment on attachment 89339 [details] Patch Rejecting attachment 89339 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-4', 'apply-..." exit_code: 2 Last 500 characters of output: atching file Source/WebKit2/WebProcess/WebPage/WebPage.h Hunk #4 succeeded at 610 (offset 3 lines). patching file Tools/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp patching file Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.h patching file Tools/WebKitTestRunner/TestController.cpp Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Sam Weinig', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/8452238
Well, I'll just land it manually then.
Committed r84206: <http://trac.webkit.org/changeset/84206>