Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(286)

Issue 2276073002: Fix invalidation of absolute sized SVG content. (Closed)

Created:
4 years, 4 months ago by wkorman
Modified:
4 years, 2 months ago
Reviewers:
pdr., Xianzhu
CC:
chromium-reviews, szager+layoutwatch_chromium.org, zoltan1, blink-reviews-layout_chromium.org, pdr+renderingwatchlist_chromium.org, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, blink-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix invalidation of absolute sized SVG content. Minor cleanup of svg/custom/absolute-sized-content-with-resources.html LayoutTest with no functional changes. BUG=490725, 640273

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -35 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 chunk +1 line, -1 line 1 comment Download
A third_party/WebKit/LayoutTests/svg/custom/absolute-sized-content-with-resources.html View 1 chunk +26 lines, -0 lines 0 comments Download
D third_party/WebKit/LayoutTests/svg/custom/absolute-sized-content-with-resources.xhtml View 1 chunk +0 lines, -34 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp View 1 chunk +8 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
wkorman
I realize this is a hammer but given limited familiarity with SVG am unsure where ...
4 years, 4 months ago (2016-08-24 17:54:56 UTC) #3
Xianzhu
I would prefer https://codereview.chromium.org/2276783002/ which looks clearer about the real problem than this CL. https://codereview.chromium.org/2276073002/diff/1/third_party/WebKit/LayoutTests/TestExpectations ...
4 years, 4 months ago (2016-08-24 18:06:23 UTC) #4
wkorman
On 2016/08/24 18:06:23, Xianzhu wrote: > I would prefer https://codereview.chromium.org/2276783002/ which looks clearer > about ...
4 years, 4 months ago (2016-08-24 18:10:35 UTC) #5
pdr.
On 2016/08/24 at 18:10:35, wkorman wrote: > On 2016/08/24 18:06:23, Xianzhu wrote: > > I ...
4 years, 4 months ago (2016-08-24 18:17:33 UTC) #6
wkorman
4 years, 4 months ago (2016-08-24 18:21:19 UTC) #7
On 2016/08/24 18:17:33, pdr. wrote:
> On 2016/08/24 at 18:10:35, wkorman wrote:
> > On 2016/08/24 18:06:23, Xianzhu wrote:
> > > I would prefer https://codereview.chromium.org/2276783002/ which looks
> clearer
> > > about the real problem than this CL.
> > 
> > Actually that other change does not fix this test. We could merge them, but
in
> one case, specific to foreign-objects, we need to ignore whether the previous
> paint invalidation rect and new are different, so the conditionals could be
hard
> to read. My plan was to review after landing one, and either merge the
> conditionals or keep separate if easier to read/maintain that way.
> 
> This change is really high up in the invalidation stack. Can you investigate
how
> all of the other svg tests are passing without this change?

This could be the only one that is absolute-sized and changes the width of the
SVG element after initial layout:

wkorman@lemur:/ssd2/git/chrome/src$ find ./third_party/WebKit/LayoutTests/svg
-name "*.*html" | grep -v expected | xargs grep setProperty | grep width
./third_party/WebKit/LayoutTests/svg/as-image/svg-image-change-content-size.xhtml:
   document.getElementById("contentBox").style.setProperty("width", "600px");
./third_party/WebKit/LayoutTests/svg/custom/relative-sized-content-with-resources.xhtml:
           document.getElementById("contentBox").style.setProperty("width",
"400px");
./third_party/WebKit/LayoutTests/svg/custom/relative-sized-inner-svg.xhtml:     
      document.getElementById("contentBox").style.setProperty("width", "400px");
./third_party/WebKit/LayoutTests/svg/custom/object-sizing-no-width-height-change-content-box-size.xhtml:
           document.getElementById("contentBox").style.setProperty("width",
"400px");
./third_party/WebKit/LayoutTests/svg/custom/relative-sized-use-on-symbol.xhtml: 
          document.getElementById("contentBox").style.setProperty("width",
"400px");
./third_party/WebKit/LayoutTests/svg/custom/relative-sized-use-without-attributes-on-symbol.xhtml:
           document.getElementById("contentBox").style.setProperty("width",
"400px");
./third_party/WebKit/LayoutTests/svg/custom/absolute-sized-content-with-resources.xhtml:
           document.getElementById("contentBox").style.setProperty("width",
"400px");
./third_party/WebKit/LayoutTests/svg/custom/relative-sized-content.xhtml:       
    document.getElementById("contentBox").style.setProperty("width", "400px");
./third_party/WebKit/LayoutTests/svg/custom/relative-sized-image.xhtml:         
  document.getElementById("contentBox").style.setProperty("width", "400px");
./third_party/WebKit/LayoutTests/svg/custom/relative-sized-shadow-tree-content.xhtml:
           document.getElementById("contentBox").style.setProperty("width",
"400px");
./third_party/WebKit/LayoutTests/svg/custom/relative-sized-deep-shadow-tree-content.xhtml:
           document.getElementById("contentBox").style.setProperty("width",
"400px");
./third_party/WebKit/LayoutTests/svg/custom/relative-sized-shadow-tree-content-with-symbol.xhtml:
           document.getElementById("contentBox").style.setProperty("width",
"400px");

Powered by Google App Engine
This is Rietveld 408576698