144631 2015-05-05 10:34:39 -0700 Merge CachedFrameBase into CachedFrame 2015-05-06 03:09:53 -0700 1 1 1 Unclassified WebKit Page Loading 528+ (Nightly build) Unspecified Unspecified ASSIGNED P2 Normal --- 1 cdumez cdumez ap beidson commit-queue darin dbates japhet kling koivisto oldest_to_newest 1091808 0 cdumez 2015-05-05 10:34:39 -0700 Merge CachedFrameBase into CachedFrame. There is no reason to split this into 2 classes since CachedFrame is the only subclass of CachedFrameBase. 1091810 1 252388 cdumez 2015-05-05 10:38:09 -0700 Created attachment 252388 Patch 1092032 2 252388 dbates 2015-05-05 21:18:58 -0700 Comment on attachment 252388 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252388&action=review > Source/WebCore/history/CachedFrame.cpp:135 > + , m_url(frame.document()->url()) [1] This line assumes that frame.document() is a non-null pointer. > Source/WebCore/history/CachedFrame.cpp:137 > + , m_isComposited(frame.view()->hasCompositedContent()) [2] This line assumes that frame.view() is a non-null pointer. > Source/WebCore/history/CachedFrame.h:45 > Document* document() const { return m_document.get(); } I know you didn't write this code. The use of const in this function declaration is disingenuous since the underlying Document object can be modified through the pointer. There is an unwritten convention [3] for non-container type data structures of making functions that return non-const raw pointers be non-const. Can we make this function be non-const? Or can we make this function return a const Document*? Or even better, can we make this function return a reference by [1]? [3] "On returning mutable pointers from const methods", <https://lists.webkit.org/pipermail/webkit-dev/2012-October/022576.html> (start of email thread). > Source/WebCore/history/CachedFrame.h:46 > FrameView* view() const { return m_view.get(); } Similarly, can we make this return a reference by [2]? Or make this function be non-const? Or make this function return a const FrameView*? > Source/WebCore/history/CachedFrame.h:63 > + RefPtr<Document> m_document; We should make this a Ref instead of a RefPtr because of [1]. > Source/WebCore/history/CachedFrame.h:65 > + RefPtr<FrameView> m_view; We should make this a Ref instead of a RefPtr because of [2]. 1092033 3 dbates 2015-05-05 21:29:54 -0700 I'm unclear if we want this change given that CachedFrameBase was explicitly extracted from CachedFrame in <https://trac.webkit.org/changeset/47989>: "The pattern of creating a 'CachedFrameBase' as the interface CachedFrame provides to FrameLoader was suggested by Darin Adler." 252388 2015-05-05 10:38:09 -0700 2015-05-06 03:09:53 -0700 Patch bug-144631-20150505103654.patch text/plain 6394 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMTgzNzgxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNzgwYjFhNTQwMzRiYWZj OWU3NmFlYmE3NTFjOGFlMGM3OTU0NmI1Ny4uNDliZGM1MzcyYjMwMTU4ODc5YjJlNzMyYTBlZmI2 NGJhNTQwZDIwZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI1IEBACisyMDE1LTA1LTA1ICBDaHJp cyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgTWVyZ2UgQ2FjaGVkRnJhbWVC YXNlIGludG8gQ2FjaGVkRnJhbWUKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTE0NDYzMQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIE1lcmdlIENhY2hlZEZyYW1lQmFzZSBpbnRvIENhY2hlZEZyYW1lLiBUaGVy ZSBpcyBubyByZWFzb24gdG8gc3BsaXQKKyAgICAgICAgdGhpcyBpbnRvIDIgY2xhc3NlcyBzaW5j ZSBDYWNoZWRGcmFtZSBpcyB0aGUgb25seSBzdWJjbGFzcyBvZgorICAgICAgICBDYWNoZWRGcmFt ZUJhc2UuCisKKyAgICAgICAgKiBoaXN0b3J5L0NhY2hlZEZyYW1lLmNwcDoKKyAgICAgICAgKFdl YkNvcmU6OkNhY2hlZEZyYW1lOjp+Q2FjaGVkRnJhbWUpOgorICAgICAgICAoV2ViQ29yZTo6Q2Fj aGVkRnJhbWU6OnJlc3RvcmUpOgorICAgICAgICAoV2ViQ29yZTo6Q2FjaGVkRnJhbWU6OkNhY2hl ZEZyYW1lKToKKyAgICAgICAgKiBoaXN0b3J5L0NhY2hlZEZyYW1lLmg6CisgICAgICAgIChXZWJD b3JlOjpDYWNoZWRGcmFtZTo6ZG9jdW1lbnQpOiBEZWxldGVkLgorICAgICAgICAoV2ViQ29yZTo6 Q2FjaGVkRnJhbWU6OmRvY3VtZW50TG9hZGVyKTogRGVsZXRlZC4KKyAgICAgICAgKiBsb2FkZXIv RnJhbWVMb2FkZXIuY3BwOgorICAgICAgICAoV2ViQ29yZTo6RnJhbWVMb2FkZXI6Om9wZW4pOgor ICAgICAgICAqIGxvYWRlci9GcmFtZUxvYWRlci5oOgorCiAyMDE1LTA1LTA0ICBDaHJpcyBEdW1l eiAgPGNkdW1lekBhcHBsZS5jb20+CiAKICAgICAgICAgQ3Jhc2ggYXQgY29tLmFwcGxlLldlYktp dC5XZWJDb250ZW50IGF0IGNvbS5hcHBsZS5XZWJDb3JlOiBXZWJDb3JlOjpjcmVhdGVXaW5kb3cg KyAxODUKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2hpc3RvcnkvQ2FjaGVkRnJhbWUuY3Bw IGIvU291cmNlL1dlYkNvcmUvaGlzdG9yeS9DYWNoZWRGcmFtZS5jcHAKaW5kZXggMTg2M2MxNjZj NWM4MzA3NDIyNWEzZjViYWVmMTEwZTU2NGQwODg3ZC4uYjhmOThkNGMxMGNkNmYyYWYwN2I1ODdm ODFmNzk5ZjRlOWRlZTUwNCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvaGlzdG9yeS9DYWNo ZWRGcmFtZS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvaGlzdG9yeS9DYWNoZWRGcmFtZS5jcHAK QEAgLTU5LDE3ICs1OSw3IEBAIG5hbWVzcGFjZSBXZWJDb3JlIHsKIAogREVGSU5FX0RFQlVHX09O TFlfR0xPQkFMKFdURjo6UmVmQ291bnRlZExlYWtDb3VudGVyLCBjYWNoZWRGcmFtZUNvdW50ZXIs ICgiQ2FjaGVkRnJhbWUiKSk7CiAKLUNhY2hlZEZyYW1lQmFzZTo6Q2FjaGVkRnJhbWVCYXNlKEZy YW1lJiBmcmFtZSkKLSAgICA6IG1fZG9jdW1lbnQoZnJhbWUuZG9jdW1lbnQoKSkKLSAgICAsIG1f ZG9jdW1lbnRMb2FkZXIoZnJhbWUubG9hZGVyKCkuZG9jdW1lbnRMb2FkZXIoKSkKLSAgICAsIG1f dmlldyhmcmFtZS52aWV3KCkpCi0gICAgLCBtX3VybChmcmFtZS5kb2N1bWVudCgpLT51cmwoKSkK LSAgICAsIG1faXNNYWluRnJhbWUoIWZyYW1lLnRyZWUoKS5wYXJlbnQoKSkKLSAgICAsIG1faXND b21wb3NpdGVkKGZyYW1lLnZpZXcoKS0+aGFzQ29tcG9zaXRlZENvbnRlbnQoKSkKLXsKLX0KLQot Q2FjaGVkRnJhbWVCYXNlOjp+Q2FjaGVkRnJhbWVCYXNlKCkKK0NhY2hlZEZyYW1lOjp+Q2FjaGVk RnJhbWUoKQogewogI2lmbmRlZiBOREVCVUcKICAgICBjYWNoZWRGcmFtZUNvdW50ZXIuZGVjcmVt ZW50KCk7CkBAIC03OCw3ICs2OCw3IEBAIENhY2hlZEZyYW1lQmFzZTo6fkNhY2hlZEZyYW1lQmFz ZSgpCiAgICAgQVNTRVJUKCFtX2RvY3VtZW50KTsKIH0KIAotdm9pZCBDYWNoZWRGcmFtZUJhc2U6 OnJlc3RvcmUoKQordm9pZCBDYWNoZWRGcmFtZTo6cmVzdG9yZSgpCiB7CiAgICAgQVNTRVJUKG1f ZG9jdW1lbnQtPnZpZXcoKSA9PSBtX3ZpZXcpOwogCkBAIC0xMzksNyArMTI5LDEyIEBAIHZvaWQg Q2FjaGVkRnJhbWVCYXNlOjpyZXN0b3JlKCkKIH0KIAogQ2FjaGVkRnJhbWU6OkNhY2hlZEZyYW1l KEZyYW1lJiBmcmFtZSkKLSAgICA6IENhY2hlZEZyYW1lQmFzZShmcmFtZSkKKyAgICA6IG1fZG9j dW1lbnQoZnJhbWUuZG9jdW1lbnQoKSkKKyAgICAsIG1fZG9jdW1lbnRMb2FkZXIoZnJhbWUubG9h ZGVyKCkuZG9jdW1lbnRMb2FkZXIoKSkKKyAgICAsIG1fdmlldyhmcmFtZS52aWV3KCkpCisgICAg LCBtX3VybChmcmFtZS5kb2N1bWVudCgpLT51cmwoKSkKKyAgICAsIG1faXNNYWluRnJhbWUoIWZy YW1lLnRyZWUoKS5wYXJlbnQoKSkKKyAgICAsIG1faXNDb21wb3NpdGVkKGZyYW1lLnZpZXcoKS0+ aGFzQ29tcG9zaXRlZENvbnRlbnQoKSkKIHsKICNpZm5kZWYgTkRFQlVHCiAgICAgY2FjaGVkRnJh bWVDb3VudGVyLmluY3JlbWVudCgpOwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvaGlzdG9y eS9DYWNoZWRGcmFtZS5oIGIvU291cmNlL1dlYkNvcmUvaGlzdG9yeS9DYWNoZWRGcmFtZS5oCmlu ZGV4IGFlM2I1OGVmOGU4YjA2NTAwMjg3YWI4NjdjMmVlMDJhMGY0ZGI0NTUuLmM3MmUxMmRkZTEw MjZiNzczMWZlOTg4MzJjM2FjNDI3NjJjODBlMjggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3Jl L2hpc3RvcnkvQ2FjaGVkRnJhbWUuaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9oaXN0b3J5L0NhY2hl ZEZyYW1lLmgKQEAgLTI2LDYyICsyNiw1MCBAQAogI2lmbmRlZiBDYWNoZWRGcmFtZV9oCiAjZGVm aW5lIENhY2hlZEZyYW1lX2gKIAotI2luY2x1ZGUgIkRPTVdpbmRvdy5oIgogI2luY2x1ZGUgIlVS TC5oIgogI2luY2x1ZGUgIlNjcmlwdENhY2hlZEZyYW1lRGF0YS5oIgogI2luY2x1ZGUgPHd0Zi9S ZWZQdHIuaD4KIAogbmFtZXNwYWNlIFdlYkNvcmUgewogCi1jbGFzcyBDYWNoZWRGcmFtZTsKIGNs YXNzIENhY2hlZEZyYW1lUGxhdGZvcm1EYXRhOwogY2xhc3MgRG9jdW1lbnQ7CiBjbGFzcyBEb2N1 bWVudExvYWRlcjsKIGNsYXNzIEZyYW1lVmlldzsKLWNsYXNzIE5vZGU7CiAKLWNsYXNzIENhY2hl ZEZyYW1lQmFzZSB7CitjbGFzcyBDYWNoZWRGcmFtZSB7CiBwdWJsaWM6Ci0gICAgdm9pZCByZXN0 b3JlKCk7CisgICAgZXhwbGljaXQgQ2FjaGVkRnJhbWUoRnJhbWUmKTsKKyAgICB+Q2FjaGVkRnJh bWUoKTsKIAogICAgIERvY3VtZW50KiBkb2N1bWVudCgpIGNvbnN0IHsgcmV0dXJuIG1fZG9jdW1l bnQuZ2V0KCk7IH0KICAgICBGcmFtZVZpZXcqIHZpZXcoKSBjb25zdCB7IHJldHVybiBtX3ZpZXcu Z2V0KCk7IH0KICAgICBjb25zdCBVUkwmIHVybCgpIGNvbnN0IHsgcmV0dXJuIG1fdXJsOyB9CiAg ICAgYm9vbCBpc01haW5GcmFtZSgpIHsgcmV0dXJuIG1faXNNYWluRnJhbWU7IH0KIAotcHJvdGVj dGVkOgotICAgIENhY2hlZEZyYW1lQmFzZShGcmFtZSYpOwotICAgIH5DYWNoZWRGcmFtZUJhc2Uo KTsKLSAgICAKLSAgICBSZWZQdHI8RG9jdW1lbnQ+IG1fZG9jdW1lbnQ7Ci0gICAgUmVmUHRyPERv Y3VtZW50TG9hZGVyPiBtX2RvY3VtZW50TG9hZGVyOwotICAgIFJlZlB0cjxGcmFtZVZpZXc+IG1f dmlldzsKLSAgICBVUkwgbV91cmw7Ci0gICAgc3RkOjp1bmlxdWVfcHRyPFNjcmlwdENhY2hlZEZy YW1lRGF0YT4gbV9jYWNoZWRGcmFtZVNjcmlwdERhdGE7Ci0gICAgc3RkOjp1bmlxdWVfcHRyPENh Y2hlZEZyYW1lUGxhdGZvcm1EYXRhPiBtX2NhY2hlZEZyYW1lUGxhdGZvcm1EYXRhOwotICAgIGJv b2wgbV9pc01haW5GcmFtZTsKLSAgICBib29sIG1faXNDb21wb3NpdGVkOwotICAgIAotICAgIFZl Y3RvcjxzdGQ6OnVuaXF1ZV9wdHI8Q2FjaGVkRnJhbWU+PiBtX2NoaWxkRnJhbWVzOwotfTsKLQot Y2xhc3MgQ2FjaGVkRnJhbWUgOiBwcml2YXRlIENhY2hlZEZyYW1lQmFzZSB7Ci1wdWJsaWM6Ci0g ICAgZXhwbGljaXQgQ2FjaGVkRnJhbWUoRnJhbWUmKTsKLQogICAgIHZvaWQgb3BlbigpOwogICAg IHZvaWQgY2xlYXIoKTsKICAgICB2b2lkIGRlc3Ryb3koKTsKKyAgICB2b2lkIHJlc3RvcmUoKTsK IAogICAgIFdFQkNPUkVfRVhQT1JUIHZvaWQgc2V0Q2FjaGVkRnJhbWVQbGF0Zm9ybURhdGEoc3Rk Ojp1bmlxdWVfcHRyPENhY2hlZEZyYW1lUGxhdGZvcm1EYXRhPik7CiAgICAgV0VCQ09SRV9FWFBP UlQgQ2FjaGVkRnJhbWVQbGF0Zm9ybURhdGEqIGNhY2hlZEZyYW1lUGxhdGZvcm1EYXRhKCk7CiAK LSAgICB1c2luZyBDYWNoZWRGcmFtZUJhc2U6OmRvY3VtZW50OwotICAgIHVzaW5nIENhY2hlZEZy YW1lQmFzZTo6dmlldzsKLSAgICB1c2luZyBDYWNoZWRGcmFtZUJhc2U6OnVybDsKICAgICBEb2N1 bWVudExvYWRlciogZG9jdW1lbnRMb2FkZXIoKSBjb25zdCB7IHJldHVybiBtX2RvY3VtZW50TG9h ZGVyLmdldCgpOyB9CiAKICAgICBpbnQgZGVzY2VuZGFudEZyYW1lQ291bnQoKSBjb25zdDsKKwor cHJpdmF0ZToKKyAgICBSZWZQdHI8RG9jdW1lbnQ+IG1fZG9jdW1lbnQ7CisgICAgUmVmUHRyPERv Y3VtZW50TG9hZGVyPiBtX2RvY3VtZW50TG9hZGVyOworICAgIFJlZlB0cjxGcmFtZVZpZXc+IG1f dmlldzsKKyAgICBVUkwgbV91cmw7CisgICAgc3RkOjp1bmlxdWVfcHRyPFNjcmlwdENhY2hlZEZy YW1lRGF0YT4gbV9jYWNoZWRGcmFtZVNjcmlwdERhdGE7CisgICAgc3RkOjp1bmlxdWVfcHRyPENh Y2hlZEZyYW1lUGxhdGZvcm1EYXRhPiBtX2NhY2hlZEZyYW1lUGxhdGZvcm1EYXRhOworICAgIGJv b2wgbV9pc01haW5GcmFtZTsKKyAgICBib29sIG1faXNDb21wb3NpdGVkOworCisgICAgVmVjdG9y PHN0ZDo6dW5pcXVlX3B0cjxDYWNoZWRGcmFtZT4+IG1fY2hpbGRGcmFtZXM7CiB9OwogCiB9IC8v IG5hbWVzcGFjZSBXZWJDb3JlCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvRnJh bWVMb2FkZXIuY3BwIGIvU291cmNlL1dlYkNvcmUvbG9hZGVyL0ZyYW1lTG9hZGVyLmNwcAppbmRl eCA0NzkxY2Q4NGRiNDllMDFmMmRmYWNhY2RjYzMyMjdkZjkwMDNkMThmLi4yOGY1ZTQ2NTE3NGM5 OTM5ZTFlYmUxNjVkYjg4OGIwOTA2NGIyOWM2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9s b2FkZXIvRnJhbWVMb2FkZXIuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2xvYWRlci9GcmFtZUxv YWRlci5jcHAKQEAgLTIwMzUsNyArMjAzNSw3IEBAIHZvaWQgRnJhbWVMb2FkZXI6OnByZXBhcmVG b3JDYWNoZWRQYWdlUmVzdG9yZSgpCiAgICAgfQogfQogCi12b2lkIEZyYW1lTG9hZGVyOjpvcGVu KENhY2hlZEZyYW1lQmFzZSYgY2FjaGVkRnJhbWUpCit2b2lkIEZyYW1lTG9hZGVyOjpvcGVuKENh Y2hlZEZyYW1lJiBjYWNoZWRGcmFtZSkKIHsKICAgICBtX2lzQ29tcGxldGUgPSBmYWxzZTsKICAg ICAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2xvYWRlci9GcmFtZUxvYWRlci5oIGIvU291 cmNlL1dlYkNvcmUvbG9hZGVyL0ZyYW1lTG9hZGVyLmgKaW5kZXggOGY0NjJkMjkxNWIyOGFkZDFm MTcwZTYzOWZkMzY2MmQwNmJhZjQyOC4uNmMyZDRiZmZhMWZkNGE2NjY1NGIzNTdlYWM0MGM5YzI3 MjNiYzViZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvbG9hZGVyL0ZyYW1lTG9hZGVyLmgK KysrIGIvU291cmNlL1dlYkNvcmUvbG9hZGVyL0ZyYW1lTG9hZGVyLmgKQEAgLTUyLDcgKzUyLDcg QEAKIG5hbWVzcGFjZSBXZWJDb3JlIHsKIAogY2xhc3MgQXJjaGl2ZTsKLWNsYXNzIENhY2hlZEZy YW1lQmFzZTsKK2NsYXNzIENhY2hlZEZyYW1lOwogY2xhc3MgQ2FjaGVkUGFnZTsKIGNsYXNzIENh Y2hlZFJlc291cmNlOwogY2xhc3MgQ2hyb21lOwpAQCAtMTI1LDcgKzEyNSw3IEBAIHB1YmxpYzoK ICAgICBXRUJDT1JFX0VYUE9SVCB2b2lkIHJlbG9hZChib29sIGVuZFRvRW5kUmVsb2FkID0gZmFs c2UpOwogICAgIFdFQkNPUkVfRVhQT1JUIHZvaWQgcmVsb2FkV2l0aE92ZXJyaWRlRW5jb2Rpbmco Y29uc3QgU3RyaW5nJiBvdmVycmlkZUVuY29kaW5nKTsKIAotICAgIHZvaWQgb3BlbihDYWNoZWRG cmFtZUJhc2UmKTsKKyAgICB2b2lkIG9wZW4oQ2FjaGVkRnJhbWUmKTsKICAgICB2b2lkIGxvYWRJ dGVtKEhpc3RvcnlJdGVtJiwgRnJhbWVMb2FkVHlwZSk7CiAgICAgSGlzdG9yeUl0ZW0qIHJlcXVl c3RlZEhpc3RvcnlJdGVtKCkgY29uc3QgeyByZXR1cm4gbV9yZXF1ZXN0ZWRIaXN0b3J5SXRlbS5n ZXQoKTsgfQogCg==