Bug 131652

Summary: Remove unnecessary null checking in NavigatorContentUtils
Product: WebKit Reporter: Gyuyoung Kim <gyuyoung.kim>
Component: New BugsAssignee: Gyuyoung Kim <gyuyoung.kim>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Gyuyoung Kim 2014-04-14 17:38:38 PDT
According to my investigation, it looks there is no change that document can be null.So, it seems to me that we don't need to check if document is null when frame is existed. 

In Frame.cpp, m_doc is set in Frame::setDocument(). I think m_doc can't be null. Even when I try to find same null check, I couldn't find it in WebKit.

void Frame::setDocument(PassRefPtr<Document> newDocument)
{
    ASSERT(!newDocument || newDocument->frame() == this);

    if (m_doc && m_doc->hasLivingRenderTree() && !m_doc->inPageCache())
        m_doc->prepareForDestruction();

    m_doc = newDocument.get();
    ASSERT(!m_doc || m_doc->domWindow());
    ASSERT(!m_doc || m_doc->domWindow()->frame() == this);

    // Don't use m_doc because it can be overwritten and we want to guarantee
    // that the document is not destroyed during this function call.
    if (newDocument)
        newDocument->didBecomeCurrentDocumentInFrame();
}
Comment 1 Gyuyoung Kim 2014-04-14 17:39:44 PDT
Created attachment 229330 [details]
Patch
Comment 2 Gyuyoung Kim 2014-04-14 17:40:34 PDT
CC'ing Darin, Darin, if frame()->document() can be null, please let me know.
Comment 3 Darin Adler 2014-04-15 07:37:15 PDT
Comment on attachment 229330 [details]
Patch

Frame::document is mostly non-null. If it was truly always non-null then we (Kling) would have changed it to a Document& instead of a Document* and it would be clear it doesn’t need null checks.

I think the only times it can be null are very early in the frame lifetime, so this patch is OK.

But really our goal for the project is to use references for things that can’t be null so that such things aren’t a matter of style, instead just making it impossible to include extra null checks.
Comment 4 WebKit Commit Bot 2014-04-15 09:19:35 PDT
Comment on attachment 229330 [details]
Patch

Clearing flags on attachment: 229330

Committed r167308: <http://trac.webkit.org/changeset/167308>
Comment 5 WebKit Commit Bot 2014-04-15 09:19:41 PDT
All reviewed patches have been landed.  Closing bug.