Bug 176260 - REGRESSION (r219145): Toggling layer borders on a static document no longer works immediately
Summary: REGRESSION (r219145): Toggling layer borders on a static document no longer w...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-09-01 17:39 PDT by Simon Fraser (smfr)
Modified: 2018-01-08 10:47 PST (History)
5 users (show)

See Also:


Attachments
patch (1.45 KB, patch)
2018-01-08 05:40 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (4.48 KB, patch)
2018-01-08 06:49 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (6.17 KB, patch)
2018-01-08 10:01 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2017-09-01 17:39:42 PDT
After r219145, toggling compositing layer borders, either via the Debug menu, or via the button in Web Inspector, no longer immediately hides or shows the layer borders. You have to reload the page, or trigger some other style recalc for the change to take effect.
Comment 1 Radar WebKit Bug Importer 2017-09-01 17:40:08 PDT
<rdar://problem/34219966>
Comment 2 Antti Koivisto 2018-01-08 05:40:02 PST
Created attachment 330696 [details]
patch
Comment 3 Antti Koivisto 2018-01-08 06:49:58 PST
Created attachment 330701 [details]
patch
Comment 4 Simon Fraser (smfr) 2018-01-08 08:28:33 PST
Comment on attachment 330696 [details]
patch

I don't know if this i the right fix, and why it works. Layer borders are not painted; they are mapped to CA borders. And for layers with no backing store, setNeedsDisplay() is a no-op.
Comment 5 Simon Fraser (smfr) 2018-01-08 08:29:28 PST
Comment on attachment 330701 [details]
patch

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

> Source/WebCore/rendering/RenderLayerCompositor.cpp:325
> +        m_layerNeedsCompositingUpdate = true;

Maybe we only need this part?
Comment 6 Antti Koivisto 2018-01-08 09:30:44 PST
(In reply to Simon Fraser (smfr) from comment #5)
> Comment on attachment 330701 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=330701&action=review
> 
> > Source/WebCore/rendering/RenderLayerCompositor.cpp:325
> > +        m_layerNeedsCompositingUpdate = true;
> 
> Maybe we only need this part?

That's what I thought too but it doesn't work in MiniBrowser without also forcing paint. It is not clear to me what is supposed to make us paint.
Comment 7 Antti Koivisto 2018-01-08 09:31:51 PST
Safari and layout test are fine without the forced paint so I can land just that too.
Comment 8 Antti Koivisto 2018-01-08 09:43:00 PST
Actually Safari also needs forced paint to get repaint indicator right. Unlike borders that seems to be painted on top of the layer content.
Comment 9 Antti Koivisto 2018-01-08 10:01:05 PST
Created attachment 330710 [details]
patch
Comment 10 Antti Koivisto 2018-01-08 10:02:13 PST
Limited the repaint to the main layer that shows the counter.
Comment 11 WebKit Commit Bot 2018-01-08 10:47:30 PST
Comment on attachment 330710 [details]
patch

Clearing flags on attachment: 330710

Committed r226521: <https://trac.webkit.org/changeset/226521>
Comment 12 WebKit Commit Bot 2018-01-08 10:47:32 PST
All reviewed patches have been landed.  Closing bug.