HistoryController shouldn't call confusingAndOftenMisusedAttached

HistoryController shouldn't call confusingAndOftenMisusedAttached

It's not entirely clear to me why this check is needed at all.  I tried just
removing it entirely but that caused a test to fail. Given that we just got the
document from a frame, we must be worried about the case where we're in the
middle of unloading the document.

Regardless, checking whether the document has a RenderView is more direct than
asking whether it is "attached".

R=japhet,esprehn
BUG=299011
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=158515

[esprehn, 2013-09-27 18:23:45]

lgtm

https://codereview.chromium.org/24976002/diff/1/Source/core/loader/HistoryCon...
File Source/core/loader/HistoryController.cpp (right):

https://codereview.chromium.org/24976002/diff/1/Source/core/loader/HistoryCon...
Source/core/loader/HistoryController.cpp:126: if
(m_currentItem->isCurrentDocument(document) && document->renderer()) {
I usually use renderView() since it's more clear, we should probably make
Document::renderer private.

[abarth-chromium, 2013-09-27 18:26:12]

https://codereview.chromium.org/24976002/diff/1/Source/core/loader/HistoryCon...
File Source/core/loader/HistoryController.cpp (right):

https://codereview.chromium.org/24976002/diff/1/Source/core/loader/HistoryCon...
Source/core/loader/HistoryController.cpp:126: if
(m_currentItem->isCurrentDocument(document) && document->renderer()) {
On 2013/09/27 18:23:45, esprehn wrote:
> I usually use renderView() since it's more clear, we should probably make
> Document::renderer private.

Ok.  That will require me to pull in another header (because it's defined in
RenderView.h), but that's fine.

[esprehn, 2013-09-27 18:32:00]

On 2013/09/27 18:26:12, abarth wrote:
>
https://codereview.chromium.org/24976002/diff/1/Source/core/loader/HistoryCon...
> File Source/core/loader/HistoryController.cpp (right):
> 
>
https://codereview.chromium.org/24976002/diff/1/Source/core/loader/HistoryCon...
> Source/core/loader/HistoryController.cpp:126: if
> (m_currentItem->isCurrentDocument(document) && document->renderer()) {
> On 2013/09/27 18:23:45, esprehn wrote:
> > I usually use renderView() since it's more clear, we should probably make
> > Document::renderer private.
> 
> Ok.  That will require me to pull in another header (because it's defined in
> RenderView.h), but that's fine.

Oh yeah... whatever you think is best then. Maybe Document needs something like
isRendered() that checks renderer().

[abarth-chromium, 2013-09-27 20:34:51]

On 2013/09/27 18:32:00, esprehn wrote:
> Oh yeah... whatever you think is best then. Maybe Document needs something
like
> isRendered() that checks renderer().

IMHO, the best thing would be if we had an explicit state machine to query.  I'm
adding one in:

https://codereview.chromium.org/25036004/

[abarth-chromium, 2013-09-30 05:06:29]

https://codereview.chromium.org/24976002/diff/9001/Source/core/loader/History...
File Source/core/loader/HistoryController.cpp (right):

https://codereview.chromium.org/24976002/diff/9001/Source/core/loader/History...
Source/core/loader/HistoryController.cpp:126: if
(m_currentItem->isCurrentDocument(document) && document->isActive()) {
I updated this CL to use the new hotness.

[commit-bot: I haz the power, 2013-09-30 05:06:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/abarth@chromium.org/24976002/9001

[commit-bot: I haz the power, 2013-09-30 06:24:51]

Change committed as 158515