|Product:||WebKit||Reporter:||Arunprasad Rajkumar <arurajku>|
|Component:||WebKit Qt||Assignee:||Nobody <webkit-unassigned>|
|Severity:||Normal||CC:||allan.jensen, ararunprasad, commit-queue, hausmann, jturcotte, kenneth, robert|
|Version:||528+ (Nightly build)|
|Bug Depends on:||120403|
Description Arunprasad Rajkumar 2013-08-14 03:26:00 PDT
Comment 2 Jocelyn Turcotte 2013-08-21 03:10:52 PDT
Comment on attachment 209210 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209210&action=review > Source/WebKit/qt/ChangeLog:9 > + DRT introduced by http://trac.webkit.org/r61433. We usually just write r61433 without the full URL in ChangeLogs. > Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:164 > + // Load empty url to send onunload event to currently running page. > + // onunload event is mandatory for LayoutTests/plugins/open-and-close-window-with-plugin.html and > + // LayoutTests/plugins/geturlnotify-during-document-teardown.html. Please mention that this has to do with the virtual table still being intact at this point.
Comment 4 Jocelyn Turcotte 2013-08-21 09:23:19 PDT
Comment on attachment 209266 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209266&action=review > Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:166 > + // lead(eg: calling window.alert from window.onunload) to call the > + // virtual functions of the partially deleted WebPage object. > + // At this point vptr of QWebPage points to the derived class WebPage. This is enough information but still too confusing: - "from ~QWebPage will lead" -> "from ~QWebPage would lead". You want to avoid it. - It won't "call the virtual functions of the partially deleted WebPage object" in ~QWebPage, it will call virtual functions of QWebPage as the WebPage part of the vtable has already been unwinded. - "At this point vptr of QWebPage points to the derived class WebPage.", at which point, I assume that you mean here in ~WebPage? Please clarify. - "vptr of QWebPage" -> "the vtable of QWebPage" - "eg" -> "e.g."
Comment 6 Arunprasad Rajkumar 2013-08-22 12:13:02 PDT
(In reply to comment #5) > Created an attachment (id=209382) [details] > Patch Some of the LayoutTests for "http" were failing regardless of this fix. May be my Layout Test setup issue.
Comment 7 Jocelyn Turcotte 2013-08-23 02:49:17 PDT
Comment 9 Jocelyn Turcotte 2013-08-26 02:42:17 PDT
Comment on attachment 209490 [details] Patch Thanks :)
Comment 10 WebKit Commit Bot 2013-08-26 03:06:22 PDT
Comment on attachment 209490 [details] Patch Clearing flags on attachment: 209490 Committed r154593: <http://trac.webkit.org/changeset/154593>
Comment 11 WebKit Commit Bot 2013-08-26 03:06:26 PDT
All reviewed patches have been landed. Closing bug.
Comment 12 Allan Sandfeld Jensen 2013-08-28 05:24:01 PDT
This patch caused an additional 50+ tests to become flaky with Qt 5.1 on WebKit1 http://build.webkit.sed.hu/builders/x86-64 Linux Qt Release - Qt5.1 - WebKit1?numbuilds=100