Bug 131652 - Remove unnecessary null checking in NavigatorContentUtils
Summary: Remove unnecessary null checking in NavigatorContentUtils
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gyuyoung Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-14 17:38 PDT by Gyuyoung Kim
Modified: 2014-04-15 09:19 PDT (History)
2 users (show)

See Also:


Attachments
Patch (2.69 KB, patch)
2014-04-14 17:39 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.