94368 2012-08-17 11:21:51 -0700 [WK2] Get rid of WebContext::m_pendingMessagesToPostToInjectedBundle 2012-10-02 16:35:04 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 98210 1 ap ap andersca jberlin sam oldest_to_newest 698505 0 ap 2012-08-17 11:21:51 -0700 I'm very much unsure about this patch, because there must have been a reason to do this. However, the patch that added m_pendingMessagesToPostToInjectedBundle didn't list any rationale, and I couldn't catch this array being used in debugger, even with some extensions enabled. Initialization messages are now kept separate, WebProcessProxy has its own m_pendingMessages array in case someone sends other messages early. So, the only case that isn't covered is when WebProcessProxy::isValid() returns false. I do not see how queueing messages sent to a process in this state can be of any use - we'll start from a clean slate when relaunching it anyway. 698511 1 159165 ap 2012-08-17 11:24:49 -0700 Created attachment 159165 proposed patch 699760 2 159165 ap 2012-08-20 11:58:53 -0700 Comment on attachment 159165 proposed patch Some things to consider: - Perhaps injectedBundleInitializationUserData should be sent before all other queued process messages. If we rely on process proxy queueing, that could result in some injected bundle messages being sent before injected bundle initialization. - If we drop any messages on the floor, consider returning a boolean from postMessageToInjectedBundle(). 728067 3 ap 2012-09-25 16:18:33 -0700 > - Perhaps injectedBundleInitializationUserData should be sent before all other queued process messages. If we rely on process proxy queueing, that could result in some injected bundle messages being sent before injected bundle initialization. WebProcessProxy::sendMessage only does queueing when the process is launching, so WebProcess::InitializeWebProcess is always the first message already, and it carries injectedBundleInitializationUserData. > - If we drop any messages on the floor, consider returning a boolean from postMessageToInjectedBundle(). I think that if we're to do something like this, it should be done separately, in a fix that has known observable effect. Just returning false would not be terribly helpful, because that wouldn't tell the caller how many of the processes were offline. 728080 4 165701 ap 2012-09-25 16:33:51 -0700 Created attachment 165701 Patch updated to ToT 728081 5 165701 andersca 2012-09-25 16:37:14 -0700 Comment on attachment 165701 Patch updated to ToT View in context: https://bugs.webkit.org/attachment.cgi?id=165701&action=review > Source/WebKit2/UIProcess/WebContext.cpp:567 > + // FIXME: Can we encode the message body outside the loop for all the processes? Please add another FIXME here too - we should fail to send the message if the body contains any references to WKPageRefs/WKFrameRefs etc since they're local to a process. 728084 6 ap 2012-09-25 16:52:20 -0700 Committed <http://trac.webkit.org/changeset/129575>. 159165 2012-08-17 11:24:49 -0700 2012-09-25 16:33:51 -0700 proposed patch PendingMessageQueue.txt text/plain 2946 ap SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDEyNTkxNCkKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBACisyMDEyLTA4LTE3ICBBbGV4ZXkg UHJvc2t1cnlha292ICA8YXBAYXBwbGUuY29tPgorCisgICAgICAgIFtXSzJdIEdldCByaWQgb2Yg V2ViQ29udGV4dDo6bV9wZW5kaW5nTWVzc2FnZXNUb1Bvc3RUb0luamVjdGVkQnVuZGxlCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD05NDM2OAorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEl0IGRvZXNuJ3QgYXBw ZWFyIHRvIGJlIG9mIGFueSB1c2UuCisKKyAgICAgICAgKiBVSVByb2Nlc3MvV2ViQ29udGV4dC5j cHA6CisgICAgICAgIChXZWJLaXQ6OldlYkNvbnRleHQ6OmNyZWF0ZU5ld1dlYlByb2Nlc3MpOgor ICAgICAgICAoV2ViS2l0OjpXZWJDb250ZXh0Ojpwb3N0TWVzc2FnZVRvSW5qZWN0ZWRCdW5kbGUp OgorICAgICAgICAqIFVJUHJvY2Vzcy9XZWJDb250ZXh0Lmg6CisKIDIwMTItMDgtMTcgIENsYXVk aW8gU2FhdmVkcmEgIDxjc2FhdmVkcmFAaWdhbGlhLmNvbT4KIAogICAgICAgICBbR1RLXSBBZGQg V0syIEFQSSB0byBnZXQgc3VnZ2VzdGVkIGZpbGVuYW1lIGZvciBVUkkgcmVzcG9uc2VzCkluZGV4 OiBTb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViQ29udGV4dC5jcHAKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g U291cmNlL1dlYktpdDIvVUlQcm9jZXNzL1dlYkNvbnRleHQuY3BwCShyZXZpc2lvbiAxMjU3OTYp CisrKyBTb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViQ29udGV4dC5jcHAJKHdvcmtpbmcgY29w eSkKQEAgLTMzOCwxMyArMzM4LDYgQEAgUGFzc1JlZlB0cjxXZWJQcm9jZXNzUHJveHk+IFdlYkNv bnRleHQ6OgogICAgICAgICBpbmplY3RlZEJ1bmRsZUluaXRpYWxpemF0aW9uVXNlckRhdGEgPSBt X2luamVjdGVkQnVuZGxlSW5pdGlhbGl6YXRpb25Vc2VyRGF0YTsKICAgICBwcm9jZXNzLT5zZW5k KE1lc3NhZ2VzOjpXZWJQcm9jZXNzOjpJbml0aWFsaXplV2ViUHJvY2VzcyhwYXJhbWV0ZXJzLCBX ZWJDb250ZXh0VXNlck1lc3NhZ2VFbmNvZGVyKGluamVjdGVkQnVuZGxlSW5pdGlhbGl6YXRpb25V c2VyRGF0YS5nZXQoKSkpLCAwKTsKIAotICAgIGZvciAoc2l6ZV90IGkgPSAwOyBpICE9IG1fcGVu ZGluZ01lc3NhZ2VzVG9Qb3N0VG9JbmplY3RlZEJ1bmRsZS5zaXplKCk7ICsraSkgewotICAgICAg ICBwYWlyPFN0cmluZywgUmVmUHRyPEFQSU9iamVjdD4gPiYgbWVzc2FnZSA9IG1fcGVuZGluZ01l c3NhZ2VzVG9Qb3N0VG9JbmplY3RlZEJ1bmRsZVtpXTsKLSAgICAgICAgcHJvY2Vzcy0+ZGVwcmVj YXRlZFNlbmQoSW5qZWN0ZWRCdW5kbGVNZXNzYWdlOjpQb3N0TWVzc2FnZSwgMCwgQ29yZUlQQzo6 SW4obWVzc2FnZS5maXJzdCwgV2ViQ29udGV4dFVzZXJNZXNzYWdlRW5jb2RlcihtZXNzYWdlLnNl Y29uZC5nZXQoKSkpKTsKLSAgICB9Ci0gICAgLy8gRklYTUUgKE11bHRpLVdlYlByb2Nlc3MpOiBX aGF0IGRvZXMgdGhpcyBtZWFuIGluIHRoZSBicmF2ZSBuZXcgd29ybGQ/Ci0gICAgbV9wZW5kaW5n TWVzc2FnZXNUb1Bvc3RUb0luamVjdGVkQnVuZGxlLmNsZWFyKCk7Ci0KICAgICByZXR1cm4gcHJv Y2Vzcy5yZWxlYXNlKCk7CiB9CiAKQEAgLTUxNSwxMCArNTA4LDggQEAgRG93bmxvYWRQcm94eSog V2ViQ29udGV4dDo6ZG93bmxvYWQoV2ViUAogdm9pZCBXZWJDb250ZXh0Ojpwb3N0TWVzc2FnZVRv SW5qZWN0ZWRCdW5kbGUoY29uc3QgU3RyaW5nJiBtZXNzYWdlTmFtZSwgQVBJT2JqZWN0KiBtZXNz YWdlQm9keSkKIHsKICAgICBpZiAobV9wcm9jZXNzTW9kZWwgPT0gUHJvY2Vzc01vZGVsU2hhcmVk U2Vjb25kYXJ5UHJvY2VzcykgewotICAgICAgICBpZiAobV9wcm9jZXNzZXMuaXNFbXB0eSgpIHx8 ICFtX3Byb2Nlc3Nlc1swXS0+Y2FuU2VuZE1lc3NhZ2UoKSkgewotICAgICAgICAgICAgbV9wZW5k aW5nTWVzc2FnZXNUb1Bvc3RUb0luamVjdGVkQnVuZGxlLmFwcGVuZChzdGQ6Om1ha2VfcGFpciht ZXNzYWdlTmFtZSwgbWVzc2FnZUJvZHkpKTsKKyAgICAgICAgaWYgKG1fcHJvY2Vzc2VzLmlzRW1w dHkoKSkKICAgICAgICAgICAgIHJldHVybjsKLSAgICAgICAgfQogCiAgICAgICAgIC8vIEZJWE1F OiBXZSBzaG91bGQgY29uc2lkZXIgcmV0dXJuaW5nIGZhbHNlIGZyb20gdGhpcyBmdW5jdGlvbiBp ZiB0aGUgbWVzc2FnZUJvZHkgY2Fubm90CiAgICAgICAgIC8vIGJlIGVuY29kZWQuCkluZGV4OiBT b3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViQ29udGV4dC5oCj09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJj ZS9XZWJLaXQyL1VJUHJvY2Vzcy9XZWJDb250ZXh0LmgJKHJldmlzaW9uIDEyNTc5NikKKysrIFNv dXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9XZWJDb250ZXh0LmgJKHdvcmtpbmcgY29weSkKQEAgLTI5 Nyw4ICsyOTcsNiBAQCBwcml2YXRlOgogICAgIGJvb2wgbV9hbHdheXNVc2VzQ29tcGxleFRleHRD b2RlUGF0aDsKICAgICBib29sIG1fc2hvdWxkVXNlRm9udFNtb290aGluZzsKIAotICAgIFZlY3Rv cjxwYWlyPFN0cmluZywgUmVmUHRyPEFQSU9iamVjdD4gPiA+IG1fcGVuZGluZ01lc3NhZ2VzVG9Q b3N0VG9JbmplY3RlZEJ1bmRsZTsKLQogICAgIENhY2hlTW9kZWwgbV9jYWNoZU1vZGVsOwogCiAg ICAgV2ViRG93bmxvYWRDbGllbnQgbV9kb3dubG9hZENsaWVudDsK 165701 2012-09-25 16:33:51 -0700 2012-09-25 16:37:14 -0700 Patch updated to ToT PendingMessages.txt text/plain 3474 ap SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDEyOTU3MikKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBACisyMDEyLTA5LTI1ICBBbGV4ZXkg UHJvc2t1cnlha292ICA8YXBAYXBwbGUuY29tPgorCisgICAgICAgIEdldCByaWQgb2YgV2ViQ29u dGV4dDo6bV9wZW5kaW5nTWVzc2FnZXNUb1Bvc3RUb0luamVjdGVkQnVuZGxlCisgICAgICAgIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD05NDM2OAorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEl0IGRvZXNuJ3QgYXBwZWFyIHRv IGJlIG9mIGFueSB1c2UuCisKKyAgICAgICAgKiBVSVByb2Nlc3MvV2ViQ29udGV4dC5jcHA6Cisg ICAgICAgIChXZWJLaXQ6OldlYkNvbnRleHQ6OmNyZWF0ZU5ld1dlYlByb2Nlc3MpOgorICAgICAg ICAoV2ViS2l0OjpXZWJDb250ZXh0Ojpwb3N0TWVzc2FnZVRvSW5qZWN0ZWRCdW5kbGUpOgorICAg ICAgICAqIFVJUHJvY2Vzcy9XZWJDb250ZXh0Lmg6CisKIDIwMTItMDktMjQgIFNpbW9uIEZyYXNl ciAgPHNpbW9uLmZyYXNlckBhcHBsZS5jb20+CiAKICAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzEy MzUxOTA2PiBIYXZlIER1bXBSZW5kZXJUcmVlIGFuZCBXZWJLaXRUZXN0UnVubmVyIGNyYXNoIGxv Z3Mgc2hvdyB3aGljaCB0ZXN0IGNyYXNoZWQKSW5kZXg6IFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vz cy9XZWJDb250ZXh0LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3Mv V2ViQ29udGV4dC5jcHAJKHJldmlzaW9uIDEyOTAzNikKKysrIFNvdXJjZS9XZWJLaXQyL1VJUHJv Y2Vzcy9XZWJDb250ZXh0LmNwcAkod29ya2luZyBjb3B5KQpAQCAtMzY2LDEzICszNjYsNiBAQCBQ YXNzUmVmUHRyPFdlYlByb2Nlc3NQcm94eT4gV2ViQ29udGV4dDo6CiAgICAgICAgIGluamVjdGVk QnVuZGxlSW5pdGlhbGl6YXRpb25Vc2VyRGF0YSA9IG1faW5qZWN0ZWRCdW5kbGVJbml0aWFsaXph dGlvblVzZXJEYXRhOwogICAgIHByb2Nlc3MtPnNlbmQoTWVzc2FnZXM6OldlYlByb2Nlc3M6Oklu aXRpYWxpemVXZWJQcm9jZXNzKHBhcmFtZXRlcnMsIFdlYkNvbnRleHRVc2VyTWVzc2FnZUVuY29k ZXIoaW5qZWN0ZWRCdW5kbGVJbml0aWFsaXphdGlvblVzZXJEYXRhLmdldCgpKSksIDApOwogCi0g ICAgZm9yIChzaXplX3QgaSA9IDA7IGkgIT0gbV9wZW5kaW5nTWVzc2FnZXNUb1Bvc3RUb0luamVj dGVkQnVuZGxlLnNpemUoKTsgKytpKSB7Ci0gICAgICAgIHBhaXI8U3RyaW5nLCBSZWZQdHI8QVBJ T2JqZWN0PiA+JiBtZXNzYWdlID0gbV9wZW5kaW5nTWVzc2FnZXNUb1Bvc3RUb0luamVjdGVkQnVu ZGxlW2ldOwotICAgICAgICBwcm9jZXNzLT5kZXByZWNhdGVkU2VuZChJbmplY3RlZEJ1bmRsZU1l c3NhZ2U6OlBvc3RNZXNzYWdlLCAwLCBDb3JlSVBDOjpJbihtZXNzYWdlLmZpcnN0LCBXZWJDb250 ZXh0VXNlck1lc3NhZ2VFbmNvZGVyKG1lc3NhZ2Uuc2Vjb25kLmdldCgpKSkpOwotICAgIH0KLSAg ICAvLyBGSVhNRSAoTXVsdGktV2ViUHJvY2VzcykgKDk0MzY4KTogV2hhdCBkb2VzIHRoaXMgbWVh biBpbiB0aGUgYnJhdmUgbmV3IHdvcmxkPwotICAgIG1fcGVuZGluZ01lc3NhZ2VzVG9Qb3N0VG9J bmplY3RlZEJ1bmRsZS5jbGVhcigpOwotCiAgICAgcmV0dXJuIHByb2Nlc3MucmVsZWFzZSgpOwog fQogCkBAIC01NjYsMTggKzU1OSwxMiBAQCBEb3dubG9hZFByb3h5KiBXZWJDb250ZXh0Ojpkb3du bG9hZChXZWJQCiAKIHZvaWQgV2ViQ29udGV4dDo6cG9zdE1lc3NhZ2VUb0luamVjdGVkQnVuZGxl KGNvbnN0IFN0cmluZyYgbWVzc2FnZU5hbWUsIEFQSU9iamVjdCogbWVzc2FnZUJvZHkpCiB7Ci0g ICAgaWYgKG1fcHJvY2Vzc2VzLmlzRW1wdHkoKSkgewotICAgICAgICBtX3BlbmRpbmdNZXNzYWdl c1RvUG9zdFRvSW5qZWN0ZWRCdW5kbGUuYXBwZW5kKHN0ZDo6bWFrZV9wYWlyKG1lc3NhZ2VOYW1l LCBtZXNzYWdlQm9keSkpOworICAgIGlmIChtX3Byb2Nlc3Nlcy5pc0VtcHR5KCkpCiAgICAgICAg IHJldHVybjsKLSAgICB9CiAKICAgICBmb3IgKHNpemVfdCBpID0gMDsgaSA8IG1fcHJvY2Vzc2Vz LnNpemUoKTsgKytpKSB7Ci0gICAgICAgIC8vIEZJWE1FIChNdWx0aS1XZWJQcm9jZXNzKTogRXZv bHZlIG1fcGVuZGluZ01lc3NhZ2VzVG9Qb3N0VG9JbmplY3RlZEJ1bmRsZSB0byB3b3JrIHdpdGgg bXVsdGlwbGUgc2Vjb25kYXJ5IHByb2Nlc3Nlcy4KLSAgICAgICAgaWYgKCFtX3Byb2Nlc3Nlc1tp XS0+Y2FuU2VuZE1lc3NhZ2UoKSkgewotICAgICAgICAgICAgbV9wZW5kaW5nTWVzc2FnZXNUb1Bv c3RUb0luamVjdGVkQnVuZGxlLmFwcGVuZChzdGQ6Om1ha2VfcGFpcihtZXNzYWdlTmFtZSwgbWVz c2FnZUJvZHkpKTsKLSAgICAgICAgICAgIGNvbnRpbnVlOwotICAgICAgICB9CiAgICAgICAgIC8v IEZJWE1FOiBXZSBzaG91bGQgY29uc2lkZXIgcmV0dXJuaW5nIGZhbHNlIGZyb20gdGhpcyBmdW5j dGlvbiBpZiB0aGUgbWVzc2FnZUJvZHkgY2Fubm90IGJlIGVuY29kZWQuCisgICAgICAgIC8vIEZJ WE1FOiBDYW4gd2UgZW5jb2RlIHRoZSBtZXNzYWdlIGJvZHkgb3V0c2lkZSB0aGUgbG9vcCBmb3Ig YWxsIHRoZSBwcm9jZXNzZXM/CiAgICAgICAgIG1fcHJvY2Vzc2VzW2ldLT5kZXByZWNhdGVkU2Vu ZChJbmplY3RlZEJ1bmRsZU1lc3NhZ2U6OlBvc3RNZXNzYWdlLCAwLCBDb3JlSVBDOjpJbihtZXNz YWdlTmFtZSwgV2ViQ29udGV4dFVzZXJNZXNzYWdlRW5jb2RlcihtZXNzYWdlQm9keSkpKTsKICAg ICB9CiB9CkluZGV4OiBTb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViQ29udGV4dC5oCj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9XZWJDb250ZXh0LmgJKHJldmlzaW9u IDEyOTAzNikKKysrIFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9XZWJDb250ZXh0LmgJKHdvcmtp bmcgY29weSkKQEAgLTMwNyw4ICszMDcsNiBAQCBwcml2YXRlOgogICAgIGJvb2wgbV9hbHdheXNV c2VzQ29tcGxleFRleHRDb2RlUGF0aDsKICAgICBib29sIG1fc2hvdWxkVXNlRm9udFNtb290aGlu ZzsKIAotICAgIFZlY3RvcjxwYWlyPFN0cmluZywgUmVmUHRyPEFQSU9iamVjdD4gPiA+IG1fcGVu ZGluZ01lc3NhZ2VzVG9Qb3N0VG9JbmplY3RlZEJ1bmRsZTsKLQogICAgIENhY2hlTW9kZWwgbV9j YWNoZU1vZGVsOwogCiAgICAgV2ViRG93bmxvYWRDbGllbnQgbV9kb3dubG9hZENsaWVudDsK