[webkit-reviews] review granted: [Bug 34661] REGRESSION (Safari 4.0-> Safari 4.0.4) NPP_SetWindow no longer sets a clipRect of (0, 0, 0, 0) when it becomes hidden : [Attachment 48258] Proposed fix that properly clips in-process and out-of-process Core Animation plug-ins

[bugzilla-daemon at webkit.org]

Mark Rowe (bdash) <mrowe at apple.com> has granted Kevin Decker
<kdecker at apple.com>'s request for review:
Bug 34661: REGRESSION (Safari 4.0-> Safari 4.0.4) NPP_SetWindow no longer sets
a clipRect of (0,0,0,0) when it becomes hidden
https://bugs.webkit.org/show_bug.cgi?id=34661

Attachment 48258: Proposed fix that properly clips in-process and
out-of-process Core Animation plug-ins
https://bugs.webkit.org/attachment.cgi?id=48258&action=review

------- Additional Comments from Mark Rowe (bdash) <mrowe at apple.com>
> @@ -351,6 +333,13 @@
>  
>	   window.clipRect.bottom = window.clipRect.top;
>	   window.clipRect.left = window.clipRect.right;
> +	   
> +	   // Core Animation plug-ins need to be updated (with a 0,0,0,0
clipRect) when
> +	   // moved to a background tab. We don't do this for Core Graphics
plug-ins as
> +	   // older versions of Flash have historical WebKit-specific code that
isn't
> +	   // compatible with this behavior.

This comment would be clearer if the parenthesized clause were part of the
sentence.

> +    // Core Animation plug-ins need to be updated (with a 0,0,0,0 clipRect)
when
> +    // moved to a background tab. We don't do this for Core Graphics
plug-ins as
> +    // older versions of Flash have historical WebKit-specific code that
isn't
> +    // compatible with this behavior.    

Same here.

> +- (BOOL)superviewsHaveSuperviews
> +{
> +    NSView *contentView = [[self window] contentView];
> +    NSView *view;
> +    for (view = self; view != nil; view = [view superview]) { 
> +	   if (view == contentView) {
> +	       return YES;
> +	   }
> +    }
> +    return NO;
> +}

You should clean up the style of this while you’re moving it -- move the
declaration of view in to the loop, remove the explicit comparison with nil,
and the extra braces.

> +- (BOOL)shouldClipOutPlugin
> +{
> +    NSWindow *window = [self window];
> +    return window == nil || [window isMiniaturized] || [NSApp isHidden] ||
![self superviewsHaveSuperviews] || [self isHiddenOrHasHiddenAncestor];
> +}

Same here with the explicit comparison with nil.

r=me