131652 2014-04-14 17:38:38 -0700 Remove unnecessary null checking in NavigatorContentUtils 2014-04-15 09:19:41 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 gyuyoung.kim gyuyoung.kim commit-queue darin oldest_to_newest 1000683 0 gyuyoung.kim 2014-04-14 17:38:38 -0700 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(); } 1000684 1 229330 gyuyoung.kim 2014-04-14 17:39:44 -0700 Created attachment 229330 Patch 1000685 2 gyuyoung.kim 2014-04-14 17:40:34 -0700 CC'ing Darin, Darin, if frame()->document() can be null, please let me know. 1000846 3 229330 darin 2014-04-15 07:37:15 -0700 Comment on attachment 229330 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. 1000858 4 229330 commit-queue 2014-04-15 09:19:35 -0700 Comment on attachment 229330 Patch Clearing flags on attachment: 229330 Committed r167308: <http://trac.webkit.org/changeset/167308> 1000859 5 commit-queue 2014-04-15 09:19:41 -0700 All reviewed patches have been landed. Closing bug. 229330 2014-04-14 17:39:44 -0700 2014-04-15 09:19:35 -0700 Patch bug-131652-20140415093922.patch text/plain 2754 gyuyoung.kim U3VidmVyc2lvbiBSZXZpc2lvbjogMTY3MjgzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNDY3OTMzZmUwYjUyYWNl NzZmYzY1ZDk2MDRkMjNmYTM4OGE0ZGVjMy4uYjZhZGVkZmIzN2IzNGZjZGI5MmRmOTkyMjVhMDBi MTIyOGMyMGIxYyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDE0LTA0LTE0ICBHeXV5 b3VuZyBLaW0gIDxneXV5b3VuZy5raW1Ac2Ftc3VuZy5jb20+CisKKyAgICAgICAgUmVtb3ZlIHVu bmVjZXNzYXJ5IG51bGwgY2hlY2tpbmcgaW4gTmF2aWdhdG9yQ29udGVudFV0aWxzCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMzE2NTIKKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBTb21lIGZ1bmN0aW9ucyBo YXZlIGNoZWNrZWQgaWYgZG9jdW1lbnQgaXMgbnVsbC4gSG93ZXZlciwgZG9jdW1lbnQgaXMgYWx3 YXlzIG5vdAorICAgICAgICBudWxsIHdoZW4gZnJhbWUgaXMgZXhpc3RlZC4KKworICAgICAgICBO byBuZXcgdGVzdHMsIG5vIGJlaGF2aW9yIGNoYW5nZXMuCisKKyAgICAgICAgKiBNb2R1bGVzL25h dmlnYXRvcmNvbnRlbnR1dGlscy9OYXZpZ2F0b3JDb250ZW50VXRpbHMuY3BwOgorICAgICAgICAo V2ViQ29yZTo6TmF2aWdhdG9yQ29udGVudFV0aWxzOjpyZWdpc3RlclByb3RvY29sSGFuZGxlcik6 CisgICAgICAgIChXZWJDb3JlOjpOYXZpZ2F0b3JDb250ZW50VXRpbHM6OmlzUHJvdG9jb2xIYW5k bGVyUmVnaXN0ZXJlZCk6CisgICAgICAgIChXZWJDb3JlOjpOYXZpZ2F0b3JDb250ZW50VXRpbHM6 OnVucmVnaXN0ZXJQcm90b2NvbEhhbmRsZXIpOgorCiAyMDE0LTA0LTE0ICBCZW0gSm9uZXMtQmV5 ICA8YmpvbmVzYmVAYWRvYmUuY29tPgogCiAgICAgICAgIFtDU1MgU2hhcGVzXSBSZW1vdmUgc29t ZSBsZWZ0b3ZlciBzaGFwZS1pbnNpZGUgY29kZQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUv TW9kdWxlcy9uYXZpZ2F0b3Jjb250ZW50dXRpbHMvTmF2aWdhdG9yQ29udGVudFV0aWxzLmNwcCBi L1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvbmF2aWdhdG9yY29udGVudHV0aWxzL05hdmlnYXRvckNv bnRlbnRVdGlscy5jcHAKaW5kZXggYmFkZDFjOTY3MmQzNmYxMDBiZGNmYzRhMTBhZDBmOGFjY2Mz YzhmZC4uZGQ4OGE1NDhhZjY1MWVmN2ZhZjBlOTE1ZmRmN2IxMDNmNjlhODQ5NSAxMDA2NDQKLS0t IGEvU291cmNlL1dlYkNvcmUvTW9kdWxlcy9uYXZpZ2F0b3Jjb250ZW50dXRpbHMvTmF2aWdhdG9y Q29udGVudFV0aWxzLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL25hdmlnYXRvcmNv bnRlbnR1dGlscy9OYXZpZ2F0b3JDb250ZW50VXRpbHMuY3BwCkBAIC0xMzYsMTEgKzEzNiw3IEBA IHZvaWQgTmF2aWdhdG9yQ29udGVudFV0aWxzOjpyZWdpc3RlclByb3RvY29sSGFuZGxlcihOYXZp Z2F0b3IqIG5hdmlnYXRvciwgY29uc3QKICAgICBpZiAoIW5hdmlnYXRvci0+ZnJhbWUoKSkKICAg ICAgICAgcmV0dXJuOwogCi0gICAgRG9jdW1lbnQqIGRvY3VtZW50ID0gbmF2aWdhdG9yLT5mcmFt ZSgpLT5kb2N1bWVudCgpOwotICAgIGlmICghZG9jdW1lbnQpCi0gICAgICAgIHJldHVybjsKLQot ICAgIFVSTCBiYXNlVVJMID0gZG9jdW1lbnQtPmJhc2VVUkwoKTsKKyAgICBVUkwgYmFzZVVSTCA9 IG5hdmlnYXRvci0+ZnJhbWUoKS0+ZG9jdW1lbnQoKS0+YmFzZVVSTCgpOwogCiAgICAgaWYgKCF2 ZXJpZnlDdXN0b21IYW5kbGVyVVJMKGJhc2VVUkwsIHVybCwgZWMpKQogICAgICAgICByZXR1cm47 CkBAIC0xNzgsMTEgKzE3NCw3IEBAIFN0cmluZyBOYXZpZ2F0b3JDb250ZW50VXRpbHM6OmlzUHJv dG9jb2xIYW5kbGVyUmVnaXN0ZXJlZChOYXZpZ2F0b3IqIG5hdmlnYXRvciwKICAgICBpZiAoIW5h dmlnYXRvci0+ZnJhbWUoKSkKICAgICAgICAgcmV0dXJuIGRlY2xpbmVkOwogCi0gICAgRG9jdW1l bnQqIGRvY3VtZW50ID0gbmF2aWdhdG9yLT5mcmFtZSgpLT5kb2N1bWVudCgpOwotICAgIGlmICgh ZG9jdW1lbnQpCi0gICAgICAgIHJldHVybiBkZWNsaW5lZDsKLQotICAgIFVSTCBiYXNlVVJMID0g ZG9jdW1lbnQtPmJhc2VVUkwoKTsKKyAgICBVUkwgYmFzZVVSTCA9IG5hdmlnYXRvci0+ZnJhbWUo KS0+ZG9jdW1lbnQoKS0+YmFzZVVSTCgpOwogCiAgICAgaWYgKCF2ZXJpZnlDdXN0b21IYW5kbGVy VVJMKGJhc2VVUkwsIHVybCwgZWMpKQogICAgICAgICByZXR1cm4gZGVjbGluZWQ7CkBAIC0xOTgs MTEgKzE5MCw3IEBAIHZvaWQgTmF2aWdhdG9yQ29udGVudFV0aWxzOjp1bnJlZ2lzdGVyUHJvdG9j b2xIYW5kbGVyKE5hdmlnYXRvciogbmF2aWdhdG9yLCBjb25zCiAgICAgaWYgKCFuYXZpZ2F0b3It PmZyYW1lKCkpCiAgICAgICAgIHJldHVybjsKIAotICAgIERvY3VtZW50KiBkb2N1bWVudCA9IG5h dmlnYXRvci0+ZnJhbWUoKS0+ZG9jdW1lbnQoKTsKLSAgICBpZiAoIWRvY3VtZW50KQotICAgICAg ICByZXR1cm47Ci0gICAgCi0gICAgVVJMIGJhc2VVUkwgPSBkb2N1bWVudC0+YmFzZVVSTCgpOwor ICAgIFVSTCBiYXNlVVJMID0gbmF2aWdhdG9yLT5mcmFtZSgpLT5kb2N1bWVudCgpLT5iYXNlVVJM KCk7CiAKICAgICBpZiAoIXZlcmlmeUN1c3RvbUhhbmRsZXJVUkwoYmFzZVVSTCwgdXJsLCBlYykp CiAgICAgICAgIHJldHVybjsK