129147 2014-02-21 03:09:30 -0800 Fix crash in Youtube site when loading is finished. 2019-05-03 06:58:50 -0700 1 1 1 Unclassified WebKit WebKit EFL 528+ (Nightly build) Unspecified Unspecified RESOLVED INVALID P2 Normal --- 1 jh718.park webkit-unassigned ap koivisto lucas.de.marchi simon.fraser zalan oldest_to_newest 983166 0 jh718.park 2014-02-21 03:09:30 -0800 In EFL MiniBrowser, Youtube site crashes when loading is finished. 983175 1 jh718.park 2014-02-21 04:54:05 -0800 Reproduce paths are as below: common precondition: In Tools/MiniBrowser/efl/main.c, comment out a statement below. //ewk_settings_prefferred_minimum_contents_width_set(settings, 0); 1. execute EFL MiniBrowser with options like below for using fixed layout ./MiniBrowser -s 720x1280 -r 1.0 -T 1 -L 1 http://www.youtube.com 2. In WebChromeClient::dispatchViewportPropertiesDidChange(), revise if(m_page->useFixedLayout()) - > if(m_page->useFixedLayout()) for the test is the equivalence because the revision makes it as if it uses fixed layout and it leads to the same code flow as 1. 983196 2 224862 jh718.park 2014-02-21 06:58:00 -0800 Created attachment 224862 Patch 983202 3 224863 jh718.park 2014-02-21 07:54:38 -0800 Created attachment 224863 Patch 983203 4 224864 jh718.park 2014-02-21 07:57:19 -0800 Created attachment 224864 Patch 983961 5 224864 ap 2014-02-24 11:15:34 -0800 Comment on attachment 224864 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=224864&action=review > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1361 > +#if PLATFORM(EFL) > + view->scheduleRelayout(); > +#else > view->forceLayout(); > +#endif This doesn't look like it can be right. Why is Efl different? 983967 6 224864 simon.fraser 2014-02-24 11:18:41 -0800 Comment on attachment 224864 Patch Patch is way too mysterious. 984137 7 jh718.park 2014-02-24 18:29:59 -0800 As far as I know, currently Only EFL browser uses fixed layout, and other browser is not. So when using fixed layout, in WebChromeClient::dispatchViewportPropertiesDidChange() the code flow leads to the m_page->sendViewportAttributesChanged(); while other browsers not using fixed layout leads to return; becuase of the statement if (!m_page->useFixedLayout()) return; That's why I made this patch, but if it is not reasonable, I will withdraw it. 984145 8 simon.fraser 2014-02-24 18:43:12 -0800 If that's true then: 1. Your channgelog should have explained that 2. Your patch should do if (usingFixedLayout)... rather than a #if PLATFORM BTW, iOS uses fixed layout and doesn't crash. 984180 9 jh718.park 2014-02-24 21:17:41 -0800 The crash's cause is as below: 1. In ViewportStyleResolver::resolve(), m_document->updateViewportArguments() is called and in there, forcelayout() is called sequentially, and it leads to the deletion of the ViewportStyleResolver itself. 2. This time, the remained statement in ViewportStyleResolver::resolve(), i.e. m_propertySet = 0; is called. It means the access to already deleted ViewportStyleResolver instance's member variable. It means that with some conditions, the log is displayed as below When I put the logs in ViewportStyleResolver::resolve()'s start and end part, respectively. I would be glad if I can get to know what the condition is exactly. ViewportStyleResolver::resolve(1) start ViewportStyleResolver::resolve(2) start ViewportStyleResolver::resolve(2) end ViewportStyleResolver::resolve(1) end The above sequence cause the deletion of the ViewportStyleResolver even it's member function's execution is not completed yet. FYI, When I executed EFL MiniBrowser with -r(resolution) 1.0 and iteration try, I couldn't see the webprocess crash sometimes, but more than 2.0 almost always(-r>=2.0). If it's not occured in iOS, then it will be needed to know the difference of efl and iOS in code flow from the statement, m_document->updateViewportArguments() in ViewportStyleResolver::resolve(). In the next changeLog and patch, I'll reflect the comment Mr.Simon Fraser have left. BTW, I have a curiosity. In the first patch, I just only used scheduleRelayout() not with #if PLATFORM(...), and I faced an build error in mac and mac-wk2. Why is it? 987858 10 zalan 2014-03-06 14:25:08 -0800 (In reply to comment #8) > If that's true then: > 1. Your channgelog should have explained that > 2. Your patch should do if (usingFixedLayout)... rather than a #if PLATFORM > > BTW, iOS uses fixed layout and doesn't crash. The difference is that iOS takes a different codepath at WebChromeClient::dispatchViewportPropertiesDidChange. By taking a quick look at the code, it seems EFL ends up segfaulting due to calling layout recursively. setFixedLayoutSize() -> forceLayout() -> layout() -> styleResolverChanged() -> updateActiveStyleSheets() -> appendAuthorStyleSheets() -> viewportStyleResolver()->resolve() -> m_document->updateViewportArguments() -> dispatchViewportPropertiesDidChange() -> sendViewportAttributesChanged() -> setFixedLayoutSize() Provided we are inside this loop, calling scheduleRelayout() at this point is a noop as scheduling is disabled for the FrameView. (as a side node, bug 52309 introduced this API and I am not sure if the caller expects sync layout, if it does, i'd branch out and make a new function with this async behavior.) > BTW, I have a curiosity. In the first patch, I just only used scheduleRelayout() not with #if PLATFORM(...), and I faced an build error in mac and mac-wk2. Why is it? scheduleRelayout needs to be exported out if you intend to use it in WebKit2. (WebCore.exp.in) 224862 2014-02-21 06:58:00 -0800 2014-02-21 07:54:31 -0800 Patch bug-129147-20140221235805.patch text/plain 1432 jh718.park U3VidmVyc2lvbiBSZXZpc2lvbjogMTY0NDgxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggZDVhZTEzYjE1MjJhMjc4 ZjdiZDQ0NzJlNjA3ZWU5YmE0MmY5YjE5NS4uYmI1YTdlNzllZmU1ZTAzNzU4YzA5ZDEyZTZkYWQ0 YWVhY2UxZjFhZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBACisyMDE0LTAyLTIxICBKb29u Z2h1biBQYXJrICA8amg3MTgucGFya0BzYW1zdW5nLmNvbT4KKworICAgICAgICBGaXggY3Jhc2gg aW4gWW91dHViZSBzaXRlIHdoZW4gbG9hZGluZyBpcyBmaW5pc2hlZC4KKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEyOTE0Ny4KKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFdlYlByb2Nlc3MvV2ViUGFnZS9X ZWJQYWdlLmNwcDoKKyAgICAgICAgKFdlYktpdDo6V2ViUGFnZTo6c2V0Rml4ZWRMYXlvdXRTaXpl KToKKwogMjAxNC0wMi0yMCAgWmFsYW4gQnVqdGFzICA8emFsYW5AYXBwbGUuY29tPgogCiAgICAg ICAgIFJlbW92ZSByZWR1bmRhbnQgZGV2aWNlU2NhbGVGYWN0b3IoKSBmdW5jdGlvbnMgYW5kIG1h a2UgY2FsbGVycyB1c2UgRG9jdW1lbnQ6OmRldmljZVNjYWxlRmFjdG9yKCkgd2hlbiBhY2Nlc3Np YmxlLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL1dlYlBh Z2UuY3BwIGIvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL1dlYlBhZ2UuY3BwCmlu ZGV4IGIyNTNjMDdiMzFhZTcwMGM2ZGQwNDhmMjdmZTVlMWQzOWVjMjg3YzMuLmE5ODliMTIyNzI4 MDExNWY2NTgyMzdhMTU1MDYxMzY5MGE1YjUwNzUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQy L1dlYlByb2Nlc3MvV2ViUGFnZS9XZWJQYWdlLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQ cm9jZXNzL1dlYlBhZ2UvV2ViUGFnZS5jcHAKQEAgLTEzNTQsNyArMTM1NCw3IEBAIHZvaWQgV2Vi UGFnZTo6c2V0Rml4ZWRMYXlvdXRTaXplKGNvbnN0IEludFNpemUmIHNpemUpCiAgICAgdmlldy0+ c2V0Rml4ZWRMYXlvdXRTaXplKHNpemUpOwogICAgIC8vIERvIG5vdCBmb3JjZSBpdCB1bnRpbCB0 aGUgZmlyc3QgbGF5b3V0LCB0aGlzIHdvdWxkIHRoZW4gYmVjb21lIG91ciBmaXJzdCBsYXlvdXQg cHJlbWF0dXJlbHkuCiAgICAgaWYgKHZpZXctPmRpZEZpcnN0TGF5b3V0KCkpCi0gICAgICAgIHZp ZXctPmZvcmNlTGF5b3V0KCk7CisgICAgICAgIHZpZXctPnNjaGVkdWxlUmVsYXlvdXQoKTsKIH0K IAogdm9pZCBXZWJQYWdlOjpsaXN0ZW5Gb3JMYXlvdXRNaWxlc3RvbmVzKHVpbnQzMl90IG1pbGVz dG9uZXMpCg== 224863 2014-02-21 07:54:38 -0800 2014-02-21 07:57:12 -0800 Patch bug-129147-20140222005442.patch text/plain 1396 jh718.park U3VidmVyc2lvbiBSZXZpc2lvbjogMTY0NDgxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggZDVhZTEzYjE1MjJhMjc4 ZjdiZDQ0NzJlNjA3ZWU5YmE0MmY5YjE5NS4uYmI1YTdlNzllZmU1ZTAzNzU4YzA5ZDEyZTZkYWQ0 YWVhY2UxZjFhZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBACisyMDE0LTAyLTIxICBKb29u Z2h1biBQYXJrICA8amg3MTgucGFya0BzYW1zdW5nLmNvbT4KKworICAgICAgICBGaXggY3Jhc2gg aW4gWW91dHViZSBzaXRlIHdoZW4gbG9hZGluZyBpcyBmaW5pc2hlZC4KKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEyOTE0Ny4KKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFdlYlByb2Nlc3MvV2ViUGFnZS9X ZWJQYWdlLmNwcDoKKyAgICAgICAgKFdlYktpdDo6V2ViUGFnZTo6c2V0Rml4ZWRMYXlvdXRTaXpl KToKKwogMjAxNC0wMi0yMCAgWmFsYW4gQnVqdGFzICA8emFsYW5AYXBwbGUuY29tPgogCiAgICAg ICAgIFJlbW92ZSByZWR1bmRhbnQgZGV2aWNlU2NhbGVGYWN0b3IoKSBmdW5jdGlvbnMgYW5kIG1h a2UgY2FsbGVycyB1c2UgRG9jdW1lbnQ6OmRldmljZVNjYWxlRmFjdG9yKCkgd2hlbiBhY2Nlc3Np YmxlLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL1dlYlBh Z2UuY3BwIGIvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL1dlYlBhZ2UuY3BwCmlu ZGV4IGIyNTNjMDdiMzFhZTcwMGM2ZGQwNDhmMjdmZTVlMWQzOWVjMjg3YzMuLmYwNTVhMmIyODEy NWU3MjIwMjA1MzJjNDY1Nzg5YmQ1YjEwY2NiNjIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQy L1dlYlByb2Nlc3MvV2ViUGFnZS9XZWJQYWdlLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQ cm9jZXNzL1dlYlBhZ2UvV2ViUGFnZS5jcHAKQEAgLTEzNTQsNiArMTM1NCw5IEBAIHZvaWQgV2Vi UGFnZTo6c2V0Rml4ZWRMYXlvdXRTaXplKGNvbnN0IEludFNpemUmIHNpemUpCiAgICAgdmlldy0+ c2V0Rml4ZWRMYXlvdXRTaXplKHNpemUpOwogICAgIC8vIERvIG5vdCBmb3JjZSBpdCB1bnRpbCB0 aGUgZmlyc3QgbGF5b3V0LCB0aGlzIHdvdWxkIHRoZW4gYmVjb21lIG91ciBmaXJzdCBsYXlvdXQg cHJlbWF0dXJlbHkuCiAgICAgaWYgKHZpZXctPmRpZEZpcnN0TGF5b3V0KCkpCisjaWYgUExBVEZP Uk0oRUZMKQorICAgICAgICB2aWV3LT5zY2hlZHVsZVJlbGF5b3V0KCk7CisjZWxzZQogICAgICAg ICB2aWV3LT5mb3JjZUxheW91dCgpOwogfQogCg== 224864 2014-02-21 07:57:19 -0800 2014-02-24 11:18:41 -0800 Patch bug-129147-20140222005724.patch text/plain 1467 jh718.park U3VidmVyc2lvbiBSZXZpc2lvbjogMTY0NDgxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggZDVhZTEzYjE1MjJhMjc4 ZjdiZDQ0NzJlNjA3ZWU5YmE0MmY5YjE5NS4uYmI1YTdlNzllZmU1ZTAzNzU4YzA5ZDEyZTZkYWQ0 YWVhY2UxZjFhZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBACisyMDE0LTAyLTIxICBKb29u Z2h1biBQYXJrICA8amg3MTgucGFya0BzYW1zdW5nLmNvbT4KKworICAgICAgICBGaXggY3Jhc2gg aW4gWW91dHViZSBzaXRlIHdoZW4gbG9hZGluZyBpcyBmaW5pc2hlZC4KKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEyOTE0Ny4KKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFdlYlByb2Nlc3MvV2ViUGFnZS9X ZWJQYWdlLmNwcDoKKyAgICAgICAgKFdlYktpdDo6V2ViUGFnZTo6c2V0Rml4ZWRMYXlvdXRTaXpl KToKKwogMjAxNC0wMi0yMCAgWmFsYW4gQnVqdGFzICA8emFsYW5AYXBwbGUuY29tPgogCiAgICAg ICAgIFJlbW92ZSByZWR1bmRhbnQgZGV2aWNlU2NhbGVGYWN0b3IoKSBmdW5jdGlvbnMgYW5kIG1h a2UgY2FsbGVycyB1c2UgRG9jdW1lbnQ6OmRldmljZVNjYWxlRmFjdG9yKCkgd2hlbiBhY2Nlc3Np YmxlLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL1dlYlBh Z2UuY3BwIGIvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL1dlYlBhZ2UuY3BwCmlu ZGV4IGIyNTNjMDdiMzFhZTcwMGM2ZGQwNDhmMjdmZTVlMWQzOWVjMjg3YzMuLmQyMmNmMWZmYzMy N2Q5ZjMxMTc1MGM3MGM3YTcyZjQ5ZGY0NmJjMDggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQy L1dlYlByb2Nlc3MvV2ViUGFnZS9XZWJQYWdlLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQ cm9jZXNzL1dlYlBhZ2UvV2ViUGFnZS5jcHAKQEAgLTEzNTQsNyArMTM1NCwxMSBAQCB2b2lkIFdl YlBhZ2U6OnNldEZpeGVkTGF5b3V0U2l6ZShjb25zdCBJbnRTaXplJiBzaXplKQogICAgIHZpZXct PnNldEZpeGVkTGF5b3V0U2l6ZShzaXplKTsKICAgICAvLyBEbyBub3QgZm9yY2UgaXQgdW50aWwg dGhlIGZpcnN0IGxheW91dCwgdGhpcyB3b3VsZCB0aGVuIGJlY29tZSBvdXIgZmlyc3QgbGF5b3V0 IHByZW1hdHVyZWx5LgogICAgIGlmICh2aWV3LT5kaWRGaXJzdExheW91dCgpKQorI2lmIFBMQVRG T1JNKEVGTCkKKyAgICAgICAgdmlldy0+c2NoZWR1bGVSZWxheW91dCgpOworI2Vsc2UKICAgICAg ICAgdmlldy0+Zm9yY2VMYXlvdXQoKTsKKyNlbmRpZgogfQogCiB2b2lkIFdlYlBhZ2U6Omxpc3Rl bkZvckxheW91dE1pbGVzdG9uZXModWludDMyX3QgbWlsZXN0b25lcykK