[webkit-reviews] review denied: [Bug 15765] ASSERTION FAILED: m_frame->page() in FrameLoader::tokenizerProcessedData using the new GMail interface : [Attachment 18170] Remove the two asserts

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Holger Freyther
<freyther at handhelds.org>'s request for review:
Bug 15765: ASSERTION FAILED: m_frame->page() in
FrameLoader::tokenizerProcessedData using the new GMail interface
http://bugs.webkit.org/show_bug.cgi?id=15765

Attachment 18170: Remove the two asserts
http://bugs.webkit.org/attachment.cgi?id=18170&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
(In reply to comment #7)
> WebCore::Frame::pageDestroyed() is called because a Frame gets removed from
the
> FrameSet as the result of JavaScript code. But there were data for the
> HTMLTokenizer of the Document and the m_timer was started. The timer fires
> after we have removed the Frame from the FrameSet.

The new m_frame->page() assert is causing trouble, but the old
m_frame->document() assert is not. I see no reason to remove both because one
is failing.

What I would expect in a case like what you describe is that either
FrameLoader::stop or FrameLoader::stopLoading would get called. The surprise
here is that the frame can be removed without loading stopping.

> FrameLoader::tokenizerProcessedData calls checkCompleted() and
checkCompleted()
> is checking for if (m_frame->document()) and (m_frame->page()) at various
> places and I would argue that removing the asserts is fine as the methods may

> return 0 and checkCompleted() will continue to work

Some of those checks are for *after* checkCompleted has dispatched events like
the load event; they don't necessarily mean its good to call the function when
page is 0.

I think it might be OK to remove the assertion, but it would be better to
figure out why the load has not been stopped.