Bug 68291

Summary: ASSERTION FAILED: documentLoader in WebKit/Source/WebCore/inspector/InspectorInstrumentation.cpp(597)
Product: WebKit Reporter: Tony Chang <tony>
Component: Web Inspector (Deprecated)Assignee: Vsevolod Vlasov <vsevik>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, loislo, pfeldman, vsevik, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
test server
none
Patch
none
Patch tony: review+

Description Tony Chang 2011-09-16 16:36:36 PDT
When loading a multipart page, I hit this assert in Chrome.  It seems to work fine when using test_shell.

ASSERTION FAILED: documentLoader
../../third_party/WebKit/Source/WebCore/inspector/InspectorInstrumentation.cpp(597) : static void WebCore::InspectorInstrumentation::loadEventFiredImpl(WebCore::InstrumentingAgents*, WebCore::Frame*, const WebCore::KURL&)


Specifically, if I run "out/Debug/chrome http://localhost:8099/multipart", I hit the assert.

I've attached the python webserver (just run python test-server-simple.py to launch).
Comment 1 Tony Chang 2011-09-16 16:37:26 PDT
Created attachment 107745 [details]
test server
Comment 2 Vsevolod Vlasov 2011-09-21 10:55:07 PDT
Probably related: http://code.google.com/p/chromium/issues/detail?id=94392
Comment 3 Vsevolod Vlasov 2011-09-22 06:15:52 PDT
Created attachment 108323 [details]
Patch
Comment 4 Vsevolod Vlasov 2011-09-22 06:29:14 PDT
I was not able to reproduce this bug, but this piece of code could be simplified to ensure this does not happens anymore.
Comment 5 WebKit Review Bot 2011-09-22 10:27:48 PDT
Comment on attachment 108323 [details]
Patch

Attachment 108323 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9794302
Comment 6 Vsevolod Vlasov 2011-09-22 11:18:56 PDT
Created attachment 108368 [details]
Patch
Comment 7 Tony Chang 2011-09-22 16:13:42 PDT
Comment on attachment 108368 [details]
Patch

Seems fine.  Might want to have someone more familiar with Inspector code also take a look.
Comment 8 Yury Semikhatsky 2011-09-22 23:47:48 PDT
(In reply to comment #7)
> (From update of attachment 108368 [details])
> Seems fine.  Might want to have someone more familiar with Inspector code also take a look.

Tony, are you sure url != documentLoader->requestURL() check was redundant there?
Comment 9 Yury Semikhatsky 2011-09-23 00:21:14 PDT
Comment on attachment 108368 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=108368&action=review

> Source/WebCore/inspector/InspectorInstrumentation.cpp:-602
> -    if (frame->page()->mainFrame() != frame || url != documentLoader->requestURL())

Do we receive load event notifications only for the frame main resource? If not, the check should be preserved.
Comment 10 Vsevolod Vlasov 2011-09-23 03:20:21 PDT
(In reply to comment #9)
> (From update of attachment 108368 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=108368&action=review
> 
> > Source/WebCore/inspector/InspectorInstrumentation.cpp:-602
> > -    if (frame->page()->mainFrame() != frame || url != documentLoader->requestURL())
> 
> Do we receive load event notifications only for the frame main resource? If not, the check should be preserved.

loadEventFired() and domContentLoadedEventFired() are called only from DOMWindow and Document always passing frame main resource url.
Comment 11 Vsevolod Vlasov 2011-09-23 03:46:29 PDT
Committed r95790: <http://trac.webkit.org/changeset/95790>