Bug 73802 - [Qt] REGRESSION(101967): It made editing/style/iframe-onload-crash-mac.html timeout
: [Qt] REGRESSION(101967): It made editing/style/iframe-onload-crash-mac.html t...
Status: RESOLVED FIXED
: WebKit
Tools / Tests
: 528+ (Nightly build)
: All All
: P1 Critical
Assigned To:
:
: Qt, QtTriaged
:
: 73737 79666 79668
  Show dependency treegraph
 
Reported: 2011-12-05 00:10 PST by
Modified: 2012-05-17 10:54 PST (History)


Attachments
proposed fix (1.39 KB, patch)
2012-04-16 00:01 PST, Kristóf Kosztyó
no flags Review Patch | Details | Formatted Diff | Diff
Patch (6.54 KB, patch)
2012-05-16 17:55 PST, Caio Marcelo de Oliveira Filho
no flags Review Patch | Details | Formatted Diff | Diff
Patch for landing (6.65 KB, patch)
2012-05-17 09:48 PST, Caio Marcelo de Oliveira Filho
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-12-05 00:10:20 PST
After r101967 editing/style/iframe-onload-crash-mac.html fails with timeout.
------- Comment #1 From 2011-12-05 00:21:32 PST -------
I skipped the failing test on Qt: http://trac.webkit.org/changeset/101976
------- Comment #2 From 2011-12-05 08:12:13 PST -------
Does the test time out when it's ran by itself?
------- Comment #3 From 2011-12-05 08:13:19 PST -------
(In reply to comment #2)
> Does the test time out when it's ran by itself?

Yes.
------- Comment #4 From 2011-12-05 08:19:11 PST -------
But I don't understand why this is failing only on Qt. e.g. GTK must be using the same behavior as Qt (this test only behaves differently on GTK), but it's not timing out there.

Regardless, I don't have any machine that can build Qt at the moment, and I need to let someone from Qt port investigate this as this failure is happening only on Qt.
------- Comment #5 From 2012-04-16 00:01:42 PST -------
Created an attachment (id=137288) [details]
proposed fix

Hi
I have found this test is falling into an infinite loop at ApplyStyleCommand::pushDownInlineStyleAroundNode
------- Comment #6 From 2012-04-16 00:04:49 PST -------
(From update of attachment 137288 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=137288&action=review

> Source/WebCore/editing/ApplyStyleCommand.cpp:968
> +        if (!(current->contains(targetNode)))
> +            break;

This should never be the case. When/why is this happening?
------- Comment #7 From 2012-04-16 00:39:45 PST -------
(In reply to comment #6)
> (From update of attachment 137288 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=137288&action=review
> 
> > Source/WebCore/editing/ApplyStyleCommand.cpp:968
> > +        if (!(current->contains(targetNode)))
> > +            break;
> 
> This should never be the case. When/why is this happening?

It happens when this test run, but I don't know why. This test fails into an infinite loop and it assert with debug build.
------- Comment #8 From 2012-04-16 00:42:29 PST -------
(In reply to comment #7)
> (In reply to comment #6)
> > (From update of attachment 137288 [details] [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=137288&action=review
> > 
> > > Source/WebCore/editing/ApplyStyleCommand.cpp:968
> > > +        if (!(current->contains(targetNode)))
> > > +            break;
> > 
> > This should never be the case. When/why is this happening?
> 
> It happens when this test run, but I don't know why. This test fails into an infinite loop and it assert with debug build.

Could you step it through on the debugger and tell us how we fall into this bad state?
------- Comment #9 From 2012-05-16 12:51:23 PST -------
I'm investigating this.
------- Comment #10 From 2012-05-16 17:55:21 PST -------
Created an attachment (id=142382) [details]
Patch
------- Comment #11 From 2012-05-16 17:59:45 PST -------
(From update of attachment 142382 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=142382&action=review

> Source/WebCore/editing/ApplyStyleCommand.cpp:995
> +            Node* child = currentChildren.at(i).get();

Please do currentChildren[i].get(); instead.

> Source/WebCore/editing/ApplyStyleCommand.cpp:996
> +            if (!child->parentNode())

We could go with an even stricter check like child->parentNode() != current.
------- Comment #12 From 2012-05-17 09:48:59 PST -------
Created an attachment (id=142491) [details]
Patch for landing
------- Comment #13 From 2012-05-17 10:53:57 PST -------
(From update of attachment 142491 [details])
Clearing flags on attachment: 142491

Committed r117463: <http://trac.webkit.org/changeset/117463>
------- Comment #14 From 2012-05-17 10:54:06 PST -------
All reviewed patches have been landed.  Closing bug.