[webkit-reviews] review denied: [Bug 107494] Add window occlusion criteria to determine page visibility on Mac : [Attachment 183876] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Kiran Muppala
<cmuppala at apple.com>'s request for review:
Bug 107494: Add window occlusion criteria to determine page visibility on Mac
https://bugs.webkit.org/show_bug.cgi?id=107494

Attachment 183876: Patch
https://bugs.webkit.org/attachment.cgi?id=183876&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=183876&action=review


r- pending some discussion of the implementation

> Source/WebKit2/UIProcess/API/mac/WKView.mm:1881
> +    if (context == &windowNumberPropertyObserverContext) {
> +	   id isPriorChange = [change
objectForKey:NSKeyValueChangeNotificationIsPriorKey];
> +	   if (isPriorChange && [isPriorChange isKindOfClass:[NSNumber class]]
&& ([isPriorChange boolValue] == YES))
> +	       [self _removeFromWindowViewsMap];
> +	   else
> +	       [self _addToWindowViewsMap];
> +    }

I don't understand why you have to observe the NSWindows' windowNumber
property. Does this ever change after window creation? Can't you just update
the window view's map in viewWillMoveToWindow/viewDidMoveToWindow?

> Source/WebKit2/UIProcess/API/mac/WKView.mm:2341
> +    WindowViewsMap *windowViewMap = [WKView _windowViewsMap];
> +    WindowViewsMap::iterator it = windowViewMap->find(windowID);
> +    if (it != windowViewMap->end()) {
> +	   HashSet<WKView *>& views = it->value;
> +	   for(HashSet<WKView *>::iterator view_it = views.begin(), view_end =
views.end(); view_it != view_end; ++view_it) {
> +	       WKView *view = *view_it;
> +	      
view->_data->_page->viewStateDidChange(WebPageProxy::ViewIsVisible);
> +	   }
> +    }

So another way to do this would be to keep track of all WKViews, and just go
through them all asking which window they are in.