Bug 37684

(In reply to comment #0) > The attached patch improves this. It is necessary to mention this bug report on the Changelog. Also, do you want it to be reviewed? If so you have to change it to r? . Kenneth, can you take a look at this? LGTM. ;)

I guess you wanted to put this up for review

I did want to put it up for review. Sorry about that :-)

Comment on attachment 53489 [details] patch that reduces memcpy'ing Looks good Anders!

Comment on attachment 53489 [details] patch that reduces memcpy'ing I'll add the bug number to the changelog and upload a new patch. Just following the guidelines... ;)

Created attachment 55837 [details] Patch Added the bug number to the Changelog.

Comment on attachment 53489 [details] patch that reduces memcpy'ing Clearing flags of old patch.

Comment on attachment 55837 [details] Patch Rejecting patch 55837 from commit-queue. Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--force']" exit_code: 2 Parsed 2 diffs from patch file(s). patching file WebCore/ChangeLog patch: **** malformed patch at line 18: 2010-04-15 Adam Roben <aroben@apple.com> patching file WebCore/bridge/qt/qt_class.cpp Full output: http://webkit-commit-queue.appspot.com/results/2315058

Created attachment 56062 [details] New version of the patch

Comment on attachment 56062 [details] New version of the patch Rejecting patch 56062 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--ignore-tests', 'compositing/iframes', '--quiet']" exit_code: 1 Last 500 characters of output: ng Java tests make: Nothing to be done for `default'. Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Skipped list contained 'compositing/iframes/composited-iframe.html', but no file of that name could be found Testing 18341 test cases. fast/loader/recursive-before-unload-crash.html -> failed Exiting early after 1 failures. 13417 tests run. 198.65s total testing time 13416 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 4 test cases (<1%) had stderr output Full output: http://webkit-commit-queue.appspot.com/results/2265108

Anders, you cannot remove the "Reviewed by NOBODY (OOPS!!)". If you do that we cannot land using the commit-queue. It has to be exactly like the prepare-ChangeLog scripts writes it. Now you can reupload the patch with this fixed and reset r? and cq? -- or -- You can upload the patch with "Reviewed by Kenneth Rohde Christiansen" and just set cq? The latter is preferred as anyone with committer-rights can change the cq? to cq+ Another option would be to get someone to land it manually using webkit-patch or similar.

Comment on attachment 56062 [details] New version of the patch I think you were just bit by a flaky test.

Comment on attachment 56062 [details] New version of the patch Clearing flags on attachment: 56062 Committed r59563: <http://trac.webkit.org/changeset/59563>

All reviewed patches have been landed. Closing bug.

Revision r59563 cherry-picked into qtwebkit-2.0 with commit 4bb49bb59d4f763069ae6bb84b9648d6ed0bfd1d

This commit causes a bug which prevents signals from being matched. It introduces second local variable named "index", which then overrides the proper method number variable (also called "index") this causes signals from being found in some cases. The affected line is: QtRuntimeMetaMethod* val = new (exec) QtRuntimeMetaMethod(exec, identifier, static_cast<QtInstance*>(inst), index, normal, false);

Uh, I wasn't able to see that from the diff. I will fix it later today

Patch shadowed a variable.

Fixed regression in r59606

Oops. My bad.

<cherr-pick-for-backport: r59606>

<cherry-pick-for-backport: r59606>

Revision r59606 cherry-picked into qtwebkit-2.0 with commit 807157e42add842605ec67d9363dd3f1861748ca