116670 2013-05-23 07:19:16 -0700 Fix loader/go-back-cached-main-resource.html test 2013-05-24 03:42:36 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 0 rego webkit-unassigned ap cgarcia commit-queue darin oldest_to_newest 892858 0 rego 2013-05-23 07:19:16 -0700 This bug is to fix the issue explained in comment the following comment about loader/go-back-cached-main-resource.html layout test: https://bugs.webkit.org/show_bug.cgi?id=112418#c29 Comment by "Alexey Proskuryakov": > (From update of attachment 197215 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=197215&action=review > > > LayoutTests/loader/go-back-cached-main-resource.html:36 > > + target.postMessage('navigate-back', '*'); > > + // Wait a bit for resource load callbacks > > + setTimeout(function () { > > It is expected that the page does not get an onload when being restored from page cache, so a 'first-page' message is not sent. > > A better way to check for page being restored from page cache is to handle pageshow event on it. 892862 1 202708 rego 2013-05-23 07:25:08 -0700 Created attachment 202708 Patch 892945 2 202708 ap 2013-05-23 10:29:29 -0700 Comment on attachment 202708 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=202708&action=review Getting rid of the timeout is a very welcome improvement, so r=me. Please consider the suggestion below. > LayoutTests/loader/resources/first-page.html:21 > +<body onpageshow="registerListeners();"> This is quite misleading. We only need to register listeners on first load, they are already present when restoring from cache. But the function does more than its name says, which is why it's necessary to call it every time! One way to clean this up would be to rename the function to "didShowPage", and use event.persisted property to check if we are handling the initial load. Or just use an anonymous function, and register it as window.onpageshow = function (event) { ... } 892955 3 202708 commit-queue 2013-05-23 10:52:34 -0700 Comment on attachment 202708 Patch Clearing flags on attachment: 202708 Committed r150594: <http://trac.webkit.org/changeset/150594> 892956 4 commit-queue 2013-05-23 10:52:36 -0700 All reviewed patches have been landed. Closing bug. 893256 5 rego 2013-05-24 03:42:36 -0700 (In reply to comment #2) > (From update of attachment 202708 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=202708&action=review > > Getting rid of the timeout is a very welcome improvement, so r=me. Please consider the suggestion below. Thanks for the review. > > LayoutTests/loader/resources/first-page.html:21 > > +<body onpageshow="registerListeners();"> > > This is quite misleading. We only need to register listeners on first load, they are already present when restoring from cache. But the function does more than its name says, which is why it's necessary to call it every time! > > One way to clean this up would be to rename the function to "didShowPage", and use event.persisted property to check if we are handling the initial load. Or just use an anonymous function, and register it as window.onpageshow = function (event) { ... } Thanks for the information, I've changed the test in a new bug #116709 in order to register listeners only on initial load. 202708 2013-05-23 07:25:08 -0700 2013-05-23 10:52:34 -0700 Patch bug-116670-20130523162411.patch text/plain 2967 rego U3VidmVyc2lvbiBSZXZpc2lvbjogMTUwNDI3CmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9DaGFu Z2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKaW5kZXggZThmYjc0ZjYxMTg5NWY2OTA3OWEx OWJjMDlhNTg1YjFjNWE5MDNjMC4uNjZmNzljZmYzODAxMDZhNmVmMDM0NTJmYzA0NGZmMmJkZmU1 ZDgwNCAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3Rz L0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEzLTA1LTIzICBNYW51ZWwgUmVnbyBDYXNh c25vdmFzICA8cmVnb0BpZ2FsaWEuY29tPgorCisgICAgICAgIEZpeCBsb2FkZXIvZ28tYmFjay1j YWNoZWQtbWFpbi1yZXNvdXJjZS5odG1sIHRlc3QKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTExNjY3MAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIFRoZSBwYWdlIGRvZXMgbm90IGdldCBhbiBvbmxvYWQgd2hl biBiZWluZyByZXN0b3JlZCBmcm9tIHBhZ2UgY2FjaGUuIFRoZQorICAgICAgICB0ZXN0IGNhc2Ug Y2FzZSBpcyBtb2RpZmllZCB0byBoYW5kbGUgcGFnZXNob3cgZXZlbnQgaW5zdGVhZC4KKworICAg ICAgICAqIGxvYWRlci9nby1iYWNrLWNhY2hlZC1tYWluLXJlc291cmNlLmh0bWw6IFdhaXQgZm9y IHRoZSB0aGlyZAorICAgICAgICAnZmlyc3QtcGFnZScgbWVzc2FnZSB0byBmaW5pc2ggdGhlIHRl c3QuCisgICAgICAgICogbG9hZGVyL3Jlc291cmNlcy9maXJzdC1wYWdlLmh0bWw6IFVzZSBvbnBh Z2VzaG93IGluc3RlYWQgb2Ygb25sb2FkLgorICAgICAgICAqIGxvYWRlci9yZXNvdXJjZXMvb3Ro ZXItcGFnZS5odG1sOiBEaXR0by4KKwogMjAxMy0wNS0yMSAgUnlvc3VrZSBOaXdhICA8cm5pd2FA d2Via2l0Lm9yZz4KIAogICAgICAgICBbTWFjXSBzdmcvYmF0aWsvcGFpbnRzL3BhdHRlcm5SZWdp b25zLXBvc2l0aW9uZWQtb2JqZWN0cy5zdmcgaXMgZmxha3kKZGlmZiAtLWdpdCBhL0xheW91dFRl c3RzL2xvYWRlci9nby1iYWNrLWNhY2hlZC1tYWluLXJlc291cmNlLmh0bWwgYi9MYXlvdXRUZXN0 cy9sb2FkZXIvZ28tYmFjay1jYWNoZWQtbWFpbi1yZXNvdXJjZS5odG1sCmluZGV4IDQ1YjUxNWJk ZTFhMDQ1Y2M5Yzk2MzA1MTliNmMzODNiY2JmNjg4MDQuLjc1M2Q1YjAxN2ViZGUwNzc2MzA3ZmZk Yzc4YzI4MjgzNDZhMzQ0YzEgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3RzL2xvYWRlci9nby1iYWNr LWNhY2hlZC1tYWluLXJlc291cmNlLmh0bWwKKysrIGIvTGF5b3V0VGVzdHMvbG9hZGVyL2dvLWJh Y2stY2FjaGVkLW1haW4tcmVzb3VyY2UuaHRtbApAQCAtMjIsNiArMjIsOCBAQAogICAgICAgICAg ICAgZmlyc3RQYWdlVmlzaXRzKys7CiAgICAgICAgICAgICBpZiAoZmlyc3RQYWdlVmlzaXRzIDwg MykKICAgICAgICAgICAgICAgICB0YXJnZXQucG9zdE1lc3NhZ2UoJ25hdmlnYXRlLW90aGVyLXBh Z2UnLCAnKicpOworICAgICAgICAgICAgaWYgKGZpcnN0UGFnZVZpc2l0cyA9PSAzKQorICAgICAg ICAgICAgICAgIHdpbmRvdy50ZXN0UnVubmVyLm5vdGlmeURvbmUoKTsKICAgICAgICAgICAgIHJl dHVybjsKICAgICAgICAgfQogCkBAIC0zMiwxMiArMzQsNiBAQAogICAgICAgICAgICAgICAgIGJy ZWFrOwogICAgICAgICAgICAgY2FzZSAyOgogICAgICAgICAgICAgICAgIHRhcmdldC5wb3N0TWVz c2FnZSgnbmF2aWdhdGUtYmFjaycsICcqJyk7Ci0gICAgICAgICAgICAgICAgLy8gV2FpdCBhIGJp dCBmb3IgcmVzb3VyY2UgbG9hZCBjYWxsYmFja3MKLSAgICAgICAgICAgICAgICBzZXRUaW1lb3V0 KGZ1bmN0aW9uICgpIHsKLSAgICAgICAgICAgICAgICAgICAgdGFyZ2V0LmNsb3NlKCk7Ci0gICAg ICAgICAgICAgICAgICAgIGlmICh3aW5kb3cudGVzdFJ1bm5lcikKLSAgICAgICAgICAgICAgICAg ICAgICAgIHdpbmRvdy50ZXN0UnVubmVyLm5vdGlmeURvbmUoKTsKLSAgICAgICAgICAgICAgICAg ICAgfSwgMjAwKTsKICAgICAgICAgICAgICAgICBicmVhazsKICAgICAgICAgICAgIH0KICAgICAg ICAgICAgIHJldHVybjsKZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL2xvYWRlci9yZXNvdXJjZXMv Zmlyc3QtcGFnZS5odG1sIGIvTGF5b3V0VGVzdHMvbG9hZGVyL3Jlc291cmNlcy9maXJzdC1wYWdl Lmh0bWwKaW5kZXggOTUyMGJiZDBiMjc1ODU0MTQ0OTE5ZWZjZWQ3MmJjMzYyZDkxMTFiOS4uNGQy YzJmMzBjYThkMjIwMTJhZjBmZGEwYjRkMDQyOGVkMTYyMjkwMiAxMDA2NDQKLS0tIGEvTGF5b3V0 VGVzdHMvbG9hZGVyL3Jlc291cmNlcy9maXJzdC1wYWdlLmh0bWwKKysrIGIvTGF5b3V0VGVzdHMv bG9hZGVyL3Jlc291cmNlcy9maXJzdC1wYWdlLmh0bWwKQEAgLTE4LDYgKzE4LDYgQEAKICAgICB9 CiAgICAgPC9zY3JpcHQ+CiA8L2hlYWQ+Ci08Ym9keSBvbmxvYWQ9InJlZ2lzdGVyTGlzdGVuZXJz KCk7Ij4KKzxib2R5IG9ucGFnZXNob3c9InJlZ2lzdGVyTGlzdGVuZXJzKCk7Ij4KIDwvYm9keT4K IDwvaHRtbD4KZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL2xvYWRlci9yZXNvdXJjZXMvb3RoZXIt cGFnZS5odG1sIGIvTGF5b3V0VGVzdHMvbG9hZGVyL3Jlc291cmNlcy9vdGhlci1wYWdlLmh0bWwK aW5kZXggZWM4Nzg4NjM0Nzg0MTY3MzlhOWE0MmFhMzk2NzljYjMyYTRjYmM5MC4uYzdmNzQ2YTFi MDAzOGIwMmJiYTkyZjFiZWNkYjEwMjFkN2QxNGU2YSAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMv bG9hZGVyL3Jlc291cmNlcy9vdGhlci1wYWdlLmh0bWwKKysrIGIvTGF5b3V0VGVzdHMvbG9hZGVy L3Jlc291cmNlcy9vdGhlci1wYWdlLmh0bWwKQEAgLTI0LDYgKzI0LDYgQEAKICAgICB9CiAgICAg PC9zY3JpcHQ+CiA8L2hlYWQ+Ci08Ym9keSBvbmxvYWQ9InJlZ2lzdGVyTGlzdGVuZXJzKCk7Ij4K Kzxib2R5IG9ucGFnZXNob3c9InJlZ2lzdGVyTGlzdGVuZXJzKCk7Ij4KIDwvYm9keT4KIDwvaHRt bD4K