These were added http://trac.webkit.org/changeset/66251, but need to be implemented for V8. Of course, the related layout tests are also failing. cc-ing dglazkov to triage. I'm going to add to test_expectations for now.
Created attachment 83411 [details] Patch
With this patch and another small one to Chromium, Chromium supports the WebKit Full Screen APIs and passes all the layout tests when --enable-fullscreen is specified. The layout tests remain disabled until we make this the default. As mentioned in the patch description, the fullscreen rendering still needs to be implemented.
Comment on attachment 83411 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=83411&action=review I only reviewed the WebKit/chromium changes. > Source/WebKit/chromium/public/WebSettings.h:114 > + virtual void setFullScreenEnabled(bool) = 0; LGTM for this WebKit API change. > Source/WebKit/chromium/src/ChromeClientImpl.cpp:872 > + // TODO(ddorwin): We may need to call these someplace else when window resizes. TODO(ddorwin) -> FIXME > Source/WebKit/chromium/src/ChromeClientImpl.cpp:879 > + // TODO(ddorwin): We may need to call these someplace else when window resizes. ditto > Source/WebKit/chromium/src/ChromeClientImpl.cpp:886 > + // TODO(ddorwin): Implement. ditto
Created attachment 83561 [details] Addressed Darin's feedback. TODO -> FIXME
Comment on attachment 83561 [details] Addressed Darin's feedback. TODO -> FIXME View in context: https://bugs.webkit.org/attachment.cgi?id=83561&action=review Seems reasonable. > Source/WebKit/chromium/src/ChromeClientImpl.cpp:886 > + // FIXME: Implement. we have a notImplemented() call which may be more useful than a FIXME.
Comment on attachment 83561 [details] Addressed Darin's feedback. TODO -> FIXME Clearing flags on attachment: 83561 Committed r79584: <http://trac.webkit.org/changeset/79584>
All reviewed patches have been landed. Closing bug.
This doesn't compile if ENABLE_FULLSCREEN_API is not set to true (which means it's broken on the canaries currently since they use features_overrides.gypi). At a minimum you need a feature guard inside WebSettingsImpl::setFullScreenEnabled(bool enabled).
Reverted r79584 for reason: [chromium] Patch does not compile if ENABLE_FULLSCREEN_API is not set Committed r79588: <http://trac.webkit.org/changeset/79588>
Created attachment 83724 [details] Fixed the build break when ENABLE_FULLSCREEN_API is not 1 and used notImplemented() as Eric suggested.
Comment on attachment 83724 [details] Fixed the build break when ENABLE_FULLSCREEN_API is not 1 and used notImplemented() as Eric suggested. Looks like you'll get an unused variable warning in WebSettingsImpl::setFullScreenEnabled(bool enabled) if ENABLE(FULLSCREEN_API) is false, which is a compile error on at least some configs. We have an UNUSED_PARAM() macro for such things.
Created attachment 83726 [details] Added UNUSED_PARAM(). Thanks for catching that. The Mac Chromium config is apparently not one of them.
Comment on attachment 83726 [details] Added UNUSED_PARAM(). View in context: https://bugs.webkit.org/attachment.cgi?id=83726&action=review R=me, but please fix the nit before committing. > Source/WebKit/chromium/src/ChromeClientImpl.cpp:889 > + please do not add gratuitous whitespace. only one new line here.
http://trac.webkit.org/changeset/79588 might have broken GTK Linux 32-bit Release
Created attachment 83747 [details] Removed whitespace.
Comment on attachment 83747 [details] Removed whitespace. Clearing flags on attachment: 83747 Committed r79774: <http://trac.webkit.org/changeset/79774>
Broke Chromium Windows: http://build.webkit.org/builders/Chromium%20Win%20Release/builds/19456/steps/compile-webkit/logs/stdio
(In reply to comment #18) > Broke Chromium Windows: > http://build.webkit.org/builders/Chromium%20Win%20Release/builds/19456/steps/compile-webkit/logs/stdio I don't think this is a real failure after all. It's failing on build.webkit.org but building fine on build.chromium.org. We need to investigate what's happening though.
(In reply to comment #19) > (In reply to comment #18) > > Broke Chromium Windows: > > http://build.webkit.org/builders/Chromium%20Win%20Release/builds/19456/steps/compile-webkit/logs/stdio > > I don't think this is a real failure after all. It's failing on build.webkit.org but building fine on build.chromium.org. We need to investigate what's happening though. I think a clobber of the WebKit Chromium bots should resolve this. Looking at the build output, Document.cpp was not rebuilt even though the value of ENABLE_FULLSCREEN_API changed in Source/WebKit/chromium/features.gypi. The new value is being picked up in ChromeClientImpl.cpp, causing ChromeClientImpl::exitFullScreenForElement() to be compiled. The full Chromium build still has ENABLE_FULLSCREEN_API=0 until I check in the corresponding Chromium CL.
(In reply to comment #20) The Chromium Win bot is passing now.