Bug 26218 - REGRESSION (r44385): LayoutTests/fast/dom/Window/orphaned-frame-access.html fails on Debug builds
Summary: REGRESSION (r44385): LayoutTests/fast/dom/Window/orphaned-frame-access.html f...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords: LayoutTestFailure, NeedsReduction, Regression
Depends on:
Blocks:
 
Reported: 2009-06-05 09:30 PDT by David Kilzer (:ddkilzer)
Modified: 2009-06-15 01:52 PDT (History)
7 users (show)

See Also:


Attachments
When checking for main resource loading make sure that loader's frame is the main one. (3.04 KB, patch)
2009-06-09 06:33 PDT, Yury Semikhatsky
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2009-06-05 09:30:47 PDT
* 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.
Comment 1 David Kilzer (:ddkilzer) 2009-06-05 09:37:18 PDT
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
Comment 2 David Kilzer (:ddkilzer) 2009-06-07 06:14:18 PDT
An internal build bisect reveals:

Works: r44383  Fails: r44388
Comment 3 David Kilzer (:ddkilzer) 2009-06-07 06:58:53 PDT
(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>
Comment 4 David Kilzer (:ddkilzer) 2009-06-07 07:06:36 PDT
(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>
Comment 5 Pavel Feldman 2009-06-09 06:25:14 PDT
I confirm that 44385 is breaking the test, Yury has a fix on its way.
Comment 6 Yury Semikhatsky 2009-06-09 06:33:33 PDT
Created attachment 31090 [details]
When checking for main resource loading make sure that loader's frame is the main one.
Comment 7 mitz 2009-06-09 10:48:02 PDT
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.
Comment 8 Brent Fulgham 2009-06-09 17:26:14 PDT
Landed in @r44549.
Comment 9 David Kilzer (:ddkilzer) 2009-06-09 21:41:22 PDT
(In reply to comment #8)
> Landed in @r44549.

http://trac.webkit.org/changeset./44549
Comment 10 David Kilzer (:ddkilzer) 2009-06-09 21:41:40 PDT
(In reply to comment #8)
> Landed in @r44549.

http://trac.webkit.org/changeset/44549
Comment 11 Alexey Proskuryakov 2009-06-15 01:52:12 PDT
+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)?