Bug 126169

Summary: REGRESSION: plugins/plugin-remove-readystatechange.html is failing on debug bots
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: Plug-insAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, koivisto
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=130653
Attachments:
Description Flags
proposed fix none

Description Alexey Proskuryakov 2013-12-23 11:27:47 PST
plugins/plugin-remove-readystatechange.html became very flaky on debug bots recently, possibly because of lazy tree creation (the first failure is in r160911:r160914 range, just a few revisions after r160908).

http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=plugins%2Fplugin-remove-readystatechange.html

Looks like it's an extra blank line:

 ALERT: PASS: element could not be re-appended
 This test passes if it does not trip an assert in debug builds. It ensures a readystatechange event can't get dispatched until after a plugin is fully removed.
+
Comment 1 Alexey Proskuryakov 2015-01-04 11:54:51 PST
Darin had a related fix for this test in bug 130653, but that got rolled out.

I'm going to tweak the test to make it non-flaky, but it looks like a real bug in rendering code (that I have no idea how to fix).

When embed is the last rendered element in a document, and there is a newline after it, we create a renderer for the newline, and don't remove it when the embed is removed. But when there is no embed by the time the first layout occurs, the newline is ignored.

So, we get different render trees for the same DOM, depending on its history.

It's unfortunate that this bug hasn't been looked into in more than a year, even though the culprit was immediately identified.
Comment 2 Alexey Proskuryakov 2015-01-04 12:03:40 PST
Created attachment 243934 [details]
proposed fix
Comment 3 Darin Adler 2015-01-04 13:42:24 PST
(In reply to comment #1)
> It's unfortunate that this bug hasn't been looked into in more than a year,
> even though the culprit was immediately identified.

What was the culprit?
Comment 4 Alexey Proskuryakov 2015-01-04 15:24:15 PST
Comment on attachment 243934 [details]
proposed fix

> What was the culprit?

Not a specific culprit in code, but circumstantial evidence was pointing to <http://trac.webkit.org/changeset/160908>.
Comment 5 WebKit Commit Bot 2015-01-04 16:04:46 PST
Comment on attachment 243934 [details]
proposed fix

Clearing flags on attachment: 243934

Committed r177879: <http://trac.webkit.org/changeset/177879>
Comment 6 WebKit Commit Bot 2015-01-04 16:04:50 PST
All reviewed patches have been landed.  Closing bug.