Keep frames to navigate alive in HistoryController::goToEntry()

Keep frames to navigate alive in HistoryController::goToEntry()

Otherwise, if a script executed during the navigation of one frame (for
instance a popstate event handler) removes other frames from the document,
the calls to navigate those frames later on are given pointers to dead
objects.

This patch makes the more localized fix committed as r165081 redundant,
and thus reverts it from FrameLoader::loadHistoryItem().

BUG=337120
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=165724

[Jens Widell, 2014-01-23 12:18:29]

Hi arv, an improved/extended version of my previous popstate-nukes-iframe crash
fix.

[Nate Chapin, 2014-01-23 18:19:11]

https://codereview.chromium.org/145493005/diff/1/Source/core/page/HistoryCont...
File Source/core/page/HistoryController.cpp (right):

https://codereview.chromium.org/145493005/diff/1/Source/core/page/HistoryCont...
Source/core/page/HistoryController.cpp:192:
it->key->loader().loadHistoryItem(it->value->item(), HistorySameDocumentLoad,
cachePolicy);
Rather than checking m_client in loadHistoryItem(), we could just null-check
it->key->host() here.

Conceptually, we really shouldn't be trying to start a load if the frame is
detached, so maybe we should then ASSERT we're attached at the start of
loadHistoryItem().

https://codereview.chromium.org/145493005/diff/1/Source/core/page/HistoryCont...
File Source/core/page/HistoryController.h (right):

https://codereview.chromium.org/145493005/diff/1/Source/core/page/HistoryCont...
Source/core/page/HistoryController.h:159: typedef HashMap<Frame*,
OwnPtr<HistoryFrameLoad> > HistoryFrameLoadSet;
Can we change this to Hashmap<RefPtr<Frame>, HistoryItem*> instead of adding the
HistoryFrameLoad wrapper?

[Jens Widell, 2014-01-23 18:52:18]

Patch updated.

https://codereview.chromium.org/145493005/diff/1/Source/core/page/HistoryCont...
File Source/core/page/HistoryController.h (right):

https://codereview.chromium.org/145493005/diff/1/Source/core/page/HistoryCont...
Source/core/page/HistoryController.h:159: typedef HashMap<Frame*,
OwnPtr<HistoryFrameLoad> > HistoryFrameLoadSet;
On 2014/01/23 18:19:11, Nate Chapin wrote:
> Can we change this to Hashmap<RefPtr<Frame>, HistoryItem*> instead of adding
the
> HistoryFrameLoad wrapper?

Certainly; much better that way.

I think I assumed RefPtr<> couldn't be the key in a HashMap<>.  Unclear why. 
:-)

[Nate Chapin, 2014-01-23 18:55:33]

LGTM, thanks!

[commit-bot: I haz the power, 2014-01-23 18:57:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jl@opera.com/145493005/70001

[commit-bot: I haz the power, 2014-01-23 23:26:26]

Retried try job too often on linux_blink for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[commit-bot: I haz the power, 2014-01-24 06:34:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jl@opera.com/145493005/70001

[commit-bot: I haz the power, 2014-01-24 09:14:13]

Change committed as 165724