Bug 45790 – ASSERTION FAILED: m_loadEventDelayCount

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Tony Gentilcore]

On the GTK linux 32 bot, we are hitting this ASSERT occasionally.

ASSERTION FAILED: m_loadEventDelayCount
(../../WebCore/dom/Document.cpp:4785 void WebCore::Document::decrementLoadEventDelayCount())

An example of a test that has triggered it is:
fast/dom/HTMLScriptElement/script-decoding-error-after-setting-src.html

It is possible this isn't really port-specific, but just happens to have triggered there. This was almost certainly introduced by https://bugs.webkit.org/show_bug.cgi?id=45310

Stack:
#0  0x55aa3d98 in WebCore::Document::decrementLoadEventDelayCount (
    this=0xc62b788) at ../../WebCore/dom/Document.cpp:4750
#1  0x55a75b5c in WebCore::AsyncScriptRunner::timerFired (this=0x956d260, 
    timer=0x956d270) at ../../WebCore/dom/AsyncScriptRunner.cpp:87
#2  0x55a7652a in WebCore::Timer<WebCore::AsyncScriptRunner>::fired (
    this=0x956d270) at ../../WebCore/platform/Timer.h:98
#3  0x55ea42e8 in WebCore::ThreadTimers::sharedTimerFiredInternal (
    this=0x8148730) at ../../WebCore/platform/ThreadTimers.cpp:112
#4  0x55ea422f in WebCore::ThreadTimers::sharedTimerFired ()
    at ../../WebCore/platform/ThreadTimers.cpp:90
#5  0x561cd0ad in timeout_cb ()
    at ../../WebCore/platform/gtk/SharedTimerGtk.cpp:49
#6  0x5923da6c in g_timeout_dispatch (source=0x954c3b0, callback=0xbbadbeef, 
    user_data=0x0)
    at /build/buildd-glib2.0_2.24.1-1-i386-84Pp4V/glib2.0-2.24.1/glib/gmain.c:3396
#7  0x5923d2f5 in g_main_dispatch (context=0x8094b68)
    at /build/buildd-glib2.0_2.24.1-1-i386-84Pp4V/glib2.0-2.24.1/glib/gmain.c:1960
#8  IA__g_main_context_dispatch (context=0x8094b68)
    at /build/buildd-glib2.0_2.24.1-1-i386-84Pp4V/glib2.0-2.24.1/glib/gmain.c:2513
#9  0x59240fd8 in g_main_context_iterate (context=0x8094b68, 
    block=<value optimized out>, dispatch=1, self=0x8073268)
    at /build/buildd-glib2.0_2.24.1-1-i386-84Pp4V/glib2.0-2.24.1/glib/gmain.c:2591
#10 0x59241517 in IA__g_main_loop_run (loop=0xc636eb0)
    at /build/buildd-glib2.0_2.24.1-1-i386-84Pp4V/glib2.0-2.24.1/glib/gmain.c:2799
#11 0x58cebdc9 in IA__gtk_main ()
    at /build/buildd-gtk+2.0_2.20.1-1-i386-Ixfflh/gtk+2.0-2.20.1/gtk/gtkmain.c:1219
#12 0x0805c451 in runTest (testPathOrURL=...)
    at ../../WebKitTools/DumpRenderTree/gtk/DumpRenderTree.cpp:573
#13 0x0805bba7 in runTestingServerLoop ()
    at ../../WebKitTools/DumpRenderTree/gtk/DumpRenderTree.cpp:383
#14 0x0805d879 in main (argc=2, argv=0xffffc524)
    at ../../WebKitTools/DumpRenderTree/gtk/DumpRenderTree.cpp:1013

[Tony Gentilcore]

Created attachment 67823 [details]
Patch

[Adam Barth]

Comment on attachment 67823 [details]
Patch

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

> WebCore/WebCore.xcodeproj/project.pbxproj:2388
> -		893C47DF123EF4A9002B3D86 /* JSDirectoryEntryCustom.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 893C47DE123EF4A9002B3D86 /* JSDirectoryEntryCustom.cpp */; };
>  		893C47CC123EEBA2002B3D86 /* JSEntryCustom.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 893C47CA123EEBA2002B3D86 /* JSEntryCustom.cpp */; };
> +		893C47DF123EF4A9002B3D86 /* JSDirectoryEntryCustom.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 893C47DE123EF4A9002B3D86 /* JSDirectoryEntryCustom.cpp */; };

These changes seems spurious.  Please land without them.  If the project file is wrong, we can fix it in a separate commit.

> WebCore/dom/AsyncScriptRunner.cpp:81
> +    RefPtr<Document> document(m_document);

Usually we'd call this variable protector and then just m_document through the function.

> WebCore/dom/AsyncScriptRunner.cpp:87
>          scripts[i].first->execute(scripts[i].second.get());

Does this execute script when the document is detached from the frame?  It's an invariant that we never execute scripts on behalf of detached documents.  (Not really an issue for this patch, just something to think about.)

[Tony Gentilcore]

> These changes seems spurious.  Please land without them.  If the project file is wrong, we can fix it in a separate commit.

OK

> Usually we'd call this variable protector and then just m_document through the function.

Ah, right.

But I went with protect over protector.
$ grep -e "protect[(]" * -r | wc -l
     108
$ grep -e "protector[(]" * -r | wc -l
      39

> Does this execute script when the document is detached from the frame?  It's an invariant that we never execute scripts on behalf of detached documents.  (Not really an issue for this patch, just something to think about.)

I'll try to add a test and look into it separately.

[Tony Gentilcore]

Created attachment 67837 [details]
Patch for landing

[Alexey Proskuryakov]

Yes, protect(something) reads better than protector(something), as long as you're never using the protector object.

[Adam Barth]

(In reply to comment #5)
> Yes, protect(something) reads better than protector(something), as long as you're never using the protector object.

/me look around to see who wants to fix the 39 stray occurrences.

[Adam Barth]

Comment on attachment 67837 [details]
Patch for landing

Clearing flags on attachment: 67837

Committed r67728: <http://trac.webkit.org/changeset/67728>

[Adam Barth]

All reviewed patches have been landed.  Closing bug.