Merge PageVisibilityState & ViewState::IsVisible in WebKit2

• Component: WebKit2
• Status: RESOLVED FIXED

[Gavin Barraclough]

WebKit2 redundantly tracks the visibility of the view through two mechanisms - the visibility state, and the view state. Remove visibility state from the WebKit2 layer. The visibility state also tracks the 'pretender' state - so split this out and handle it separately (a change we should make in WebCore, too).

Removing the redundancy also removes the ability from the API to set a fake visibility state (IsVisible tracks the actual visibility of the view). Through private API (WKPageSetVisibilityState) a client could previously request the view be reported as hidden/visible, but this didn't really work - the override was not enforced and the API may reset the state at an arbitrary point. The mechanism is only used by testing code, which instead should actually update the view visibility (this tests more of the actual visibility mechanisms used by the browser). The one aspect of the API relied on by existing clients is the ability to initialize a hidden view as prerender -continue to support this specific functionality via WKPageSetVisibilityState, to maintain backwards compatibility.

[Gavin Barraclough]

Created attachment 220000 [details]
Fixed?

[WebKit Commit Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/events/page-visibility-transition-test-expected.txt', u'LayoutTests/fast/events/page-visibility-transition-test.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/page/Page.cpp', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/C/WKPage.cpp', u'Source/WebKit2/UIProcess/WebPageProxy.cpp', u'Source/WebKit2/UIProcess/WebPageProxy.h', u'Source/WebKit2/WebProcess/WebPage/WebPage.cpp', u'Source/WebKit2/WebProcess/WebPage/WebPage.h', u'Source/WebKit2/WebProcess/WebPage/WebPage.messages.in', u'Tools/ChangeLog', u'Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj', u'Tools/TestWebKitAPI/Tests/WebKit2/PageVisibilityState.cpp', u'Tools/TestWebKitAPI/Tests/mac/PageVisibilityStateWithWindowChanges.mm', u'Tools/WebKitTestRunner/TestController.cpp', u'Tools/WebKitTestRunner/TestController.h', u'Tools/WebKitTestRunner/efl/TestControllerEfl.cpp', u'Tools/WebKitTestRunner/gtk/TestControllerGtk.cpp', u'Tools/WebKitTestRunner/mac/TestControllerMac.mm', '--commit-queue']" exit_code: 1
ERROR: Tools/WebKitTestRunner/TestController.h:167:  The parameter name "hidden" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 18 files


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

[EFL EWS Bot]

Comment on attachment 220000 [details]
Fixed?

Attachment 220000 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/5033067089494016

[Gavin Barraclough]

Created attachment 220002 [details]
style, efl fixes

[EFL EWS Bot]

Comment on attachment 220002 [details]
style, efl fixes

Attachment 220002 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/6107173985714176

[Gavin Barraclough]

Created attachment 220008 [details]
efl build fix take 2

[Gavin Barraclough]

Created attachment 220022 [details]
Stupid autocorrect :-( ... s/pretender/prerender/ in Changelog

[Alexey Proskuryakov]

Comment on attachment 220022 [details]
Stupid autocorrect :-( ... s/pretender/prerender/ in Changelog

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

> Source/WebCore/ChangeLog:20
> +        by the browser). The one aspect of the API relied on by existing clients is the ability to
> +        initialize a hidden view as prerender -continue to support this specific functionality via

Seems like there should be a space in "-continue".

> Source/WebCore/ChangeLog:23
> +
> +

Extra blank line.

> Source/WebCore/ChangeLog:24
> +        WebCore - chnages the the API tests exposed a bug, a view should only ever come out of the

chnages, the the

> Source/WebKit2/ChangeLog:20
> +        initialize a hidden view as prerender -continue to support this specific functionality via

-continue

> Source/WebKit2/ChangeLog:23
> +
> +

Extra blank line.

> Tools/ChangeLog:23
> +        initialize a hidden view as prerender -continue to support this specific functionality via
> +        WKPageSetVisibilityState, to maintain backwards compatibility.
> +
> +

...

Does this really need to be repeated in every ChangeLog? I usually put extra info into a project that had the most changes - they are all combined for commit message anyway.

> Tools/WebKitTestRunner/mac/TestControllerMac.mm:90
> +        [window makeKeyAndOrderFront:nil];

Hopefully it's not forced back on screen, so nothing visually blinks and prevents using the computer while tests are running?

> Tools/WebKitTestRunner/mac/TestControllerMac.mm:91
> +        [NSApp activateIgnoringOtherApps:YES];

Why is this needed? A comment would probably be of use.

[Gavin Barraclough]

(In reply to comment #8)
> > Tools/WebKitTestRunner/mac/TestControllerMac.mm:90
> > +        [window makeKeyAndOrderFront:nil];
> 
> Hopefully it's not forced back on screen, so nothing visually blinks and prevents using the computer while tests are running?

No,from my testing the window is not actually forced onscreen.

> > Tools/WebKitTestRunner/mac/TestControllerMac.mm:91
> > +        [NSApp activateIgnoringOtherApps:YES];
> 
> Why is this needed? A comment would probably be of use.

You're right, looks like this was overkill - removed.

Thanks Alexey!

[Gavin Barraclough]

Committed revision 161105.