Bug 58502 – Highly experimental fullscreen implementation for Chromium

• Component: New Bugs
• Status: RESOLVED WONTFIX

[Andrew Scherkus]

Highly experimental fullscreen implementation for Chromium

[Andrew Scherkus]

Created attachment 89512 [details]
Patch

[Andrew Scherkus]

Created attachment 89658 [details]
Patch

[Andrew Scherkus]

Created attachment 89709 [details]
Patch

[Andrew Scherkus]

Applied a tiny bit of darin's patch to ChromeClientImpl for paints/scrolls and now WebViewImpl swaps between m_client and m_fullscreen->client() as needed.

[Darin Fisher (:fishd, Google)]

Comment on attachment 89709 [details]
Patch

Looking better...

[Andrew Scherkus]

Created attachment 90461 [details]
Patch

[Andrew Scherkus]

Alright in this latest patch I experimented with seeing what would happen if we forced all existing users of WebViewImpl::client() to explicitly use WebWidgetClient or WebViewClient -- in effect simulating what would happen if WebViewClient didn't inherit from WebWidgetClient. I changed the method names so I could see which classes use WebWidgetClient as opposed to WebViewClient, etc etc

Result: works ok -- cursors get properly set, invalidations work, etc...

Also in this version WebFullscreenWidgetImpl is a pure delegate to the existing WebViewImpl meaning we might be able to do without it.

Basically I wonder if we can get to the point where we have the following set of objects to play with:
  - ChromeClientImpl (implements ChromeClient)
  - RenderView (implements WebViewClient)
  - RenderWidget (implements WebWidgetClient)
  - RenderFullscreenWidget (implements WebWidgetClient)
  - WebViewImpl (implements WebView)

If we allowed swapping WebWidgets on WebWidgetClients at run-time (as opposed to creation time), we could do some neat tricks where we point the existing RenderWidget to a NULL/fake WebWidget (so the old HWND doesn't try to render stuff), then swap in the existing WebViewImpl into the RenderFullscreenWidget -- avoiding the need for having WebFullscreenWidgetImpl.

I'm a bit worried about having both RenderWidget/View and RenderWidgetFullscreen point at the same WebViewImpl.

Again this is all new ground for me so happy to discuss in person!

[Andrew Scherkus]

Created attachment 99187 [details]
Less crazy active WebWidgetClient patch

Updated to work on trunk

[WebKit Review Bot]

Attachment 99187 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/chromium/ChangeLog', u'Sourc..." exit_code: 1

Source/WebKit/chromium/src/WebFullscreenWidgetImpl.h:38:  Missing space before {  [whitespace/braces] [5]
Source/WebKit/chromium/src/WebFullscreenWidgetImpl.h:58:  The parameter name "canvas" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit/chromium/src/WebFullscreenWidgetImpl.h:58:  The parameter name "rect" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit/chromium/src/WebFullscreenWidgetImpl.h:75:  The parameter name "direction" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit/chromium/src/WebFullscreenWidgetImpl.h:85:  One space before end of line comments  [whitespace/comments] [5]
Source/WebKit/chromium/src/WebFullscreenWidgetImpl.h:88:  The parameter name "client" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit/chromium/src/WebViewImpl.cpp:2692:  WebViewImpl::active_client is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit/chromium/src/WebViewImpl.h:457:  active_client is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit/chromium/src/WebViewImpl.h:593:  One space before end of line comments  [whitespace/comments] [5]
Source/WebKit/chromium/src/WebFullscreenWidgetImpl.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit/chromium/src/WebFullscreenWidgetImpl.cpp:84:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 11 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andrew Scherkus]

Created attachment 99194 [details]
Crazy view/widget split patch

Crazy view/widget split patch