Bug 26218 – REGRESSION (r44385): LayoutTests/fast/dom/Window/orphaned-frame-access.html fails on Debug builds

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[David Kilzer (:ddkilzer)]

* SUMMARY
LayoutTests/fast/dom/Window/orphaned-frame-access.html fails on the Leopard buildbot:

http://build.webkit.org/builders/Leopard%20Intel%20Debug%20%28Tests%29/builds/1769

and when run under guard malloc.

* STEPS TO REPRODUCE
$ ./WebKitTools/Scripts/build-webkit --debug
$ ./WebKitTools/Scripts/run-webkit-tests --debug -g fast/dom/Window

* RESULTS
Test fails thusly:

-property: PASS ... array: PASS ... missing property: PASS.
+property: PASS ... array: PASS ... [object CommentConstructor]

* REGRESSION
This seems to be a recent regression.

[David Kilzer (:ddkilzer)]

Actually, this doesn't require running under guard malloc to reproduce, just a Debug build:

$ ./WebKitTools/Scripts/run-webkit-tests --debug fast/dom/Window/orphaned-frame-access.html

[David Kilzer (:ddkilzer)]

An internal build bisect reveals:

Works: r44383  Fails: r44388

[David Kilzer (:ddkilzer)]

(In reply to comment #2)
> An internal build bisect reveals:
> 
> Works: r44383  Fails: r44388

However improbable this seems, a git-bisect reveals that this broke with a Debug build of r44385.

<http://trac.webkit.org/changeset/44385>

[David Kilzer (:ddkilzer)]

(In reply to comment #3)
> (In reply to comment #2)
> > An internal build bisect reveals:
> > 
> > Works: r44383  Fails: r44388
> 
> However improbable this seems, a git-bisect reveals that this broke with a
> Debug build of r44385.
> 
> <http://trac.webkit.org/changeset/44385>

Note that I had to apply the build fix in r44388 to both r44384 and r44385 to test this.

<http://trac.webkit.org/changeset/44388>

[Pavel Feldman]

I confirm that 44385 is breaking the test, Yury has a fix on its way.

[Yury Semikhatsky]

Created attachment 31090 [details]
When checking for main resource loading make sure that loader's frame is the main one.

Comment on attachment 31090 [details]
When checking for main resource loading make sure that loader's frame is the main one.

> +bool InspectorController::isMainResourceLoader(DocumentLoader* loader, const KURL& requestUrl) {
> +    return loader->frame() == m_inspectedPage->mainFrame() && requestUrl == loader->requestURL();
> +}

The opening brace at the beginning of a function definition should go on a separate line.

[Brent Fulgham]

Landed in @r44549.

[David Kilzer (:ddkilzer)]

(In reply to comment #8)
> Landed in @r44549.

http://trac.webkit.org/changeset./44549

[David Kilzer (:ddkilzer)]

(In reply to comment #8)
> Landed in @r44549.

http://trac.webkit.org/changeset/44549

[Alexey Proskuryakov]

+bool InspectorController::isMainResourceLoader(DocumentLoader* loader, const KURL& requestUrl)
+{
+    return loader->frame() == m_inspectedPage->mainFrame() && requestUrl == loader->requestURL();
+}

This looks like a bad misnomer to me. Our MainResourceLoader class is used for any frame's main resource, so the words "main resource" are not appropriate here.

Could you please make a patch to fix this (in a new bug)?