Bug 73802

Summary:

[Qt] REGRESSION(101967): It made editing/style/iframe-onload-crash-mac.html timeout

Product:

WebKit

Reporter:

Csaba Osztrogonác <ossy>

Component:

Tools / Tests

Assignee:

Caio Marcelo de Oliveira Filho <cmarcelo>

Status:

RESOLVED FIXED

Severity:

Critical

CC:

cmarcelo, darin, eric, kkristof, ossy, rniwa, tkent, webkit.review.bot, webkit-sed

Priority:

Keywords:

Qt, QtTriaged

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

73737, 79666, 79668

Attachments:

Description	Flags
proposed fix	none
Patch	none
Patch for landing	none

Csaba Osztrogonác

Reported 2011-12-05 00:10:20 PST

After r101967 editing/style/iframe-onload-crash-mac.html fails with timeout.

Attachments
proposed fix (1.39 KB, patch) 2012-04-16 00:01 PDT, Kristóf Kosztyó	no flags	Details Formatted Diff Diff
Patch (6.54 KB, patch) 2012-05-16 17:55 PDT, Caio Marcelo de Oliveira Filho	no flags	Details Formatted Diff Diff
Patch for landing (6.65 KB, patch) 2012-05-17 09:48 PDT, Caio Marcelo de Oliveira Filho	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Csaba Osztrogonác

Comment 1 2011-12-05 00:21:32 PST

I skipped the failing test on Qt: http://trac.webkit.org/changeset/101976

Ryosuke Niwa

Comment 2 2011-12-05 08:12:13 PST

Does the test time out when it's ran by itself?

Csaba Osztrogonác

Comment 3 2011-12-05 08:13:19 PST

(In reply to comment #2) > Does the test time out when it's ran by itself? Yes.

Ryosuke Niwa

Comment 4 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.

Kristóf Kosztyó

Comment 5 2012-04-16 00:01:42 PDT

Created attachment 137288 [details] proposed fix Hi I have found this test is falling into an infinite loop at ApplyStyleCommand::pushDownInlineStyleAroundNode

Ryosuke Niwa

Comment 6 2012-04-16 00:04:49 PDT

Comment on attachment 137288 [details] proposed fix 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?

Kristóf Kosztyó

Comment 7 2012-04-16 00:39:45 PDT

(In reply to comment #6) > (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? 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.

Ryosuke Niwa

Comment 8 2012-04-16 00:42:29 PDT

(In reply to comment #7) > (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. Could you step it through on the debugger and tell us how we fall into this bad state?

Caio Marcelo de Oliveira Filho

Comment 9 2012-05-16 12:51:23 PDT

I'm investigating this.

Caio Marcelo de Oliveira Filho

Comment 10 2012-05-16 17:55:21 PDT

Created attachment 142382 [details] Patch

Ryosuke Niwa

Comment 11 2012-05-16 17:59:45 PDT

Comment on attachment 142382 [details] Patch 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.

Caio Marcelo de Oliveira Filho

Comment 12 2012-05-17 09:48:59 PDT

Created attachment 142491 [details] Patch for landing

WebKit Review Bot

Comment 13 2012-05-17 10:53:57 PDT

Comment on attachment 142491 [details] Patch for landing Clearing flags on attachment: 142491 Committed r117463: <http://trac.webkit.org/changeset/117463>

WebKit Review Bot

Comment 14 2012-05-17 10:54:06 PDT

All reviewed patches have been landed. Closing bug.

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