[Webkit-unassigned] [Bug 52819] Crash in WebCore::HistoryController::itemsAreClones

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=52819





--- Comment #24 from Charles Reis <creis at chromium.org>  2011-02-03 15:02:52 PST ---
(From update of attachment 81018)
View in context: https://bugs.webkit.org/attachment.cgi?id=81018&action=review

Thanks.  I've got an improved fix but my approach on the layout test isn't going to work.  (DumpRenderTree doesn't show error pages in either Chromium or WebKit.)  Thankfully, there's another set of repro steps in bug 53708.  I think I'll try to fix that issue here, since the layout test for that works here as well.

>>> Source/WebCore/loader/HistoryController.cpp:613
>>>  {
>> 
>> With item, you are potentially passing ownership, and it makes sense to use a PassRefPtr.  For fromItem, I only see a .get() comparison, so the PassRefPtr isn't needed.
> 
> Sorry for the commenting fail here - the inline review comments feature never does what I intend.  sigh.  Hopefully lining up the line numbers manually will make it obvious what I meant.

The bug was that m_provisionalItem and fromItem were the same, so that assigning item to m_provisionalItem deleted the old value (i.e., fromItem) while we still needed it.  I was using PassRefPtr for fromItem to prevent this, but you're right that it's incorrect.  All we need is to change currentItem to a RefPtr in goToItem (above) to ensure it doesn't get deleted here.

As you mentioned, we're potentially passing ownership of item, but we received it as a raw pointer anyway (in goToItem).  In retrospect, I don't think we need PassRefPtr even there.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.