145422 2015-05-27 16:39:05 -0700 WebContent crash in WebCore::Page::sessionID() const + 0 (Page.cpp:1660) 2015-06-07 17:42:36 -0700 1 1 1 Unclassified WebKit Page Loading 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=145748 InRadar P2 Normal --- 1 cdumez cdumez ap barraclough beidson darin kling koivisto sam oldest_to_newest 1097768 0 cdumez 2015-05-27 16:39:05 -0700 WebContent crash in WebCore::Page::sessionID() const + 0 (Page.cpp:1660) 0 WebCore 0x31cfa010 WebCore::Page::sessionID() const + 0 (Page.cpp:1660) 1 WebCore 0x31565378 WebCore::DocumentLoader::stopLoading() + 364 (DocumentLoader.cpp:106) 2 WebCore 0x315717b2 WebCore::DocumentLoader::detachFromFrame() + 42 (DocumentLoader.cpp:919) 3 WebCore 0x315de0a6 WebCore::FrameLoader::detachViewsAndDocumentLoader() + 46 (FrameLoader.cpp:1680) 4 WebCore 0x315d91ae WebCore::CachedFrame::destroy() + 38 (CachedFrame.cpp:259) 5 WebCore 0x315d91ca WebCore::CachedFrame::destroy() + 66 (CachedFrame.cpp:263) 6 WebCore 0x315d9166 WebCore::CachedPage::~CachedPage() + 10 (CachedPage.cpp:74) 7 WebCore 0x31cfa890 WebCore::PageCache::prune(WebCore::PruningReason) + 40 (memory:2431) 8 WebCore 0x31cfa860 WebCore::PageCache::pruneToSizeNow(unsigned int, WebCore::PruningReason) + 12 (PageCache.cpp:356) 9 WebCore 0x3154fabc WebCore::FrameLoader::commitProvisionalLoad() + 204 (FrameLoader.cpp:1760) The DocumentLoader still thinks it is loading when the PageCache entry is being destroyed. We should never have a page that is loading in the PageCache so this should never happen. Radar: <rdar://problem/20613631> 1097779 1 253819 cdumez 2015-05-27 17:00:14 -0700 Created attachment 253819 Patch 1097807 2 253819 ap 2015-05-27 19:02:19 -0700 Comment on attachment 253819 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253819&action=review > Source/WebCore/ChangeLog:11 > + We sometimes crash when destroying a PageCache CachedFrame because its > + DocumentLoader is still loading. This should never happen as we are not > + supposed to let pages are still have pending loads into the PageCache. This seems like it could have a negative effect because pages with long running requests would never be cacheable (e.g. comet-style XMLHttpRequest). Also, will PingLoader also prevent going to page cache? I used to think that we stopped all loads when entering the page cache, but I cannot find that now through code inspection. 1097811 3 253819 cdumez 2015-05-27 19:28:40 -0700 Comment on attachment 253819 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253819&action=review >> Source/WebCore/ChangeLog:11 >> + supposed to let pages are still have pending loads into the PageCache. > > This seems like it could have a negative effect because pages with long running requests would never be cacheable (e.g. comet-style XMLHttpRequest). Also, will PingLoader also prevent going to page cache? > > I used to think that we stopped all loads when entering the page cache, but I cannot find that now through code inspection. I added layout tests for page-caching when there are pending XHRs and image loads a while back and they are still passing with this change. As explained below, we *do* stop pending loads when starting a new provisional load (see stopAllLoaders() call in FrameLoader::continueLoadAfterNavigationPolicy()). So *generally*, there are no pending loads at the time we call PageCache::canCache() / PageCache::add(). However, we don't stop JS (e.g. DOM timers) at this point so nothing prevents JS from starting new loads *after* stopAllLoaders() is called (when the provisional load is started) and *before* the provisional load is committed (which is when we put the page into PageCache and actually suspend all ActiveDOMObjects, including DOMTimers). Ideally, we would find a way to avoid prevent this so that we could PageCache more but short-term I want to fix this top crasher in a safe way. 1097814 4 ap 2015-05-27 20:30:03 -0700 Thank you, this makes sense. 1098222 5 253819 darin 2015-05-29 15:38:40 -0700 Comment on attachment 253819 Patch Alexey, do you agree with Chris’s justification? If so, could you review the patch? 1098254 6 253819 beidson 2015-05-29 16:37:02 -0700 Comment on attachment 253819 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253819&action=review r+ with one change > Source/WebCore/page/DiagnosticLoggingKeys.h:63 > - static String loadingAPISenseKey(); > + static String isLoading(); isLoadingKey() 1098256 7 cdumez 2015-05-29 16:42:43 -0700 Committed r185017: <http://trac.webkit.org/changeset/185017> 1098257 8 253819 cdumez 2015-05-29 16:44:37 -0700 Comment on attachment 253819 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253819&action=review Thanks for reviewing. >> Source/WebCore/page/DiagnosticLoggingKeys.h:63 >> + static String isLoading(); > > isLoadingKey() Right, fixed before landing. 253819 2015-05-27 17:00:14 -0700 2015-05-29 16:43:48 -0700 Patch bug-145422-20150527170002.patch text/plain 5784 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMTg0OTI0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMGY4NDI3NzljMmFiYTk3 YzU0ZjBlNmU3MjY2NDU5MGQ4ODJlYmJjNy4uMDRiNmU1MTQyMmI2OWM0YzA4OWI5OThmOGE0ZDRh YWYyMzlhYzA1OCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDQzIEBACisyMDE1LTA1LTI3ICBDaHJp cyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgV2ViQ29udGVudCBjcmFzaCBp biBXZWJDb3JlOjpQYWdlOjpzZXNzaW9uSUQoKSBjb25zdCArIDAgKFBhZ2UuY3BwOjE2NjApCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNDU0MjIKKyAg ICAgICAgPHJkYXI6Ly9wcm9ibGVtLzIwNjEzNjMxPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIFdlIHNvbWV0aW1lcyBjcmFzaCB3aGVuIGRlc3Ryb3lp bmcgYSBQYWdlQ2FjaGUgQ2FjaGVkRnJhbWUgYmVjYXVzZSBpdHMKKyAgICAgICAgRG9jdW1lbnRM b2FkZXIgaXMgc3RpbGwgbG9hZGluZy4gVGhpcyBzaG91bGQgbmV2ZXIgaGFwcGVuIGFzIHdlIGFy ZSBub3QKKyAgICAgICAgc3VwcG9zZWQgdG8gbGV0IHBhZ2VzIGFyZSBzdGlsbCBoYXZlIHBlbmRp bmcgbG9hZHMgaW50byB0aGUgUGFnZUNhY2hlLgorCisgICAgICAgIEhvd2V2ZXIsIHdlIHdlcmUg dXNpbmcgRG9jdW1lbnRMb2FkZXI6OmlzTG9hZGluZ0luQVBJU2Vuc2UoKSBhcyBjaGVjaworICAg ICAgICBpbiBQYWdlQ2FjaGU6OmNhbkNhY2hlUGFnZUNvbnRhaW5pbmdUaGlzRnJhbWUoKSB3aGlj aCBpcyBub3QgZXhhY3RseQorICAgICAgICB3aGF0IHdlIHdhbnQuIGlzTG9hZGluZ0luQVBJU2Vu c2UoKSBubyBsb25nZXIgY29uc2lkZXJzIHN1YnJlc291cmNlCisgICAgICAgIGxvYWRzIG9uY2Ug dGhlIGZyYW1lIGFzIGxvYWRlZC4gVGhpcyBtZWFucyBpZiB0aGUgSlMgdHJpZ2dlcnMgYSBuZXcK KyAgICAgICAgbG9hZCBpbiBhIHN1YmZyYW1lIGFmdGVyIGl0IGhhcyBiZWVuIGxvYWRlZCwgdGhl biBpc0xvYWRpbmdJbkFQSVNlbnNlKCkKKyAgICAgICAgd2lsbCByZXR1cm4gZmFsc2UsIGRlc3Bp dGUgdGhlIHBlbmRpbmcgbG9hZC4KKworICAgICAgICBUaGlzIHBhdGNoIHJlcGxhY2VzIHRoZSBp c0xvYWRpbmdJbkFQSVNlbnNlKCkgY2hlY2sgd2l0aCBpc0xvYWRpbmcoKQorICAgICAgICBhcyB0 aGlzIHdpbGwgY29uc2lkZXIgYWxsIHBlbmRpbmcgbG9hZHMsIGV2ZW4gYWZ0ZXIgdGhlIGZyYW1l IGlzCisgICAgICAgIGxvYWRlZC4KKworICAgICAgICBJbiBtb3N0IGNhc2VzLCB1c2luZyBpc0xv YWRpbmdJbkFQSVNlbnNlKCkgd2FzIG5vdCBhbiBpc3N1ZSBiZWNhdXNlCisgICAgICAgIHdlIGNh bGwgRG9jdW1lbnRMb2FkZXI6OnN0b3BMb2FkaW5nKCkgaW4gYWxsIHN1YmZyYW1lcyBiZWZvcmUg c3RhcnRpbmcKKyAgICAgICAgYSBwcm92aXNpb25hbCBsb2FkLiBIb3dldmVyLCBub3RoaW5nIHNl ZW1zIHRvIHByZXZlbnQgSlMgZnJvbQorICAgICAgICB0cmlnZ2VyaW5nIGEgbmV3IGxvYWQgYWZ0 ZXIgdGhhdCBhbmQgYmVmb3JlIHRoZSBuZXcgbG9hZCBnZXRzIGNvbW1pdHRlZAorICAgICAgICAo d2hpY2ggaXMgd2hlbiB3ZSBzYXZlIHRoZSBwYWdlIGludG8gUGFnZUNhY2hlKS4KKworICAgICAg ICBObyBuZXcgdGVzdCBhcyB3ZSBkb24ndCBoYXZlIGEgcmVsaWFibGUgcmVwcm9kdWN0aW9uIGNh c2UgYW5kIHRoZQorICAgICAgICBpc3N1ZSBpcyB0aW1pbmcgcmVsYXRlZC4KKworICAgICAgICAq IGhpc3RvcnkvUGFnZUNhY2hlLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OmxvZ0NhbkNhY2hlRnJh bWVEZWNpc2lvbik6CisgICAgICAgIChXZWJDb3JlOjpQYWdlQ2FjaGU6OmNhbkNhY2hlUGFnZUNv bnRhaW5pbmdUaGlzRnJhbWUpOgorICAgICAgICAqIHBhZ2UvRGlhZ25vc3RpY0xvZ2dpbmdLZXlz LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkRpYWdub3N0aWNMb2dnaW5nS2V5czo6aXNMb2FkaW5n KToKKyAgICAgICAgKFdlYkNvcmU6OkRpYWdub3N0aWNMb2dnaW5nS2V5czo6bG9hZGluZ0FQSVNl bnNlS2V5KTogRGVsZXRlZC4KKyAgICAgICAgKiBwYWdlL0RpYWdub3N0aWNMb2dnaW5nS2V5cy5o OgorCiAyMDE1LTA1LTI3ICBEYW5pZWwgQmF0ZXMgIDxkYWJhdGVzQGFwcGxlLmNvbT4KIAogICAg ICAgICBBdHRlbXB0IHRvIGZpeCBpbnRlcm5hbCBidWlsZCBmb2xsb3dpbmcgPGh0dHA6Ly90cmFj LndlYmtpdC5vcmcvY2hhbmdlc2V0LzE4NDc2MD4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3Jl L2hpc3RvcnkvUGFnZUNhY2hlLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2hpc3RvcnkvUGFnZUNhY2hl LmNwcAppbmRleCA5NGVhZGFhNDZmOWY2NjVjOWY5NDExYzBlZGY5YmMzMzVmMWFkMTNmLi45Mjdi ZmQxOGIzZDJhNmY5ZGRmZjdiNWUxNDI0OTVmZjg4OWMyNDNmIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViQ29yZS9oaXN0b3J5L1BhZ2VDYWNoZS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvaGlzdG9y eS9QYWdlQ2FjaGUuY3BwCkBAIC04Niw3ICs4Niw3IEBAIGVudW0gUmVhc29uRnJhbWVDYW5ub3RC ZUluUGFnZUNhY2hlIHsKICAgICBIYXNTaGFyZWRXb3JrZXJzLCAvLyBGSVhNRTogUmVtb3ZlLgog ICAgIE5vSGlzdG9yeUl0ZW0sCiAgICAgUXVpY2tSZWRpcmVjdENvbWluZywKLSAgICBJc0xvYWRp bmdJbkFQSVNlbnNlLAorICAgIElzTG9hZGluZywKICAgICBJc1N0b3BwaW5nLAogICAgIENhbm5v dFN1c3BlbmRBY3RpdmVET01PYmplY3RzLAogICAgIERvY3VtZW50TG9hZGVyVXNlc0FwcGxpY2F0 aW9uQ2FjaGUsCkBAIC0xNTksMTAgKzE1OSwxMCBAQCBzdGF0aWMgdW5zaWduZWQgbG9nQ2FuQ2Fj aGVGcmFtZURlY2lzaW9uKEZyYW1lJiBmcmFtZSwgRGlhZ25vc3RpY0xvZ2dpbmdDbGllbnQmCiAg ICAgICAgIGxvZ1BhZ2VDYWNoZUZhaWx1cmVEaWFnbm9zdGljTWVzc2FnZShkaWFnbm9zdGljTG9n Z2luZ0NsaWVudCwgRGlhZ25vc3RpY0xvZ2dpbmdLZXlzOjpxdWlya1JlZGlyZWN0Q29taW5nS2V5 KCkpOwogICAgICAgICByZWplY3RSZWFzb25zIHw9IDEgPDwgUXVpY2tSZWRpcmVjdENvbWluZzsK ICAgICB9Ci0gICAgaWYgKGZyYW1lLmxvYWRlcigpLmRvY3VtZW50TG9hZGVyKCktPmlzTG9hZGlu Z0luQVBJU2Vuc2UoKSkgewotICAgICAgICBQQ0xPRygiICAgLURvY3VtZW50TG9hZGVyIGlzIHN0 aWxsIGxvYWRpbmcgaW4gQVBJIHNlbnNlIik7Ci0gICAgICAgIGxvZ1BhZ2VDYWNoZUZhaWx1cmVE aWFnbm9zdGljTWVzc2FnZShkaWFnbm9zdGljTG9nZ2luZ0NsaWVudCwgRGlhZ25vc3RpY0xvZ2dp bmdLZXlzOjpsb2FkaW5nQVBJU2Vuc2VLZXkoKSk7Ci0gICAgICAgIHJlamVjdFJlYXNvbnMgfD0g MSA8PCBJc0xvYWRpbmdJbkFQSVNlbnNlOworICAgIGlmIChmcmFtZS5sb2FkZXIoKS5kb2N1bWVu dExvYWRlcigpLT5pc0xvYWRpbmcoKSkgeworICAgICAgICBQQ0xPRygiICAgLURvY3VtZW50TG9h ZGVyIGlzIHN0aWxsIGxvYWRpbmciKTsKKyAgICAgICAgbG9nUGFnZUNhY2hlRmFpbHVyZURpYWdu b3N0aWNNZXNzYWdlKGRpYWdub3N0aWNMb2dnaW5nQ2xpZW50LCBEaWFnbm9zdGljTG9nZ2luZ0tl eXM6OmlzTG9hZGluZygpKTsKKyAgICAgICAgcmVqZWN0UmVhc29ucyB8PSAxIDw8IElzTG9hZGlu ZzsKICAgICB9CiAgICAgaWYgKGZyYW1lLmxvYWRlcigpLmRvY3VtZW50TG9hZGVyKCktPmlzU3Rv cHBpbmcoKSkgewogICAgICAgICBQQ0xPRygiICAgLURvY3VtZW50TG9hZGVyIGlzIGluIHRoZSBt aWRkbGUgb2Ygc3RvcHBpbmciKTsKQEAgLTMwOCw3ICszMDgsNyBAQCBib29sIFBhZ2VDYWNoZTo6 Y2FuQ2FjaGVQYWdlQ29udGFpbmluZ1RoaXNGcmFtZShGcmFtZSYgZnJhbWUpCiAgICAgICAgICYm ICEoZnJhbWUuaXNNYWluRnJhbWUoKSAmJiBkb2N1bWVudC0+dXJsKCkucHJvdG9jb2xJcygiaHR0 cHMiKSAmJiBkb2N1bWVudExvYWRlci0+cmVzcG9uc2UoKS5jYWNoZUNvbnRyb2xDb250YWluc05v U3RvcmUoKSkKICAgICAgICAgJiYgZnJhbWVMb2FkZXIuaGlzdG9yeSgpLmN1cnJlbnRJdGVtKCkK ICAgICAgICAgJiYgIWZyYW1lTG9hZGVyLnF1aWNrUmVkaXJlY3RDb21pbmcoKQotICAgICAgICAm JiAhZG9jdW1lbnRMb2FkZXItPmlzTG9hZGluZ0luQVBJU2Vuc2UoKQorICAgICAgICAmJiAhZG9j dW1lbnRMb2FkZXItPmlzTG9hZGluZygpCiAgICAgICAgICYmICFkb2N1bWVudExvYWRlci0+aXNT dG9wcGluZygpCiAgICAgICAgICYmIGRvY3VtZW50LT5jYW5TdXNwZW5kQWN0aXZlRE9NT2JqZWN0 c0ZvclBhZ2VDYWNoZSgpCiAgICAgICAgIC8vIEZJWE1FOiBXZSBzaG91bGQgaW52ZXN0aWdhdGlu ZyBjYWNoaW5nIGZyYW1lcyB0aGF0IGhhdmUgYW4gYXNzb2NpYXRlZApkaWZmIC0tZ2l0IGEvU291 cmNlL1dlYkNvcmUvcGFnZS9EaWFnbm9zdGljTG9nZ2luZ0tleXMuY3BwIGIvU291cmNlL1dlYkNv cmUvcGFnZS9EaWFnbm9zdGljTG9nZ2luZ0tleXMuY3BwCmluZGV4IDFhMGYzZDcxZTgxMTkzZjc0 MWQzYWI4YmY4YTU3MzM3MjQ2MzdhNWQuLjVmZjQxOTc0NjcxNzY5M2E2YmQ0Y2E0NzJkZjgyNDNk YTM1ZTcxZDAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BhZ2UvRGlhZ25vc3RpY0xvZ2dp bmdLZXlzLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wYWdlL0RpYWdub3N0aWNMb2dnaW5nS2V5 cy5jcHAKQEAgLTI0OCw5ICsyNDgsOSBAQCBTdHJpbmcgRGlhZ25vc3RpY0xvZ2dpbmdLZXlzOjpy ZWFzb25LZXkoKQogICAgIHJldHVybiBBU0NJSUxpdGVyYWwoInJlYXNvbiIpOwogfQogCi1TdHJp bmcgRGlhZ25vc3RpY0xvZ2dpbmdLZXlzOjpsb2FkaW5nQVBJU2Vuc2VLZXkoKQorU3RyaW5nIERp YWdub3N0aWNMb2dnaW5nS2V5czo6aXNMb2FkaW5nKCkKIHsKLSAgICByZXR1cm4gQVNDSUlMaXRl cmFsKCJsb2FkaW5nQVBJU2Vuc2UiKTsKKyAgICByZXR1cm4gQVNDSUlMaXRlcmFsKCJpc0xvYWRp bmciKTsKIH0KIAogU3RyaW5nIERpYWdub3N0aWNMb2dnaW5nS2V5czo6ZG9jdW1lbnRMb2FkZXJT dG9wcGluZ0tleSgpCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wYWdlL0RpYWdub3N0aWNM b2dnaW5nS2V5cy5oIGIvU291cmNlL1dlYkNvcmUvcGFnZS9EaWFnbm9zdGljTG9nZ2luZ0tleXMu aAppbmRleCBjNGRkYjJmYjE2OGYzNTBjNzVkNThkZmQ4MDZkMWM0N2MzZWUzY2JkLi4yOTI3M2U3 M2NmYzg1ZTc5YmU2NDY3N2U3YTI1M2IwZTkzM2FiZWVlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2Vi Q29yZS9wYWdlL0RpYWdub3N0aWNMb2dnaW5nS2V5cy5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL3Bh Z2UvRGlhZ25vc3RpY0xvZ2dpbmdLZXlzLmgKQEAgLTYwLDcgKzYwLDcgQEAgcHVibGljOgogICAg IFdFQkNPUkVfRVhQT1JUIHN0YXRpYyBTdHJpbmcgaXNSZWxvYWRJZ25vcmluZ0NhY2hlRGF0YUtl eSgpOwogICAgIHN0YXRpYyBTdHJpbmcgbG9hZGVkS2V5KCk7CiAgICAgc3RhdGljIFN0cmluZyBs b2FkaW5nS2V5KCk7Ci0gICAgc3RhdGljIFN0cmluZyBsb2FkaW5nQVBJU2Vuc2VLZXkoKTsKKyAg ICBzdGF0aWMgU3RyaW5nIGlzTG9hZGluZygpOwogICAgIHN0YXRpYyBTdHJpbmcgbWFpbkRvY3Vt ZW50RXJyb3JLZXkoKTsKICAgICBzdGF0aWMgU3RyaW5nIG1haW5SZXNvdXJjZUtleSgpOwogICAg IHN0YXRpYyBTdHJpbmcgbWVkaWFLZXkoKTsK