Bug 159329 – [Mac] Get rid of the old timey rubber-banding linen pattern.

• Component: Layout and Rendering
• Status: REOPENED

[Andreas Kling]

We're sometimes loading the linen pattern image despite never actually showing it.
This can happen if we go down rendering code paths from the WebPage constructor, before settings are fully loaded.

[Andreas Kling]

Created attachment 282512 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 282512 [details]
Patch

Clearing flags on attachment: 282512

Committed r202725: <http://trac.webkit.org/changeset/202725>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Beth Dakin]

Do we have any API clients that set backgroundExtendsBeyondPage to YES? And if so, does this change their behavior?

[Andreas Kling]

(In reply to comment #4)
> Do we have any API clients that set backgroundExtendsBeyondPage to YES? And
> if so, does this change their behavior?

AFAICT the only client of this API is Safari, which always sets it to YES, and thus nothing changes.

[Andreas Kling]

(In reply to comment #5)
> (In reply to comment #4)
> > Do we have any API clients that set backgroundExtendsBeyondPage to YES? And
> > if so, does this change their behavior?
> 
> AFAICT the only client of this API is Safari, which always sets it to YES,
> and thus nothing changes.

Sorry, I spoke too soon. I found two other clients who also set it to YES. (see cf)

[Beth Dakin]

(In reply to comment #4)
> Do we have any API clients that set backgroundExtendsBeyondPage to YES? And
> if so, does this change their behavior?

Oh yeah, I guess I meant NO. :P

[Andreas Kling]

(In reply to comment #7)
> (In reply to comment #4)
> > Do we have any API clients that set backgroundExtendsBeyondPage to YES? And
> > if so, does this change their behavior?
> 
> Oh yeah, I guess I meant NO. :P

Oh duh, late night reading comprehension pls.
I couldn't find any clients that set this to NO.

[Tim Horton]

Isn't NO the default for WKView? (but not WKWebView?)

[Tim Horton]

(In reply to comment #9)
> Isn't NO the default for WKView? (but not WKWebView?)

(it is). So you have changed behavior for WKView clients. Probably most will not care, but we should make sure (App Store and iTunes come to mind).

[Andreas Kling]

(In reply to comment #10)
> (In reply to comment #9)
> > Isn't NO the default for WKView? (but not WKWebView?)
> 
> (it is). So you have changed behavior for WKView clients. Probably most will
> not care, but we should make sure (App Store and iTunes come to mind).

I see! I also found this in WTR:

    // This setting differs between the antique and modern Mac WebKit2 API.
    // For now, maintain the antique behavior, because some tests depend on it!
    // FIXME: We should be testing the default.
    WKPageSetBackgroundExtendsBeyondPage(m_mainWebView->page(), false);

While it appears this change hasn't affected any tests, perhaps it was wrong to remove the check for backgroundShouldExtendBeyondPage() in updateOverflowControlsLayers()..

[Tim Horton]

Well, we've talked about making YES the only value, so it's possible you've just accelerated the transition :)

[Andreas Kling]

This didn't end up affecting dirty "CoreUI image data" memory after all, so I'm going to roll it out since it unintentionally changed behavior. We can revisit it in the future once we're sure we want the new behavior.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 159473