[EFL] fast/frames/frame-crash-with-page-cache.html is crashing

[Thiago Marcos P. Santos]

Test started to crash after bug 85620 was landed.

[Thiago Marcos P. Santos]

*** Bug 85981 has been marked as a duplicate of this bug. ***

[Thiago Marcos P. Santos]

Bug 85608 has nothing to do with the crash. If you revert it, it will crash anyway. Although we should investigate if closeRemainingWindowsWhenComplete() really needs to be checked before cleaning the extra views.

[Thiago Marcos P. Santos]

(In reply to comment #2)
> Bug 85608 has nothing to do with the crash. If you revert it, it will crash anyway. Although we should investigate if closeRemainingWindowsWhenComplete() really needs to be checked before cleaning the extra views.

Not really, it was set to True. So even verifying the flag, the test would still crash.

[Mikhail Pozdnyakov]

Backtrace:

1480        ASSERT(!m_frame->document() || !m_frame->document()->inPageCache());
(gdb) bt
#0  0xb404af53 in WebCore::FrameLoader::stopAllLoaders (this=0xae7db1cc, clearProvisionalItemPolicy=WebCore::ShouldClearProvisionalItem)
    at /usr/share/WebKit/Source/WebCore/loader/FrameLoader.cpp:1480
#1  0xb404e41c in WebCore::FrameLoader::detachFromParent (this=0xae7db1cc) at /usr/share/WebKit/Source/WebCore/loader/FrameLoader.cpp:2325
#2  0xb1e333f0 in _ewk_frame_smart_del (ewkFrame=0xaf3d30c0) at /usr/share/WebKit/Source/WebKit/efl/ewk/ewk_frame.cpp:232
#3  0xb7e03e00 in evas_object_smart_del (obj=0xaf3d30c0) at evas_object_smart.c:747
#4  0xb7df7435 in evas_object_del (obj=0xaf3d30c0) at evas_object_main.c:437
#5  0xb7e02770 in _evas_object_smart_members_all_del (obj=0xaf3ceed8) at evas_object_smart.c:299
#6  0xb7e044e8 in evas_object_smart_clipped_smart_del (obj=0xaf3ceed8) at evas_object_smart_clipped.c:79
#7  0xb1e33448 in _ewk_frame_smart_del (ewkFrame=0xaf3ceed8) at /usr/share/WebKit/Source/WebKit/efl/ewk/ewk_frame.cpp:242
#8  0xb7e03e00 in evas_object_smart_del (obj=0xaf3ceed8) at evas_object_smart.c:747
#9  0xb7df7435 in evas_object_del (obj=0xaf3ceed8) at evas_object_main.c:437
#10 0xb7e02770 in _evas_object_smart_members_all_del (obj=0xaf3ce7e8) at evas_object_smart.c:299
#11 0xb7e044e8 in evas_object_smart_clipped_smart_del (obj=0xaf3ce7e8) at evas_object_smart_clipped.c:79
#12 0xb1e54f76 in _ewk_view_smart_del (ewkView=0xaf3ce7e8) at /usr/share/WebKit/Source/WebKit/efl/ewk/ewk_view.cpp:896
#13 0xb7e03e00 in evas_object_smart_del (obj=0xaf3ce7e8) at evas_object_smart.c:747
#14 0xb7df7435 in evas_object_del (obj=0xaf3ce7e8) at evas_object_main.c:437
#15 0x0809fc1c in DumpRenderTreeChrome::clearExtraViews (this=0x8161638) at /usr/share/WebKit/Tools/DumpRenderTree/efl/DumpRenderTreeChrome.cpp:174
#16 0x0809d4c8 in runTest (cTestPathOrURL=0xbffff4c1 "LayoutTests/fast/frames/frame-c

[Mikhail Pozdnyakov]

Crash is lead by ewk_frame freeing functions.

loader()->detachFromParent() should be applied only for the main frame, not for each frame as traverses trough the frame tree.

loader()->cancelAndClear() resets frame's document which is not appropriate for the cached frame, not needed because detachFromParent makes clean up already.

[Mikhail Pozdnyakov]

(In reply to comment #5)
> Crash is lead by ewk_frame freeing functions.

loader()->detachFromParent() and loader()->cancelAndClear() are invoked from _ewk_frame_smart_del()

[Mikhail Pozdnyakov]

Created attachment 141005 [details]
preliminary patch

[Gyuyoung Kim]

Comment on attachment 141005 [details]
preliminary patch

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

Looks fine overall except for variable name.

> Source/WebKit/efl/ewk/ewk_frame.cpp:232
> +            EWK_FRAME_SD_GET(ewk_view_frame_main_get(smartData->view), msd);

Please use full name for variable. For example, mainSmartData instead of msd ?

[Mikhail Pozdnyakov]

Created attachment 141114 [details]
patch

[Mikhail Pozdnyakov]

(In reply to comment #8)
> (From update of attachment 141005 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=141005&action=review
> 
> Looks fine overall except for variable name.
> 
> > Source/WebKit/efl/ewk/ewk_frame.cpp:232
> > +            EWK_FRAME_SD_GET(ewk_view_frame_main_get(smartData->view), msd);
> 
> Please use full name for variable. For example, mainSmartData instead of msd ?

Thanks for taking a look! Fixed.

[Gyuyoung Kim]

Comment on attachment 141114 [details]
patch

Looks good to me.

[WebKit Review Bot]

Comment on attachment 141114 [details]
patch

Rejecting attachment 141114 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
ests/platform/efl/test_expectations.txt
Hunk #1 FAILED at 398.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/efl/test_expectations.txt.rej
patching file Source/WebKit/efl/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebKit/efl/ewk/ewk_frame.cpp
Hunk #1 succeeded at 233 (offset 4 lines).

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Noam Rosen..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/12709612

[Mikhail Pozdnyakov]

Created attachment 142421 [details]
to be landed.

[WebKit Review Bot]

Comment on attachment 142421 [details]
to be landed.

Clearing flags on attachment: 142421

Committed r117409: <http://trac.webkit.org/changeset/117409>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.