Bug 23973 - Chromium crashes at times when the view is being closed.
Summary: Chromium crashes at times when the view is being closed.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Ananta Iyengar
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-02-16 08:54 PST by Ananta Iyengar
Modified: 2009-02-26 11:33 PST (History)
3 users (show)

See Also:


Attachments
Initial attempt at a patch for this issue (1.40 KB, patch)
2009-02-16 10:04 PST, Ananta Iyengar
fishd: review-
Details | Formatted Diff | Diff
Updated patch with the indentation fixed. (1.46 KB, patch)
2009-02-26 11:22 PST, Ananta Iyengar
fishd: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ananta Iyengar 2009-02-16 08:54:43 PST
We don't have a definite reproducible case for this crash. While the crash occurs in Chromium, the callstack suggests that this could in other webkit browsers as well. The callstack is as below:-

This occurs when the view is being closed and the scrollcontents method in the ScrollView object is called. It attempts to call the repaint method on the HostWindow object returned via FrameView::hostWindow. This method can return NULL in scenarios where the frame is NULL, or if the underlying page is destroyed. I think that a NULL check for the return value of the FrameView::hostWindow call is warranted. 

I will submit a patch for this in a bit.

Thread 1 *CRASHED* (EXCEPTION_ACCESS_VIOLATION @0x00000000)

0x62d3789e	 [chrome.dll	 - scrollview.cpp:438]	 WebCore::ScrollView::scrollContents(WebCore::IntSize const &)
0x62d37831	 [chrome.dll	 - scrollview.cpp:420]	 WebCore::ScrollView::updateScrollbars(WebCore::IntSize const &)
0x62d37009	 [chrome.dll	 - scrollview.cpp:116]	 WebCore::ScrollView::setScrollbarModes(WebCore::ScrollbarMode,WebCore::ScrollbarMode)
0x62d5d8e4	 [chrome.dll	 - frameview.cpp:231]	 WebCore::FrameView::resetScrollbars()
0x62d5d7f8	 [chrome.dll	 - frameview.cpp:197]	 WebCore::FrameView::~FrameView()
0x62d5d6d6	 [chrome.dll	 + 0x003ed6d6]	 WebCore::FrameView::`vector deleting destructor'(unsigned int)
0x62e309d8	 [chrome.dll	 - renderpart.cpp:56]	 WebCore::RenderPart::~RenderPart()
0x62e30998	 [chrome.dll	 + 0x004c0998]	 WebCore::RenderPart::`vector deleting destructor'(unsigned int)
0x62d6885f	 [chrome.dll	 - renderobject.cpp:2670]	 WebCore::RenderObject::arenaDelete(WebCore::RenderArena *,void *)
0x62dadd8b	 [chrome.dll	 - renderwidget.cpp:213]	 WebCore::RenderWidget::deref(WebCore::RenderArena *)
0x62dadabc	 [chrome.dll	 - renderwidget.cpp:103]	 WebCore::RenderWidget::destroy()
0x62d18d19	 [chrome.dll	 - node.cpp:1060]	 WebCore::Node::detach()
0x62d55b4c	 [chrome.dll	 - containernode.cpp:593]	 WebCore::ContainerNode::detach()
0x62d55b4c	 [chrome.dll	 - containernode.cpp:593]	 WebCore::ContainerNode::detach()
0x62d55b4c	 [chrome.dll	 - containernode.cpp:593]	 WebCore::ContainerNode::detach()
0x62d55b4c	 [chrome.dll	 - containernode.cpp:593]	 WebCore::ContainerNode::detach()
0x62cfd86b	 [chrome.dll	 - document.cpp:1412]	 WebCore::Document::detach()
0x62ce59d4	 [chrome.dll	 - frame.cpp:217]	 WebCore::Frame::setView(WebCore::FrameView *)
0x62cf23a2	 [chrome.dll	 - frameloader.cpp:3474]	 WebCore::FrameLoader::detachFromParent()
0x62cf22d8	 [chrome.dll	 - frameloader.cpp:3452]	 WebCore::FrameLoader::frameDetached()
0x62cb57a4	 [chrome.dll	 - webview_impl.cc:776]	 WebViewImpl::Close()
0x62984cb2	 [chrome.dll	 - render_widget.cc:639]	 RenderWidget::Close()
0x62984236	 [chrome.dll	 - render_widget.cc:225]	 RenderWidget::OnClose()
0x629803d7	 [chrome.dll	 - ipc_message.h:125]	 IPC::Message::Dispatch<RenderWidget>(IPC::Message const *,RenderWidget *,void ( RenderWidget::*)(void))
0x62984007	 [chrome.dll	 - render_widget.cc:157]	 RenderWidget::OnMessageReceived(IPC::Message const &)
0x62979fc3	 [chrome.dll	 - render_view.cc:400]	 RenderView::OnMessageReceived(IPC::Message const &)
0x62997262	 [chrome.dll	 - message_router.cc:39]	 MessageRouter::RouteMessage(IPC::Message const &)
0x6299723c	 [chrome.dll	 - message_router.cc:30]	 MessageRouter::OnMessageReceived(IPC::Message const &)
0x62976ba4	 [chrome.dll	 - render_thread.cc:174]	 RenderThread::OnMessageReceived(IPC::Message const &)
0x62ae6dab	 [chrome.dll	 - task.h:312]	 RunnableMethod<SafeBrowsingService,void ( SafeBrowsingService::*)(SafeBrowsingService::BlockingPageParam const &),Tuple1<SafeBrowsingService::BlockingPageParam> >::Run()
0x62b56ebc	 [chrome.dll	 - message_loop.cc:308]	 MessageLoop::RunTask(Task *)
0x62b56ef3	 [chrome.dll	 - message_loop.cc:316]	 MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const &)
0x62b57081	 [chrome.dll	 - message_loop.cc:408]	 MessageLoop::DoWork()
0x62b6dccf	 [chrome.dll	 - message_pump_default.cc:50]	 base::MessagePumpDefault::Run(base::MessagePump::Delegate *)
0x62b56d8d	 [chrome.dll	 - message_loop.cc:197]	 MessageLoop::RunInternal()
0x62b56d5a	 [chrome.dll	 - message_loop.cc:180]	 MessageLoop::RunHandler()
0x62b56cfd	 [chrome.dll	 - message_loop.cc:154]	 MessageLoop::Run()
0x62b5cfdf	 [chrome.dll	 - thread.cc:153]	 base::Thread::ThreadMain()
0x62b5a8a9	 [chrome.dll	 - platform_thread_win.cc:26]	 `anonymous namespace'::ThreadFunc(void *)
0x76fb4910	 [kernel32.dll	 + 0x00044910]	 BaseThreadInitThunk
0x7711e4b5	 [ntdll.dll	 + 0x0003e4b5]	 __RtlUserThreadStart
0x7711e488	 [ntdll.dll	 + 0x0003e488]	 _RtlUserThreadStart
Comment 1 Ananta Iyengar 2009-02-16 10:04:13 PST
Created attachment 27699 [details]
Initial attempt at a patch for this issue
Comment 2 Jon@Chromium 2009-02-25 15:47:50 PST
I need to get this reviewed so we can (hopefully) close the bug.

Also reported in http://code.google.com/p/chromium/issues/detail?id=6319
Comment 3 Darin Fisher (:fishd, Google) 2009-02-26 11:00:27 PST
Comment on attachment 27699 [details]
Initial attempt at a patch for this issue

>Index: WebCore/platform/ScrollView.cpp
...
> void ScrollView::scrollContents(const IntSize& scrollDelta)
> {
>+    if (!hostWindow())
>+      return;
>+

nit: the return statement should be indented by 4 spaces.  R- because
of this.

ScrollView appears to be designed with the assumption that the hostWindow
will never be null, but I can see from the callstack that assumption seems
to be invalid.  So, I think your solution is probably the best we can do.

Are any of the other ScrollView methods reachable from ~FrameView?
Comment 4 Ananta Iyengar 2009-02-26 11:22:54 PST
Created attachment 28024 [details]
Updated patch with the indentation fixed.

The functions in ScrollView.cpp reachable from ~FrameView are as below:-
1. ScrollView::setScrollbarModes which calls into updateScrollbars and ends up   
   in scrollcontents where hostwindow is dereferenced.

2. ScrollView::setHasHorizontalScrollbar and 
   ScrollView::setHasVerticalScrollbar.
   These functions internally call ScrollView::removeChild. They don't reference
   hostWindow().
Comment 5 Darin Fisher (:fishd, Google) 2009-02-26 11:25:09 PST
Comment on attachment 28024 [details]
Updated patch with the indentation fixed.

LGTM
Comment 6 Darin Fisher (:fishd, Google) 2009-02-26 11:33:32 PST
Landed as http://trac.webkit.org/changeset/41260