Bug 113058 - REGRESSION (Safari 6 - ToT): Incorrectly assumes that RenderStyle data can be shared
Summary: REGRESSION (Safari 6 - ToT): Incorrectly assumes that RenderStyle data can be...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Morten Stenshorne
URL:
Keywords: InRadar, Regression
Depends on: 119425
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-22 07:49 PDT by Morten Stenshorne
Modified: 2014-04-23 11:40 PDT (History)
17 users (show)

See Also:


Attachments
test case (539 bytes, text/html)
2013-03-22 07:50 PDT, Morten Stenshorne
no flags Details
Patch (4.42 KB, patch)
2013-04-04 05:02 PDT, Morten Stenshorne
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-05 for chromium-linux-x86_64 (1.04 MB, application/zip)
2013-04-04 06:53 PDT, WebKit Review Bot
no flags Details
Patch (4.43 KB, patch)
2014-04-22 17:37 PDT, Morten Stenshorne
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 (467.26 KB, application/zip)
2014-04-22 18:36 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (497.42 KB, application/zip)
2014-04-22 18:55 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion (554.13 KB, application/zip)
2014-04-22 19:11 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Morten Stenshorne 2013-03-22 07:49:40 PDT
The RenderStyle of one element with "-webkit-logical-height" and initial writing mode is shared with a later element with the same "-webkit-logical-height" value but with non-initial writing mode.
Comment 1 Morten Stenshorne 2013-03-22 07:50:15 PDT
Created attachment 194542 [details]
test case
Comment 2 Morten Stenshorne 2013-04-04 05:02:53 PDT
Created attachment 196466 [details]
Patch
Comment 3 Andreas Kling 2013-04-04 05:10:08 PDT
Comment on attachment 196466 [details]
Patch

How is this related to the FIXME in isCacheableInMatchedPropertiesCache?

    // FIXME: CSSPropertyWebkitWritingMode modifies state when applying to document element. We can't skip the applying by caching.

In the case you are fixing, are we really only interested in the first check (the one relating to 'element') in isCacheableInMatchedPropertiesCache:


    if (element == element->document()->documentElement() && element->document()->writingModeSetOnDocumentElement())

Or in all the ones for 'style' and 'parentStyle' as well?
Comment 4 Morten Stenshorne 2013-04-04 05:39:48 PDT
I don't see how it's related at all. The "if (element == element->document()->documentElement() && element->document()->writingModeSetOnDocumentElement())" check doesn't do much for the test case, since non-default writing mode isn't set on the root. The particular check I needed for the test case is "if (style->writingMode() != RenderStyle::initialWritingMode())", but I'm sure one could create test cases that demonstrate the need for all the others as well. My idea was that, in order to figure out if two elements can share properties, you need to take a closer look at _both_ of them, not just the first one.
Comment 5 WebKit Review Bot 2013-04-04 06:53:55 PDT
Comment on attachment 196466 [details]
Patch

Attachment 196466 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17444027

New failing tests:
fast/multicol/overflow-content.html
Comment 6 WebKit Review Bot 2013-04-04 06:53:59 PDT
Created attachment 196474 [details]
Archive of layout-test-results from gce-cr-linux-05 for chromium-linux-x86_64

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-05  Port: chromium-linux-x86_64  Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04
Comment 7 Morten Stenshorne 2013-04-04 06:59:15 PDT
That failure was expected; see 113781. Need that fix for it to go away.
Comment 8 Build Bot 2013-04-04 21:22:08 PDT
Comment on attachment 196466 [details]
Patch

Attachment 196466 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17432262
Comment 9 Morten Stenshorne 2013-07-26 12:09:31 PDT
Fixed in Blink: http://code.google.com/p/chromium/issues/detail?id=236329

Unassigning myself from this one.
Comment 10 Ryosuke Niwa 2013-07-31 16:18:23 PDT
This patch has been landed in Blink: https://chromium.googlesource.com/chromium/blink/+/1a47e927dca52f4a166b52665d9ade954272c62b
Comment 11 Radar WebKit Bug Importer 2013-07-31 16:20:50 PDT
<rdar://problem/14612331>
Comment 12 Andreas Kling 2013-07-31 16:48:28 PDT
Comment on attachment 196466 [details]
Patch

Well r=me. The Blink version also removes the Element* argument to the function which is pretty neat.
Comment 13 Alexey Proskuryakov 2013-08-01 13:55:37 PDT
Andreas, do you intend to land this patch, too?
Comment 14 WebKit Commit Bot 2013-08-01 14:24:19 PDT
Comment on attachment 196466 [details]
Patch

Clearing flags on attachment: 196466

Committed r153608: <http://trac.webkit.org/changeset/153608>
Comment 15 WebKit Commit Bot 2013-08-01 14:24:22 PDT
All reviewed patches have been landed.  Closing bug.
Comment 16 Ryosuke Niwa 2013-08-01 19:05:03 PDT
It appears that this patch broke fast/multicol/overflow-content.html:

http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fmulticol%2Foverflow-content.html
Comment 17 WebKit Commit Bot 2013-08-01 19:34:44 PDT
Re-opened since this is blocked by bug 119425
Comment 18 Morten Stenshorne 2013-08-01 23:43:47 PDT
See bug 113781.
Comment 19 Morten Stenshorne 2014-04-22 17:25:46 PDT
This is now blocking bug 131809. And it still depends on the fix for bug 113781.
Comment 20 Morten Stenshorne 2014-04-22 17:37:03 PDT
Created attachment 229930 [details]
Patch
Comment 21 Build Bot 2014-04-22 18:36:09 PDT
Comment on attachment 229930 [details]
Patch

Attachment 229930 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6350183520534528

New failing tests:
fast/multicol/overflow-content.html
Comment 22 Build Bot 2014-04-22 18:36:31 PDT
Created attachment 229938 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 23 Build Bot 2014-04-22 18:55:46 PDT
Comment on attachment 229930 [details]
Patch

Attachment 229930 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5886498851258368

New failing tests:
fast/events/ghostly-mousemoves-in-subframe.html
fast/multicol/overflow-content.html
Comment 24 Build Bot 2014-04-22 18:55:53 PDT
Created attachment 229939 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 25 Build Bot 2014-04-22 19:11:09 PDT
Comment on attachment 229930 [details]
Patch

Attachment 229930 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5245981788471296

New failing tests:
fast/events/ghostly-mousemoves-in-subframe.html
fast/multicol/overflow-content.html
Comment 26 Build Bot 2014-04-22 19:11:19 PDT
Created attachment 229942 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 27 Dave Hyatt 2014-04-23 06:07:43 PDT
Comment on attachment 229930 [details]
Patch

r=me
Comment 28 WebKit Commit Bot 2014-04-23 11:40:25 PDT
Comment on attachment 229930 [details]
Patch

Clearing flags on attachment: 229930

Committed r167716: <http://trac.webkit.org/changeset/167716>
Comment 29 WebKit Commit Bot 2014-04-23 11:40:34 PDT
All reviewed patches have been landed.  Closing bug.