Title: [186683] trunk/Source/WebCore
- Revision
- 186683
- Author
- beid...@apple.com
- Date
- 2015-07-10 12:01:51 -0700 (Fri, 10 Jul 2015)
Log Message
Crash in HistoryController::updateForCommit dereferencing a null HistoryItem.
<rdar://problem/21371589> and https://bugs.webkit.org/show_bug.cgi?id=146842
Reviewed by Chris Dumez.
No new tests (Unknown how to reproduce).
This patch basically rolls back part of http://trac.webkit.org/changeset/179472.
r179472 changed HistoryController::setCurrentItem() to take a reference instead of a pointer.
Unfortunately, we sometimes call setCurrentItem(nullptr).
We'd like to *not* do that, and there are assertions in place to try to catch when we do,
but in the meantime it is not valid to dereference nullptr.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadSameDocumentItem):
* loader/HistoryController.cpp:
(WebCore::HistoryController::updateForCommit):
(WebCore::HistoryController::recursiveUpdateForCommit):
(WebCore::HistoryController::recursiveUpdateForSameDocumentNavigation):
(WebCore::HistoryController::setCurrentItem): Take a ptr instead of a ref.
(WebCore::HistoryController::createItem):
* loader/HistoryController.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (186682 => 186683)
--- trunk/Source/WebCore/ChangeLog 2015-07-10 17:53:55 UTC (rev 186682)
+++ trunk/Source/WebCore/ChangeLog 2015-07-10 19:01:51 UTC (rev 186683)
@@ -1,3 +1,31 @@
+2015-07-10 Brady Eidson <beid...@apple.com>
+
+ Crash in HistoryController::updateForCommit dereferencing a null HistoryItem.
+ <rdar://problem/21371589> and https://bugs.webkit.org/show_bug.cgi?id=146842
+
+ Reviewed by Chris Dumez.
+
+ No new tests (Unknown how to reproduce).
+
+ This patch basically rolls back part of http://trac.webkit.org/changeset/179472.
+
+ r179472 changed HistoryController::setCurrentItem() to take a reference instead of a pointer.
+ Unfortunately, we sometimes call setCurrentItem(nullptr).
+
+ We'd like to *not* do that, and there are assertions in place to try to catch when we do,
+ but in the meantime it is not valid to dereference nullptr.
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::loadSameDocumentItem):
+
+ * loader/HistoryController.cpp:
+ (WebCore::HistoryController::updateForCommit):
+ (WebCore::HistoryController::recursiveUpdateForCommit):
+ (WebCore::HistoryController::recursiveUpdateForSameDocumentNavigation):
+ (WebCore::HistoryController::setCurrentItem): Take a ptr instead of a ref.
+ (WebCore::HistoryController::createItem):
+ * loader/HistoryController.h:
+
2015-07-10 Javier Fernandez <jfernan...@igalia.com>
[CSS Grid Layout] Grid item's auto-margins are not applied correctly
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (186682 => 186683)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2015-07-10 17:53:55 UTC (rev 186682)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2015-07-10 19:01:51 UTC (rev 186683)
@@ -3181,7 +3181,7 @@
if (FrameView* view = m_frame.view())
view->setWasScrolledByUser(false);
- history().setCurrentItem(item);
+ history().setCurrentItem(&item);
// loadInSameDocument() actually changes the URL and notifies load delegates of a "fake" load
loadInSameDocument(item.url(), item.stateObject(), false);
Modified: trunk/Source/WebCore/loader/HistoryController.cpp (186682 => 186683)
--- trunk/Source/WebCore/loader/HistoryController.cpp 2015-07-10 17:53:55 UTC (rev 186682)
+++ trunk/Source/WebCore/loader/HistoryController.cpp 2015-07-10 19:01:51 UTC (rev 186683)
@@ -470,8 +470,13 @@
// the provisional item for restoring state.
// Note previousItem must be set before we close the URL, which will
// happen when the data source is made non-provisional below
+
+ // FIXME: https://bugs.webkit.org/show_bug.cgi?id=146842
+ // We should always have a provisional item when committing, but we sometimes don't.
+ // Not having one leads to us not having a m_currentItem later, which is also a terrible known issue.
+ // We should get to the bottom of this.
ASSERT(m_provisionalItem);
- setCurrentItem(*m_provisionalItem);
+ setCurrentItem(m_provisionalItem.get());
m_provisionalItem = nullptr;
// Tell all other frames in the tree to commit their provisional items and
@@ -512,7 +517,7 @@
view->setWasScrolledByUser(false);
// Now commit the provisional item
- setCurrentItem(*m_provisionalItem);
+ setCurrentItem(m_provisionalItem.get());
m_provisionalItem = nullptr;
// Restore form state (works from currentItem)
@@ -561,7 +566,7 @@
return;
// Commit the provisional item.
- setCurrentItem(*m_provisionalItem);
+ setCurrentItem(m_provisionalItem.get());
m_provisionalItem = nullptr;
// Iterate over the rest of the tree.
@@ -577,11 +582,11 @@
m_frameLoadComplete = true;
}
-void HistoryController::setCurrentItem(HistoryItem& item)
+void HistoryController::setCurrentItem(HistoryItem* item)
{
m_frameLoadComplete = false;
m_previousItem = m_currentItem;
- m_currentItem = &item;
+ m_currentItem = item;
}
void HistoryController::setCurrentItemTitle(const StringWithDirection& title)
@@ -664,7 +669,7 @@
initializeItem(item);
// Set the item for which we will save document state
- setCurrentItem(item);
+ setCurrentItem(item.ptr());
return item;
}
Modified: trunk/Source/WebCore/loader/HistoryController.h (186682 => 186683)
--- trunk/Source/WebCore/loader/HistoryController.h 2015-07-10 17:53:55 UTC (rev 186682)
+++ trunk/Source/WebCore/loader/HistoryController.h 2015-07-10 19:01:51 UTC (rev 186683)
@@ -74,7 +74,7 @@
void updateForFrameLoadCompleted();
HistoryItem* currentItem() const { return m_currentItem.get(); }
- void setCurrentItem(HistoryItem&);
+ void setCurrentItem(HistoryItem*);
void setCurrentItemTitle(const StringWithDirection&);
bool currentItemShouldBeReplaced() const;
WEBCORE_EXPORT void replaceCurrentItem(HistoryItem*);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
