146450 2015-06-30 02:18:28 -0700 [WinCairo] Compile error, WebEditorClient::didApplyStyle() should not have any parameters. 2015-06-30 10:39:46 -0700 1 1 1 Unclassified WebKit WebKit Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 peavo webkit-unassigned achristensen bfulgham commit-queue darin ossy rniwa oldest_to_newest 1105646 0 peavo 2015-06-30 02:18:28 -0700 The abstract method EditClient::didApplyStyle() does not have any parameters, WebEditorClient::didApplyStyle() should have the same signature. 1105647 1 255815 peavo 2015-06-30 02:20:58 -0700 Created attachment 255815 Patch 1105656 2 255815 ossy 2015-06-30 03:13:12 -0700 Comment on attachment 255815 Patch LGTM, r=me. Could you add an override keyword before landing to avoid possible bugs in the future here? ( Just note: This code was introduced in bug146379 - https://trac.webkit.org/changeset/186086 ) 1105657 3 peavo 2015-06-30 03:28:04 -0700 (In reply to comment #2) > Comment on attachment 255815 [details] > Patch > > LGTM, r=me. Could you add an override keyword before > landing to avoid possible bugs in the future here? > > ( Just note: This code was introduced in bug146379 - > https://trac.webkit.org/changeset/186086 ) Thanks for reviewing :) Yes, I can add the override keyword, but I seem to remember there was an agreement not to add the override keyword when overriding pure virtual methods? 1105658 4 255815 ossy 2015-06-30 03:38:18 -0700 Comment on attachment 255815 Patch I missed this agreement. But I agree, adding override keyword for overriding pure virtual function isn't necessary. Let's land it as is. 1105665 5 peavo 2015-06-30 03:47:00 -0700 (In reply to comment #4) > Comment on attachment 255815 [details] > Patch > > I missed this agreement. But I agree, adding override keyword for > overriding pure virtual function isn't necessary. Let's land it as is. Ok, thanks! 1105668 6 255815 commit-queue 2015-06-30 04:26:59 -0700 Comment on attachment 255815 Patch Clearing flags on attachment: 255815 Committed r186114: <http://trac.webkit.org/changeset/186114> 1105669 7 commit-queue 2015-06-30 04:27:05 -0700 All reviewed patches have been landed. Closing bug. 1105705 8 darin 2015-06-30 10:13:22 -0700 (In reply to comment #3) > I seem to remember there was an > agreement not to add the override keyword when overriding pure virtual > methods? This is the first I have heard of this. I think we should definitely use override in cases like that and I’d like to know more about why we wouldn’t. 1105719 9 peavo 2015-06-30 10:39:46 -0700 (In reply to comment #8) > (In reply to comment #3) > > I seem to remember there was an > > agreement not to add the override keyword when overriding pure virtual > > methods? > > This is the first I have heard of this. I think we should definitely use > override in cases like that and I’d like to know more about why we wouldn’t. Ok, sorry for spreading false rumours. I thought I picked this up somewhere, but I am probably wrong. 255815 2015-06-30 02:20:58 -0700 2015-06-30 04:26:59 -0700 Patch bug-146450-20150630111942.patch text/plain 2033 peavo SW5kZXg6IFNvdXJjZS9XZWJLaXQvd2luL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv V2ViS2l0L3dpbi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE4NjExMSkKKysrIFNvdXJjZS9XZWJLaXQv d2luL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDE1LTA2LTMw ICBQZXIgQXJuZSBWb2xsYW4gIDxwZWF2b0BvdXRsb29rLmNvbT4KKworICAgICAgICBbV2luQ2Fp cm9dIENvbXBpbGUgZXJyb3IsIFdlYkVkaXRvckNsaWVudDo6ZGlkQXBwbHlTdHlsZSgpIHNob3Vs ZCBub3QgaGF2ZSBhbnkgcGFyYW1ldGVycy4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTE0NjQ1MAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgICogV2ViQ29yZVN1cHBvcnQvV2ViRWRpdG9yQ2xpZW50LmNwcDoK KyAgICAgICAgKFdlYkVkaXRvckNsaWVudDo6c2hvdWxkQXBwbHlTdHlsZSk6CisgICAgICAgIChX ZWJFZGl0b3JDbGllbnQ6OmRpZEFwcGx5U3R5bGUpOgorICAgICAgICAqIFdlYkNvcmVTdXBwb3J0 L1dlYkVkaXRvckNsaWVudC5oOgorCiAyMDE1LTA2LTI3ICBSeW9zdWtlIE5pd2EgIDxybml3YUB3 ZWJraXQub3JnPgogCiAgICAgICAgIEZvbnQgcGFuZWwgZG9lc24ndCBnZXQgdXBkYXRlZCB3aGVu IGJvbGRpbmcgdGV4dCB2aWEgY21kK2IgaW4gTWFpbCBvbiBPUyBYCkluZGV4OiBTb3VyY2UvV2Vi S2l0L3dpbi9XZWJDb3JlU3VwcG9ydC9XZWJFZGl0b3JDbGllbnQuY3BwCj09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0t IFNvdXJjZS9XZWJLaXQvd2luL1dlYkNvcmVTdXBwb3J0L1dlYkVkaXRvckNsaWVudC5jcHAJKHJl dmlzaW9uIDE4NjExMSkKKysrIFNvdXJjZS9XZWJLaXQvd2luL1dlYkNvcmVTdXBwb3J0L1dlYkVk aXRvckNsaWVudC5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTM0NSw3ICszNDUsNyBAQCBib29sIFdl YkVkaXRvckNsaWVudDo6c2hvdWxkQXBwbHlTdHlsZShTCiAgICAgcmV0dXJuIHRydWU7CiB9CiAK LXZvaWQgV2ViRWRpdG9yQ2xpZW50OjpkaWRBcHBseVN0eWxlKFN0eWxlUHJvcGVydGllcyosIFJh bmdlKikKK3ZvaWQgV2ViRWRpdG9yQ2xpZW50OjpkaWRBcHBseVN0eWxlKCkKIHsKICAgICBub3RJ bXBsZW1lbnRlZCgpOwogfQpJbmRleDogU291cmNlL1dlYktpdC93aW4vV2ViQ29yZVN1cHBvcnQv V2ViRWRpdG9yQ2xpZW50LmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYktpdC93aW4vV2ViQ29y ZVN1cHBvcnQvV2ViRWRpdG9yQ2xpZW50LmgJKHJldmlzaW9uIDE4NjExMSkKKysrIFNvdXJjZS9X ZWJLaXQvd2luL1dlYkNvcmVTdXBwb3J0L1dlYkVkaXRvckNsaWVudC5oCSh3b3JraW5nIGNvcHkp CkBAIC02Niw3ICs2Niw3IEBAIHB1YmxpYzoKIAogICAgIGJvb2wgc2hvdWxkSW5zZXJ0Tm9kZShX ZWJDb3JlOjpOb2RlKiwgV2ViQ29yZTo6UmFuZ2UqIHJlcGxhY2luZ1JhbmdlLCBXZWJDb3JlOjpF ZGl0b3JJbnNlcnRBY3Rpb24pOwogICAgIGJvb2wgc2hvdWxkQXBwbHlTdHlsZShXZWJDb3JlOjpT dHlsZVByb3BlcnRpZXMqLCBXZWJDb3JlOjpSYW5nZSopOwotICAgIHZvaWQgZGlkQXBwbHlTdHls ZShXZWJDb3JlOjpTdHlsZVByb3BlcnRpZXMqLCBXZWJDb3JlOjpSYW5nZSopOworICAgIHZvaWQg ZGlkQXBwbHlTdHlsZSgpOwogICAgIGJvb2wgc2hvdWxkTW92ZVJhbmdlQWZ0ZXJEZWxldGUoV2Vi Q29yZTo6UmFuZ2UqLCBXZWJDb3JlOjpSYW5nZSopOwogICAgIGJvb2wgc2hvdWxkQ2hhbmdlVHlw aW5nU3R5bGUoV2ViQ29yZTo6U3R5bGVQcm9wZXJ0aWVzKiBjdXJyZW50U3R5bGUsIFdlYkNvcmU6 OlN0eWxlUHJvcGVydGllcyogdG9Qcm9wb3NlZFN0eWxlKTsKIAo=