43930 2010-08-12 13:50:22 -0700 Apparent error in FrameLoaderClientQt::createFrame 2010-08-25 22:02:16 -0700 1 1 1 Unclassified WebKit WebKit Qt 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Normal --- 0 leo.yang webkit-unassigned commit-queue eric hausmann kenneth koivisto tonikitoo oldest_to_newest 263953 0 leo.yang 2010-08-12 13:50:22 -0700 frameData.frame->loader()->loadURLIntoChildFrame(frameData.url, frameData.referrer, frameData.frame.get()); Apparent error: calling loadURLIntoChildFrame with child frame's loader. 263973 1 64262 leo.yang 2010-08-12 14:25:01 -0700 Created attachment 64262 Patch 264675 2 64262 kenneth 2010-08-14 07:57:26 -0700 Comment on attachment 64262 Patch How come this having been breaking anything? 265084 3 leo.yang 2010-08-16 07:21:14 -0700 (In reply to comment #2) > (From update of attachment 64262 [details]) > How come this having been breaking anything? Some logic in loadURLIntoChildFrame is unreachable. We can't run into the following if statement. And if we add some operations which are related with parent frame in loadURLIntoChildFrame in the future, it's buggy for qt porting. 917 if (parentItem && parentItem->children().size() && isBackForwardLoadType(loadType)) { 918 HistoryItem* childItem = parentItem->childItemWithTarget(childFrame->tree()->name()); 919 if (childItem) { 920 // Use the original URL to ensure we get all the side-effects, such as 921 // onLoad handlers, of any redirects that happened. An example of where 922 // this is needed is Radar 3213556. 923 workingURL = KURL(ParsedURLString, childItem->originalURLString()); 924 childLoadType = loadType; 925 childFrame->loader()->history()->setProvisionalItem(childItem); 926 } 927 } 267835 4 64262 commit-queue 2010-08-23 04:23:32 -0700 Comment on attachment 64262 Patch Rejecting patch 64262 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--wait-for-httpd', '--ignore-tests', 'compositing', '--quiet']" exit_code: 1 Running build-dumprendertree Compiling Java tests make: Nothing to be done for `default'. Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 20894 test cases. media/video-preload.html -> crashed Exiting early after 1 failures. 17255 tests run. 651.04s total testing time 17254 test cases (99%) succeeded 1 test case (<1%) crashed 36 test cases (<1%) had stderr output Full output: http://queues.webkit.org/results/3753576 269231 5 leo.yang 2010-08-25 07:16:44 -0700 (In reply to comment #4) > (From update of attachment 64262 [details]) > Rejecting patch 64262 from commit-queue. > > Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--wait-for-httpd', '--ignore-tests', 'compositing', '--quiet']" exit_code: 1 > Running build-dumprendertree > Compiling Java tests > make: Nothing to be done for `default'. > Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests > Testing 20894 test cases. > media/video-preload.html -> crashed > > Exiting early after 1 failures. 17255 tests run. > 651.04s total testing time > > 17254 test cases (99%) succeeded > 1 test case (<1%) crashed > 36 test cases (<1%) had stderr output > > Full output: http://queues.webkit.org/results/3753576 Code changes in the patch are only related to QT port. I tested using QtTestBrowser, it works fine for media/video-preload.html. So is it false positive? 269297 6 64262 eric 2010-08-25 09:48:02 -0700 Comment on attachment 64262 Patch The media tests a very flaky on leopard due to a CoreVideo bug. 269370 7 leo.yang 2010-08-25 11:16:40 -0700 (In reply to comment #6) > (From update of attachment 64262 [details]) > The media tests a very flaky on leopard due to a CoreVideo bug. Can someone who has commit access help land this patch? 269372 8 eric 2010-08-25 11:17:18 -0700 The cq will land it shortly. 269394 9 64262 commit-queue 2010-08-25 11:31:59 -0700 Comment on attachment 64262 Patch Clearing flags on attachment: 64262 Committed r66030: <http://trac.webkit.org/changeset/66030> 269395 10 commit-queue 2010-08-25 11:32:04 -0700 All reviewed patches have been landed. Closing bug. 269712 11 tonikitoo 2010-08-25 22:02:16 -0700 - frameData.frame->loader()->loadURLIntoChildFrame(frameData.url, frameData.referrer, frameData.frame.get()); + m_frame->loader()->loadURLIntoChildFrame(frameData.url, frameData.referrer, frameData.frame.get()); That is a great catch! Did not it fixed anything? If not, no unit tests or regression tests added? :-( It could happen to be broken again... 64262 2010-08-12 14:25:01 -0700 2010-08-25 11:31:59 -0700 Patch 0001-2010-08-12-Leo-Yang-leo.yang-torchmobile.com.cn.patch text/plain 2152 leo.yang RnJvbSBjNTMxNGNiZjFhYjIzYWI2NTljZjc2N2E1NzA1NmY0YTAxNDYxMTJmIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBMZW8gWWFuZyA8bGVvLnlhbmdAdG9yY2htb2JpbGUuY29tLmNu PgpEYXRlOiBUaHUsIDEyIEF1ZyAyMDEwIDE3OjIxOjQ1IC0wNDAwClN1YmplY3Q6IFtQQVRDSF0g MjAxMC0wOC0xMiAgTGVvIFlhbmcgIDxsZW8ueWFuZ0B0b3JjaG1vYmlsZS5jb20uY24+CgogICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgoKICAgICAgICBJbiBGcmFtZUxvYWRlckNs aWVudFF0OjpjcmVhdGVGcmFtZSB3ZSBzaG91bGQgY2FsbCBsb2FkVVJMSW50b0NoaWxkRnJhbWUK ICAgICAgICB3aXRoIHBhcmVudCBmcmFtZSdzIGxvYWRlciBpbnN0ZWFkIG9mIGNoaWxkIGZyYW1l J3MgbG9hZGVyLgogICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD00MzkzMAoKICAgICAgICAqIFdlYkNvcmVTdXBwb3J0L0ZyYW1lTG9hZGVyQ2xpZW50UXQuY3Bw OgogICAgICAgIChXZWJDb3JlOjpGcmFtZUxvYWRlckNsaWVudFF0OjpjcmVhdGVGcmFtZSk6Ci0t LQogV2ViS2l0L3F0L0NoYW5nZUxvZyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHwgICAx MSArKysrKysrKysrKwogV2ViS2l0L3F0L1dlYkNvcmVTdXBwb3J0L0ZyYW1lTG9hZGVyQ2xpZW50 UXQuY3BwIHwgICAgMiArLQogMiBmaWxlcyBjaGFuZ2VkLCAxMiBpbnNlcnRpb25zKCspLCAxIGRl bGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL1dlYktpdC9xdC9DaGFuZ2VMb2cgYi9XZWJLaXQvcXQv Q2hhbmdlTG9nCmluZGV4IDliYTdiZjguLjI0MTYxNTIgMTAwNjQ0Ci0tLSBhL1dlYktpdC9xdC9D aGFuZ2VMb2cKKysrIGIvV2ViS2l0L3F0L0NoYW5nZUxvZwpAQCAtMSwzICsxLDE0IEBACisyMDEw LTA4LTEyICBMZW8gWWFuZyAgPGxlby55YW5nQHRvcmNobW9iaWxlLmNvbS5jbj4KKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBJbiBGcmFtZUxvYWRlckNs aWVudFF0OjpjcmVhdGVGcmFtZSB3ZSBzaG91bGQgY2FsbCBsb2FkVVJMSW50b0NoaWxkRnJhbWUK KyAgICAgICAgd2l0aCBwYXJlbnQgZnJhbWUncyBsb2FkZXIgaW5zdGVhZCBvZiBjaGlsZCBmcmFt ZSdzIGxvYWRlci4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTQzOTMwCisKKyAgICAgICAgKiBXZWJDb3JlU3VwcG9ydC9GcmFtZUxvYWRlckNsaWVudFF0 LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkZyYW1lTG9hZGVyQ2xpZW50UXQ6OmNyZWF0ZUZyYW1l KToKKwogMjAxMC0wOC0xMiAgSmVyZW15IE9ybG93ICA8am9ybG93QGNocm9taXVtLm9yZz4KIAog ICAgICAgICBSZXZlcnQgZm9yIG5vdwpkaWZmIC0tZ2l0IGEvV2ViS2l0L3F0L1dlYkNvcmVTdXBw b3J0L0ZyYW1lTG9hZGVyQ2xpZW50UXQuY3BwIGIvV2ViS2l0L3F0L1dlYkNvcmVTdXBwb3J0L0Zy YW1lTG9hZGVyQ2xpZW50UXQuY3BwCmluZGV4IGNmNzZkYWUuLmI4YTcyMTkgMTAwNjQ0Ci0tLSBh L1dlYktpdC9xdC9XZWJDb3JlU3VwcG9ydC9GcmFtZUxvYWRlckNsaWVudFF0LmNwcAorKysgYi9X ZWJLaXQvcXQvV2ViQ29yZVN1cHBvcnQvRnJhbWVMb2FkZXJDbGllbnRRdC5jcHAKQEAgLTExOTAs NyArMTE5MCw3IEBAIFBhc3NSZWZQdHI8RnJhbWU+IEZyYW1lTG9hZGVyQ2xpZW50UXQ6OmNyZWF0 ZUZyYW1lKGNvbnN0IEtVUkwmIHVybCwgY29uc3QgU3RyaW5nCiAKICAgICAvLyAjIyMgc2V0IG92 ZXJyaWRlIGVuY29kaW5nIGlmIHdlIGhhdmUgb25lCiAKLSAgICBmcmFtZURhdGEuZnJhbWUtPmxv YWRlcigpLT5sb2FkVVJMSW50b0NoaWxkRnJhbWUoZnJhbWVEYXRhLnVybCwgZnJhbWVEYXRhLnJl ZmVycmVyLCBmcmFtZURhdGEuZnJhbWUuZ2V0KCkpOworICAgIG1fZnJhbWUtPmxvYWRlcigpLT5s b2FkVVJMSW50b0NoaWxkRnJhbWUoZnJhbWVEYXRhLnVybCwgZnJhbWVEYXRhLnJlZmVycmVyLCBm cmFtZURhdGEuZnJhbWUuZ2V0KCkpOwogCiAgICAgLy8gVGhlIGZyYW1lJ3Mgb25sb2FkIGhhbmRs ZXIgbWF5IGhhdmUgcmVtb3ZlZCBpdCBmcm9tIHRoZSBkb2N1bWVudC4KICAgICBpZiAoIWZyYW1l RGF0YS5mcmFtZS0+dHJlZSgpLT5wYXJlbnQoKSkKLS0gCjEuNi40LjIKCg==