Bug 73227 - REGRESSION(r101172): It made fast/dom/clone-node-style.html assert
Summary: REGRESSION(r101172): It made fast/dom/clone-node-style.html assert
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Critical
Assignee: Andreas Kling
URL:
Keywords:
: 73213 (view as bug list)
Depends on:
Blocks: 73392 73121
  Show dependency tree / graph
 
Reported: 2011-11-28 10:25 PST by Zhenyao Mo
Modified: 2011-11-30 06:07 PST (History)
9 users (show)

See Also:

Attachments
QtWebKit gdb backtrack (12.80 KB, text/plain)
2011-11-29 08:29 PST, Csaba Osztrogonác 		no flags Details
Proposed patch (5.06 KB, patch)
2011-11-29 10:11 PST, Andreas Kling 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Zhenyao Mo 2011-11-28 10:25:28 PST 
It's unclear when this started to happen.

Dimitri, can you help triage?
Comment 1 Zhenyao Mo 2011-11-28 10:29:33 PST 
Added the test to test_expectations.txt in r101264
Comment 2 Csaba Osztrogonác 2011-11-29 07:22:15 PST 
It is a regression caused by r101172. I got it with manual bisecting.
Andreas, could you check it, please?
Comment 3 Andreas Kling 2011-11-29 07:25:17 PST 
(In reply to comment #2)
> It is a regression caused by r101172. I got it with manual bisecting.
> Andreas, could you check it, please?

On it! Suspect it's the same issue as bug 73213, great that it broke a test :)
Comment 4 Csaba Osztrogonác 2011-11-29 07:27:17 PST 
Unfortunately it is a flakey crash. But you can easily reprodeuce with old-run-webkit-tests --debug fast/dom/clone-node-style.html --iterations 100
Comment 5 Csaba Osztrogonác 2011-11-29 08:29:08 PST 
Created attachment 116975 [details]
QtWebKit gdb backtrack

I reproduced this bug inside GBD with the following method:
$ gdb WebKitBuild/Debug/bin/DumpRenderTree
(gdb) run -
<ctrl+v> <ctrl+v> <ctrl+v> <ctrl+v> <ctrl+v> ... (fast/dom/clone-node-style.html was on my clipboard)

The crash backtrace is attached.
Comment 6 Csaba Osztrogonác 2011-11-29 09:11:50 PST 
I added this test to the Qt Skipped list: http://trac.webkit.org/changeset/101389

Please unskip it with the proper fix. Thanks in advance.
Comment 7 Andreas Kling 2011-11-29 10:09:08 PST 
*** Bug 73213 has been marked as a duplicate of this bug. ***
Comment 8 Andreas Kling 2011-11-29 10:11:00 PST 
Created attachment 116992 [details]
Proposed patch
Comment 9 Dan Beam 2011-11-29 21:24:18 PST 
bug 73392 may just be a straight dupe of this, but let's just make it block it for now (the proposed patch fixed the issue we're having in Chromium, btw).
Comment 10 Antti Koivisto 2011-11-30 04:56:55 PST 
Comment on attachment 116992 [details]
Proposed patch

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

r=me, with comments

> Source/WebCore/css/CSSMutableStyleDeclaration.h:-79
> -    CSSMutableStyleDeclaration& operator=(const CSSMutableStyleDeclaration&);

You probably want to hide this.

> Source/WebCore/css/CSSMutableStyleDeclaration.h:80
> +    void copyPropertiesAndStrictnessFrom(const CSSMutableStyleDeclaration&);

As discussed, it might be clearer to just have copyProperties() and use the existing accessors for transfering the strictness.
Comment 11 Andreas Kling 2011-11-30 05:14:04 PST 
Comment on attachment 116992 [details]
Proposed patch

As discussed with Antti, I'm cq+ing this as is so we get rid of the regression. I'm currently in transit and will make a follow-up patch with the requested adjustments later today.
Comment 12 WebKit Review Bot 2011-11-30 06:07:45 PST 
Comment on attachment 116992 [details]
Proposed patch

Clearing flags on attachment: 116992

Committed r101502: <http://trac.webkit.org/changeset/101502>
Comment 13 WebKit Review Bot 2011-11-30 06:07:52 PST 
All reviewed patches have been landed.  Closing bug.