Bug 142945 - REGRESSION (Yosemite): WKView visibility notifications are messed up
Summary: REGRESSION (Yosemite): WKView visibility notifications are messed up
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-22 00:14 PDT by Alexey Proskuryakov
Modified: 2015-03-23 13:42 PDT (History)
8 users (show)

See Also:


Attachments
proposed fix (14.52 KB, patch)
2015-03-22 01:06 PDT, Alexey Proskuryakov
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-mavericks (580.59 KB, application/zip)
2015-03-22 01:55 PDT, Build Bot
no flags Details
proposed fix (15.60 KB, patch)
2015-03-22 23:20 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2015-03-22 00:14:53 PDT
This test fails on Yosemite when run alone:

run-webkit-tests fast/dom/Geolocation/requestQueuingForHiddenPage.html

The reason is that we use internal AppKit notifications, and clash with NSView in how they work. NSView has its own NSWindowDidOrderOffScreenNotification and NSWindowWillOrderOnScreenNotification observer, and unregisters them when it view gets a layer-backed ancestor. This happens asynchronously when a page is created, we send an enterAcceleratedCompositingMode message from WebProcess to UI process.

As both WKView and NSView try to dynamically add/remove the same view object as an observer, there are undoubtedly many situations in which they get confused.

This particular bug happens because WKView thinks that it is an observer, but NSView has already undone the registration.
Comment 1 Alexey Proskuryakov 2015-03-22 01:06:37 PDT
Created attachment 249191 [details]
proposed fix

We still need these despite having window occlusion notifications, because a simple -[NSWindow orderOut:nil] doesn't result in an occlusion notification.
Comment 2 WebKit Commit Bot 2015-03-22 01:08:32 PDT
Attachment 249191 [details] did not pass style-queue:


ERROR: Source/WebKit/mac/WebView/WebViewData.mm:94:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit/mac/WebView/WebViewData.mm:96:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:315:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:317:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 4 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Build Bot 2015-03-22 01:55:31 PDT
Comment on attachment 249191 [details]
proposed fix

Attachment 249191 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6061185279459328

New failing tests:
inspector-protocol/runtime/getProperties.html
fast/dom/Window/new-window-opener.html
Comment 4 Build Bot 2015-03-22 01:55:35 PDT
Created attachment 249192 [details]
Archive of layout-test-results from ews101 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 5 Joseph Pecoraro 2015-03-22 01:58:30 PDT
Comment on attachment 249191 [details]
proposed fix

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

> Source/WebKit2/UIProcess/API/mac/WKView.mm:294
> +- (id)initWithView:(WKView *)view;

Nit: instancetype (here and in the other inits)?
Comment 6 Alexey Proskuryakov 2015-03-22 23:20:11 PDT
Created attachment 249220 [details]
proposed fix

The crashes were due to a pre-existing bug, where observers were not removed for views after calling -close.
Comment 7 WebKit Commit Bot 2015-03-22 23:21:54 PDT
Attachment 249220 [details] did not pass style-queue:


ERROR: Source/WebKit/mac/WebView/WebViewData.mm:98:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit/mac/WebView/WebViewData.mm:100:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:315:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:317:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 4 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 WebKit Commit Bot 2015-03-23 13:42:45 PDT
Comment on attachment 249220 [details]
proposed fix

Clearing flags on attachment: 249220

Committed r181866: <http://trac.webkit.org/changeset/181866>
Comment 9 WebKit Commit Bot 2015-03-23 13:42:51 PDT
All reviewed patches have been landed.  Closing bug.