[Webkit-unassigned] [Bug 43152] Assertion failure in FrameView::layout when modifying the DOM during pagehide with PageCache enabled

[bugzilla-daemon at webkit.org]

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


Mihai Parparita <mihaip at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1




--- Comment #6 from Mihai Parparita <mihaip at chromium.org>  2010-07-29 16:42:46 PST ---
As best as I can tell, the code path for invoking pagehide event handlers when the page cache is not enabled is:

- FrameLoader::commitProvisionalLoad
  - FrameLoader::transitionToCommitted
    - FrameLoader::closeURL
      - FrameLoader::stopLoading(UnloadEventPolicyUnloadAndPageHide)
          - DOMWindow::dispatchEvent(pageHideEvent)
    - WebFrameLoaderClient::transitionToCommittedForNewPage
      - Frame::SetView(0)
        - FrameView::unscheduleRelayout

While when the page cache is enabled, it is:

- FrameLoader::commitProvisionalLoad
  - PageCache::add
    - CachedPage::create
      - CachedFrame::create
        - FrameLoader::stopLoading(UnloadEventPolicyUnloadAndPageHide)
          - DOMWindow::dispatchEvent(pageHideEvent)
    - WebFrameLoaderClient::transitionToCommittedForNewPage
      - Frame::SetView(0)

(in the page cache case we call stopLoading with UnloadEventPolicyUnloadOnly in FrameLoader::closeURL, which is why they don't get invoked there)

The other difference is that later on, in Frame::SetView(0) (see how we get there above), there is this snippet:

235    // Detach the document now, so any onUnload handlers get run - if
236    // we wait until the view is destroyed, then things won't be
237    // hooked up enough for some JavaScript calls to work.
238    if (!view && m_doc && m_doc->attached() && !m_doc->inPageCache()) {
239        // FIXME: We don't call willRemove here. Why is that OK?
240        m_doc->detach();
241        if (m_view)
242            m_view->unscheduleRelayout();
243    }

If the document is in the page cache, not only do we not detach the document, but we don't unschedule pending layouts either. The assert goes away if the unscheduleRelayout call is moved to be outside the attached/page cache check. Generally that seems like a good thing (we don't care about pending layouts in any case if we're about to switch away from that view), but I don't know this code well enough.

This code was built up over time, so I'm not clear about the intention either (e.g. the comment about unload handlers seems wrong, since they don't get invoked by Document::detach). The relevant changes in this area are:
http://trac.webkit.org/changeset/19373
http://trac.webkit.org/changeset/25395
http://trac.webkit.org/changeset/25397

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