74691 2011-12-16 00:09:43 -0800 [Qt] Eliminate dependency to QUndoStack 2011-12-16 12:55:31 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 hausmann hausmann cmarcelo jesus menard webkit.review.bot zoltan oldest_to_newest 522592 0 hausmann 2011-12-16 00:09:43 -0800 [Qt] Eliminate dependency to QUndoStack 522593 1 119577 hausmann 2011-12-16 00:12:02 -0800 Created attachment 119577 Patch 522594 2 hausmann 2011-12-16 00:12:59 -0800 Once this is in, the next step is to get rid of the class entirely and just fold the few lines of code straight to the page client, if that's okay with you folks :) 522710 3 cmarcelo 2011-12-16 05:51:08 -0800 Looks good to me. I don't think we are getting any benefit from using QUndoStack. Regarding moving this to QtPageClient, I like the fact that QtPageClient is super-thin and just forward things to the proper object handling them. I'm afraid folding even this small chunk of code will put page client in another route... but that could be me just being a bit over-cautious ;-) 522741 4 menard 2011-12-16 06:49:27 -0800 (In reply to comment #3) > Looks good to me. I don't think we are getting any benefit from using QUndoStack. > > Regarding moving this to QtPageClient, I like the fact that QtPageClient is super-thin and just forward things to the proper object handling them. I'm afraid folding even this small chunk of code will put page client in another route... but that could be me just being a bit over-cautious ;-) +1 tend to agree with that. This is exactly how we ended up with QtWebPageProxy. 522901 5 hausmann 2011-12-16 12:06:37 -0800 (In reply to comment #3) > Looks good to me. I don't think we are getting any benefit from using QUndoStack. > > Regarding moving this to QtPageClient, I like the fact that QtPageClient is super-thin and just forward things to the proper object handling them. I'm afraid folding even this small chunk of code will put page client in another route... but that could be me just being a bit over-cautious ;-) I understand your fear, but I'm not sure if it is really another classes responsibility to handle the undo command storage. It is the _intend_ of the C API that the page ui client handles it, so why should we delegate it even further? The QtWebPageProxy became big because it implemented multiple client APIs in one shot. Anyway, I'll try to convince you with code :) - in the meantime we need to get this patch reviewed :) 522911 6 119577 kenneth 2011-12-16 12:12:53 -0800 Comment on attachment 119577 Patch im fine with it. 522941 7 119577 hausmann 2011-12-16 12:40:48 -0800 Comment on attachment 119577 Patch Thanks Kenneth :) 522959 8 119577 webkit.review.bot 2011-12-16 12:55:26 -0800 Comment on attachment 119577 Patch Clearing flags on attachment: 119577 Committed r103097: <http://trac.webkit.org/changeset/103097> 522960 9 webkit.review.bot 2011-12-16 12:55:31 -0800 All reviewed patches have been landed. Closing bug. 119577 2011-12-16 00:12:02 -0800 2011-12-16 12:55:26 -0800 Patch bug-74691-20111216091201.patch text/plain 5323 hausmann U3VidmVyc2lvbiBSZXZpc2lvbjogMTAzMDMwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggZDM0YjI0YTYwYzI0ZGFh MWQ4MTYzZmNhODY2ODM4MDA2YWY2MjQ2Ny4uYTJhMzQ3ZDk2NmRmZWEzZDdkMDM2M2JlYzQ3YTQ4 OWQ2ZTI4NDk0MiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIxIEBACisyMDExLTEyLTE2ICBTaW1v biBIYXVzbWFubiAgPHNpbW9uLmhhdXNtYW5uQG5va2lhLmNvbT4KKworICAgICAgICBbUXRdIEVs aW1pbmF0ZSBkZXBlbmRlbmN5IHRvIFFVbmRvU3RhY2sKKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTc0NjkxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgUmVwbGFjZWQgdGhlIFFVbmRvU3RhY2sgd2l0aCB0d28g dmVjdG9ycy4gV2hlbiBjYWxsaW5nIHVuYXBwbHkoKQorICAgICAgICBvbiB0aGUgZWRpdCBjb21t YW5kIHByb3h5LCBpdCB3aWxsIGF1dG9tYXRpY2FsbHkgcmUtcmVnaXN0ZXIgaXRzZWxmCisgICAg ICAgIGluIHRoZSByZWRvIHN0YWNrLgorCisgICAgICAgICogVUlQcm9jZXNzL3F0L1F0V2ViVW5k b0NvbnRyb2xsZXIuY3BwOgorICAgICAgICAoUXRXZWJVbmRvQ29udHJvbGxlcjo6cmVnaXN0ZXJF ZGl0Q29tbWFuZCk6CisgICAgICAgIChRdFdlYlVuZG9Db250cm9sbGVyOjpjbGVhckFsbEVkaXRD b21tYW5kcyk6CisgICAgICAgIChRdFdlYlVuZG9Db250cm9sbGVyOjpjYW5VbmRvUmVkbyk6Cisg ICAgICAgIChRdFdlYlVuZG9Db250cm9sbGVyOjpleGVjdXRlVW5kb1JlZG8pOgorICAgICAgICAq IFVJUHJvY2Vzcy9xdC9RdFdlYlVuZG9Db250cm9sbGVyLmg6CisKIDIwMTEtMTItMTUgIEFuZGVy cyBDYXJsc3NvbiAgPGFuZGVyc2NhQGFwcGxlLmNvbT4KIAogICAgICAgICBBZGQgc3VwcG9ydCBm b3IgYWNjZWxlcmF0ZWQgY29tcG9zaXRpbmcgdG8gdGhlIHRpbGVkIENvcmUgQW5pbWF0aW9uIGRy YXdpbmcgYXJlYQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL3F0L1F0V2Vi VW5kb0NvbnRyb2xsZXIuY3BwIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL3F0L1F0V2ViVW5k b0NvbnRyb2xsZXIuY3BwCmluZGV4IDVkNjcxOTk2M2YxZjdiYTU3ZDBjNDQ5NGU0Mzc2MzczOGI3 YTVlMDkuLjYyZDMxZThhYjc3MDZhYTQ5ZmZlYTA5OTg5MzQ0OTgwNWQ4MTQwZWYgMTAwNjQ0Ci0t LSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9xdC9RdFdlYlVuZG9Db250cm9sbGVyLmNwcAor KysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvcXQvUXRXZWJVbmRvQ29udHJvbGxlci5jcHAK QEAgLTIxLDk2ICsyMSw0MyBAQAogI2luY2x1ZGUgImNvbmZpZy5oIgogI2luY2x1ZGUgIlF0V2Vi VW5kb0NvbnRyb2xsZXIuaCIKIAotI2luY2x1ZGUgIldlYkVkaXRDb21tYW5kUHJveHkuaCIKICNp bmNsdWRlIDxxZ2xvYmFsLmg+CiAjaW5jbHVkZSA8d3RmL1JlZlB0ci5oPgogCiB1c2luZyBuYW1l c3BhY2UgV2ViS2l0OwogCi1jbGFzcyBRdFdlYlVuZG9Db21tYW5kIDogcHVibGljIFFVbmRvQ29t bWFuZCB7Ci1wdWJsaWM6Ci0gICAgUXRXZWJVbmRvQ29tbWFuZChQYXNzUmVmUHRyPFdlYkVkaXRD b21tYW5kUHJveHk+LCBRVW5kb0NvbW1hbmQqIHBhcmVudCA9IDApOwotICAgIH5RdFdlYlVuZG9D b21tYW5kKCk7Ci0KLSAgICB2b2lkIHJlZG8oKTsKLSAgICB2b2lkIHVuZG8oKTsKLQotICAgIGJv b2wgaW5VbmRvUmVkbygpIGNvbnN0IHsgcmV0dXJuIG1faW5VbmRvUmVkbzsgfTsKLQotcHJpdmF0 ZToKLSAgICBSZWZQdHI8V2ViRWRpdENvbW1hbmRQcm94eT4gbV9jb21tYW5kOwotICAgIGJvb2wg bV9maXJzdDsKLSAgICBib29sIG1faW5VbmRvUmVkbzsKLX07Ci0KLVF0V2ViVW5kb0NvbW1hbmQ6 OlF0V2ViVW5kb0NvbW1hbmQoUGFzc1JlZlB0cjxXZWJFZGl0Q29tbWFuZFByb3h5PiBjb21tYW5k LCBRVW5kb0NvbW1hbmQqIHBhcmVudCkKLSAgICA6IFFVbmRvQ29tbWFuZChwYXJlbnQpCi0gICAg LCBtX2NvbW1hbmQoY29tbWFuZCkKLSAgICAsIG1fZmlyc3QodHJ1ZSkKLSAgICAsIG1faW5VbmRv UmVkbyhmYWxzZSkKLXsKLX0KLQotUXRXZWJVbmRvQ29tbWFuZDo6flF0V2ViVW5kb0NvbW1hbmQo KQotewotfQotCi12b2lkIFF0V2ViVW5kb0NvbW1hbmQ6OnJlZG8oKQotewotICAgIG1faW5VbmRv UmVkbyA9IHRydWU7Ci0KLSAgICAvLyBJZ25vcmUgdGhlIGZpcnN0IHJlZG8gY2FsbGVkIGZyb20g UVVuZG9TdGFjazo6cHVzaCgpLgotICAgIGlmIChtX2ZpcnN0KSB7Ci0gICAgICAgIG1fZmlyc3Qg PSBmYWxzZTsKLSAgICAgICAgbV9pblVuZG9SZWRvID0gZmFsc2U7Ci0gICAgICAgIHJldHVybjsK LSAgICB9Ci0gICAgaWYgKG1fY29tbWFuZCkKLSAgICAgICAgbV9jb21tYW5kLT5yZWFwcGx5KCk7 Ci0KLSAgICBtX2luVW5kb1JlZG8gPSBmYWxzZTsKLX0KLQotdm9pZCBRdFdlYlVuZG9Db21tYW5k Ojp1bmRvKCkKLXsKLSAgICBtX2luVW5kb1JlZG8gPSB0cnVlOwotCi0gICAgaWYgKG1fY29tbWFu ZCkKLSAgICAgICAgbV9jb21tYW5kLT51bmFwcGx5KCk7Ci0KLSAgICBtX2luVW5kb1JlZG8gPSBm YWxzZTsKLX0KLQotUXRXZWJVbmRvQ29udHJvbGxlcjo6UXRXZWJVbmRvQ29udHJvbGxlcigpCi17 Ci19Ci0KIHZvaWQgUXRXZWJVbmRvQ29udHJvbGxlcjo6cmVnaXN0ZXJFZGl0Q29tbWFuZChQYXNz UmVmUHRyPFdlYkVkaXRDb21tYW5kUHJveHk+IGNvbW1hbmQsIFdlYlBhZ2VQcm94eTo6VW5kb09y UmVkbyB1bmRvT3JSZWRvKQogewotICAgIGlmICh1bmRvT3JSZWRvID09IFdlYlBhZ2VQcm94eTo6 VW5kbykgewotICAgICAgICBjb25zdCBRdFdlYlVuZG9Db21tYW5kKiB3ZWJVbmRvQ29tbWFuZCA9 IHN0YXRpY19jYXN0PGNvbnN0IFF0V2ViVW5kb0NvbW1hbmQqPihtX3VuZG9TdGFjay5jb21tYW5k KG1fdW5kb1N0YWNrLmluZGV4KCkpKTsKLSAgICAgICAgaWYgKHdlYlVuZG9Db21tYW5kICYmIHdl YlVuZG9Db21tYW5kLT5pblVuZG9SZWRvKCkpCi0gICAgICAgICAgICByZXR1cm47Ci0gICAgICAg IG1fdW5kb1N0YWNrLnB1c2gobmV3IFF0V2ViVW5kb0NvbW1hbmQoY29tbWFuZCkpOwotICAgIH0K KyAgICBpZiAodW5kb09yUmVkbyA9PSBXZWJQYWdlUHJveHk6OlVuZG8pCisgICAgICAgIG1fdW5k b1N0YWNrLmFwcGVuZChjb21tYW5kKTsKKyAgICBlbHNlCisgICAgICAgIG1fcmVkb1N0YWNrLmFw cGVuZChjb21tYW5kKTsKIH0KIAogdm9pZCBRdFdlYlVuZG9Db250cm9sbGVyOjpjbGVhckFsbEVk aXRDb21tYW5kcygpCiB7CiAgICAgbV91bmRvU3RhY2suY2xlYXIoKTsKKyAgICBtX3JlZG9TdGFj ay5jbGVhcigpOwogfQogCiBib29sIFF0V2ViVW5kb0NvbnRyb2xsZXI6OmNhblVuZG9SZWRvKFdl YlBhZ2VQcm94eTo6VW5kb09yUmVkbyB1bmRvT3JSZWRvKQogewogICAgIGlmICh1bmRvT3JSZWRv ID09IFdlYlBhZ2VQcm94eTo6VW5kbykKLSAgICAgICAgcmV0dXJuIG1fdW5kb1N0YWNrLmNhblVu ZG8oKTsKLSAgICByZXR1cm4gbV91bmRvU3RhY2suY2FuUmVkbygpOworICAgICAgICByZXR1cm4g IW1fdW5kb1N0YWNrLmlzRW1wdHkoKTsKKyAgICBlbHNlCisgICAgICAgIHJldHVybiAhbV9yZWRv U3RhY2suaXNFbXB0eSgpOwogfQogCiB2b2lkIFF0V2ViVW5kb0NvbnRyb2xsZXI6OmV4ZWN1dGVV bmRvUmVkbyhXZWJQYWdlUHJveHk6OlVuZG9PclJlZG8gdW5kb09yUmVkbykKIHsKLSAgICBpZiAo dW5kb09yUmVkbyA9PSBXZWJQYWdlUHJveHk6OlVuZG8pCi0gICAgICAgIG1fdW5kb1N0YWNrLnVu ZG8oKTsKLSAgICBlbHNlCi0gICAgICAgIG1fdW5kb1N0YWNrLnJlZG8oKTsKKyAgICBSZWZQdHI8 V2ViRWRpdENvbW1hbmRQcm94eT4gY29tbWFuZDsKKyAgICBpZiAodW5kb09yUmVkbyA9PSBXZWJQ YWdlUHJveHk6OlVuZG8pIHsKKyAgICAgICAgY29tbWFuZCA9IG1fdW5kb1N0YWNrLmxhc3QoKTsK KyAgICAgICAgbV91bmRvU3RhY2sucmVtb3ZlTGFzdCgpOworICAgICAgICBjb21tYW5kLT51bmFw cGx5KCk7CisgICAgfSBlbHNlIHsKKyAgICAgICAgY29tbWFuZCA9IG1fcmVkb1N0YWNrLmxhc3Qo KTsKKyAgICAgICAgbV9yZWRvU3RhY2sucmVtb3ZlTGFzdCgpOworICAgICAgICBjb21tYW5kLT5y ZWFwcGx5KCk7CisgICAgfQogfQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNz L3F0L1F0V2ViVW5kb0NvbnRyb2xsZXIuaCBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9xdC9R dFdlYlVuZG9Db250cm9sbGVyLmgKaW5kZXggOTA0ZjcxNGY2NmMxYzA0MGZjNGQ0MDcyNDVjYThm YzUwZmMyYjA3Ny4uYTI2MGNhMmZkM2RlNWY2M2MyNTk4ZjZiYjk2MTE0NGYwYjM1OGZlYiAxMDA2 NDQKLS0tIGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL3F0L1F0V2ViVW5kb0NvbnRyb2xsZXIu aAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvcXQvUXRXZWJVbmRvQ29udHJvbGxlci5o CkBAIC0yMiwyMyArMjIsMjAgQEAKICNkZWZpbmUgUXRXZWJVbmRvQ29udHJvbGxlcl9oCiAKICNp bmNsdWRlICJQYWdlQ2xpZW50LmgiCisjaW5jbHVkZSAiV2ViRWRpdENvbW1hbmRQcm94eS5oIgog I2luY2x1ZGUgIldlYlBhZ2VQcm94eS5oIgotI2luY2x1ZGUgPFFVbmRvU3RhY2s+CiAKIGNsYXNz IFF0V2ViVW5kb0NvbnRyb2xsZXIgewogcHVibGljOgotICAgIFF0V2ViVW5kb0NvbnRyb2xsZXIo KTsKLQotcHJpdmF0ZToKLSAgICBmcmllbmQgY2xhc3MgUXRQYWdlQ2xpZW50OwotCiAgICAgLy8g UGFnZSBDbGllbnQuCiAgICAgdm9pZCByZWdpc3RlckVkaXRDb21tYW5kKFBhc3NSZWZQdHI8V2Vi S2l0OjpXZWJFZGl0Q29tbWFuZFByb3h5PiwgV2ViS2l0OjpXZWJQYWdlUHJveHk6OlVuZG9PclJl ZG8pOwogICAgIHZvaWQgY2xlYXJBbGxFZGl0Q29tbWFuZHMoKTsKICAgICBib29sIGNhblVuZG9S ZWRvKFdlYktpdDo6V2ViUGFnZVByb3h5OjpVbmRvT3JSZWRvKTsKICAgICB2b2lkIGV4ZWN1dGVV bmRvUmVkbyhXZWJLaXQ6OldlYlBhZ2VQcm94eTo6VW5kb09yUmVkbyk7CiAKLSAgICBRVW5kb1N0 YWNrIG1fdW5kb1N0YWNrOworICAgIHR5cGVkZWYgVmVjdG9yPFJlZlB0cjxXZWJLaXQ6OldlYkVk aXRDb21tYW5kUHJveHk+ID4gQ29tbWFuZFZlY3RvcjsKKyAgICBDb21tYW5kVmVjdG9yIG1fdW5k b1N0YWNrOworICAgIENvbW1hbmRWZWN0b3IgbV9yZWRvU3RhY2s7CiB9OwogCiAjZW5kaWYgLy8g UXRXZWJVbmRvQ29udHJvbGxlcl9oCg==