Patch coming momentarily <rdar://problem/18550647>
Created attachment 240443 [details] Patch
Comment on attachment 240443 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240443&action=review > Source/WebCore/loader/FrameLoader.cpp:192 > - ASSERT(!m_inProgress || m_frame.page()); > - if (m_inProgress) > + ASSERT(!m_inProgress); This makes the ASSERT more strict, what guarantees that m_inProgress is always false here? As it's not part of the fix, maybe the change can even be landed separately first, so that bots can report on it (EWS only runs release).
(In reply to comment #2) > Comment on attachment 240443 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=240443&action=review > > > Source/WebCore/loader/FrameLoader.cpp:192 > > - ASSERT(!m_inProgress || m_frame.page()); > > - if (m_inProgress) > > + ASSERT(!m_inProgress); > > This makes the ASSERT more strict, what guarantees that m_inProgress is > always false here? Ah...that wasn't actually intentional. While it will usually be the case, the null pointer case I'm trying to fix will have m_inProgress be false. The assertion should be removed entirely, it seems.
I did run the full onslaught of tests, which didn't see it being false, but I have backtraces that show that it can be (as that's when the null pointer is relevant).
Comment on attachment 240443 [details] Patch review-; please post a patch without the assertion
Created attachment 240508 [details] Patch
Comment on attachment 240508 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240508&action=review > Source/WebCore/ChangeLog:13 > + No new tests; there is no behavior change, and it is impossible to > + reliably simulate the null pointer case without intrusive code changes. I don't think that this explanation is accurate. I believe that you expect to fix a crash, which is a behavior change. I assume that the reason why there is no patch is that we couldn't find a way to make one.
(In reply to comment #7) > Comment on attachment 240508 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=240508&action=review > > > Source/WebCore/ChangeLog:13 > > + No new tests; there is no behavior change, and it is impossible to > > + reliably simulate the null pointer case without intrusive code changes. > > I don't think that this explanation is accurate. I believe that you expect > to fix a crash, which is a behavior change. > > I assume that the reason why there is no patch is that we couldn't find a > way to make one. I can remove the "no behavior change" part, but the only way to simulate this would be to have a page that gets removed from the page cache before the load has committed, I believe, which I don't think we have a way of doing with layout tests.
Committed r175277: <http://trac.webkit.org/changeset/175277>