Bug 71996 - REGRESSION(r98542): Chromium: Rendering error on Google maps
Summary: REGRESSION(r98542): Chromium: Rendering error on Google maps
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-09 21:29 PST by David Barr
Modified: 2011-11-13 15:44 PST (History)
6 users (show)

See Also:


Attachments
Patch (1.41 KB, patch)
2011-11-09 22:04 PST, David Barr
koivisto: review-
Details | Formatted Diff | Diff
Small but not minimal repro of maps alignment issue (8.18 KB, text/html)
2011-11-10 21:44 PST, David Barr
no flags Details
Slightly better repro, visible difference when served by http (8.26 KB, text/html)
2011-11-10 21:57 PST, David Barr
no flags Details
Minimal reproduction (238 bytes, text/html)
2011-11-11 02:21 PST, David Barr
no flags Details
fix (3.97 KB, patch)
2011-11-12 04:50 PST, Antti Koivisto
kling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Barr 2011-11-09 21:29:26 PST
http://code.google.com/p/chromium/issues/detail?id=102521

What steps will reproduce the problem?
1. Maps.google.com
2. Look for directions
3. By Car and By Transport

What is the expected result?

Lines depicting the path to take 

What happens instead?

Lines are not visible 
Icons are misaligned 

Bisection identified http://trac.webkit.org/changeset/98542 as the change that caused the regression.
Comment 1 David Barr 2011-11-09 21:38:44 PST
A minimal temporary fix is to simply disable the MatchedDeclarationCache by adding "result.isCacheable = false;" to the end of CSSStyleSelector::matchAllRules().
Comment 2 David Barr 2011-11-09 22:04:57 PST
Created attachment 114434 [details]
Patch
Comment 3 Mike Lawther 2011-11-09 22:25:46 PST
I haven't been able to repro the missing direction lines, but I the misaligned icons does occur in WebKit Nightly r99757.

Antti - this looks like it might be a dupe of https://bugs.webkit.org/show_bug.cgi?id=71703, since they both seem to have been caused by the same patch as identified by David?
Comment 4 Antti Koivisto 2011-11-10 12:41:57 PST
Comment on attachment 114434 [details]
Patch

Disabling the feature stops us from getting testing. I don't think we should do this as the bug does not make the build unlivable.
Comment 5 Antti Koivisto 2011-11-10 12:44:16 PST
Could some from Google provide a reduced test case for this? maps.google.com is generally difficult to debug due to obfuscation.
Comment 6 Mike Lawther 2011-11-10 15:10:23 PST
The feature has already been demonstrated to cause at least two regressions, this one on a live site and https://bugs.webkit.org/show_bug.cgi?id=71703.

While we don't have a minimal repro yet, the feature really doesn't seem to be working as intended ie no side effects other than memory/performance improvements.

Wouldn't it be better to roll this back, or effectively disable it as in David's patch so it can be examined offline? Also, the feature was submitted with no new tests - are you confident that all the new code was covered by the existing tests?
Comment 7 David Barr 2011-11-10 21:44:06 PST
Created attachment 114625 [details]
Small but not minimal repro of maps alignment issue

This is still quite messy but is sufficient to demonstrate a change in behavior.
Comment 8 David Barr 2011-11-10 21:57:19 PST
Created attachment 114627 [details]
Slightly better repro, visible difference when served by http
Comment 9 David Barr 2011-11-11 02:21:49 PST
Created attachment 114655 [details]
Minimal reproduction

Should see concentric squares but the middle square moves to the top.
Comment 10 Antti Koivisto 2011-11-11 05:57:18 PST
Great, thank! I'm on it.
Comment 11 Eric Seidel (no email) 2011-11-11 09:39:12 PST
Thank you very much for the reduction, david.
Comment 12 Antti Koivisto 2011-11-12 04:50:31 PST
Created attachment 114830 [details]
fix

Turns out that r98542 exposed that there is a field missing from StyleBoxData copy constructor.
Comment 13 Antti Koivisto 2011-11-12 05:33:59 PST
http://trac.webkit.org/changeset/100078
Comment 14 Eric Seidel (no email) 2011-11-12 14:18:53 PST
ouch!  Thanks antti!
Comment 15 Mike Lawther 2011-11-13 14:39:40 PST
Thanks for the quick fix Antti!
Comment 16 David Barr 2011-11-13 15:44:49 PST
Thank you, Antti! I've tested rolling the change into Chrome and it's all good news.