45938 2010-09-16 18:07:59 -0700 _web_makePluginViewsPerformSelector:: mutates subviews array while iterating it 2010-09-16 21:07:40 -0700 1 1 1 Unclassified WebKit Plug-ins 528+ (Nightly build) Mac All RESOLVED FIXED P2 Normal --- 1 sullivan sullivan oldest_to_newest 280487 0 sullivan 2010-09-16 18:07:59 -0700 This is in Radar as <rdar://problem/8395558> The method -[NSArray _web_makePluginViewsPerformSelector:withObject:], which is implemented in WebHTMLView.mm, uses -[NSArray objectEnumerator] to enumerate the receiver array. The two callers to this method both send it to [self subviews]. [NSView subviews] returns the "live" mutable NSArray holding a view's subviews, so if the selector causes the view's subviews to change, the array will be mutated while it's being enumerated, which is an ObjC no-no that causes an NSException to be thrown (and ensuing havoc). 280490 1 67870 sullivan 2010-09-16 18:14:12 -0700 Created attachment 67870 Patch to avoid mutating array while enumerating it. 280511 2 67870 darin 2010-09-16 19:40:47 -0700 Comment on attachment 67870 Patch to avoid mutating array while enumerating it. Historically, objectEnumerator creating a copy of the entire array! I’m surprised that it has changed so that it no longer does so. Does this work properly when there are no subviews? Instead of initWithArray: you could have used the copy method. r=me 280523 3 sullivan 2010-09-16 21:04:09 -0700 The fast enumeration introduced with ObjC-2.0 is used with objectEnumerator; that's when the prohibition against mutating a collection while enumerating it began. Other bugs like this have been fixed in WebKit, though maybe the others were all a long time ago (e.g. <http://trac.webkit.org/changeset/24827>). initWithArray: will return an empty array when passed nil, but the documentation is not clear about this. -copy will call -initWithArray:, but I guess it's a little bit better because it's unambiguous about what you'll get with a nil initial array, so I'll switch to using that. Thanks for reviewing! 280524 4 sullivan 2010-09-16 21:07:40 -0700 Fixed in r67691. 67870 2010-09-16 18:14:12 -0700 2010-09-16 19:40:47 -0700 Patch to avoid mutating array while enumerating it. 45938_patch.txt text/plain 3631 sullivan SW5kZXg6IFdlYktpdC9tYWMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYktpdC9tYWMvQ2hh bmdlTG9nCShyZXZpc2lvbiA2NzY4MikKKysrIFdlYktpdC9tYWMvQ2hhbmdlTG9nCSh3b3JraW5n IGNvcHkpCkBAIC0xLDMgKzEsMjIgQEAKKzIwMTAtMDktMTYgIEpvaG4gU3VsbGl2YW4gIDxzdWxs aXZhbkBhcHBsZS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzgzOTU1NTg+CisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD00NTkzOAorICAgICAgICBfd2ViX21ha2VQbHVnaW5W aWV3c1BlcmZvcm1TZWxlY3Rvcjo6IG11dGF0ZXMgc3Vidmlld3MgYXJyYXkgd2hpbGUgaXRlcmF0 aW5nIGl0CisKKyAgICAgICAgKiBXZWJWaWV3L1dlYkhUTUxWaWV3Lm1tOgorICAgICAgICAoLVtX ZWJIVE1MVmlldyBfd2ViX21ha2VQbHVnaW5TdWJ2aWV3c1BlcmZvcm1TZWxlY3Rvcjp3aXRoT2Jq ZWN0Ol0pOgorICAgICAgICBDb252ZXJ0ZWQgZnJvbSAtW05TQXJyYXkgX3dlYl9tYWtlUGx1Z2lu Vmlld3NQZXJmb3JtU2VsZWN0b3I6d2l0aE9iamVjdDpdLiBUaGUgb2xkIG1ldGhvZAorICAgICAg ICB3YXMgb25seSBldmVyIGNhbGxlZCBvbiB0aGUgcmVzdWx0IG9mIC1bV2ViSFRNTFZpZXcgc3Vi dmlld3NdLiBCeSBtb3ZpbmcgdGhhdCBrbm93bGVkZ2UgaW50bworICAgICAgICB0aGlzIGhlbHBl ciBtZXRob2QsIGl0IGNhbid0IGJlIHVzZWQgaW5jb3JyZWN0bHkuIE5vdyBpdCBtYWtlcyBhIGNv cHkgb2YgW1dlYkhUTUxWaWV3IHN1YnZpZXdzXQorICAgICAgICBiZWZvcmUgZW51bWVyYXRpbmcg aXQsIHRvIGVuc3VyZSB0aGF0IHRoZSBzZWxlY3RvciBjYW4ndCBtdXRhdGUgdGhlIGFycmF5IGJl aW5nIGVudW1lcmF0ZWQuCisgICAgICAgICgtW1dlYkhUTUxWaWV3IHZpZXdXaWxsTW92ZVRvSG9z dFdpbmRvdzpdKToKKyAgICAgICAgVXBkYXRlZCBmb3IgX3dlYl9tYWtlUGx1Z2luU3Vidmlld3NQ ZXJmb3JtU2VsZWN0b3I6OiBzaWduYXR1cmUgY2hhbmdlLgorICAgICAgICAoLVtXZWJIVE1MVmll dyB2aWV3RGlkTW92ZVRvSG9zdFdpbmRvd10pOgorICAgICAgICBEaXR0by4KKwogMjAxMC0wOS0x NiAgRGFyaW4gQWRsZXIgIDxkYXJpbkBhcHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkg QW5kcmVhcyBLbGluZy4KSW5kZXg6IFdlYktpdC9tYWMvV2ViVmlldy9XZWJIVE1MVmlldy5tbQo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBXZWJLaXQvbWFjL1dlYlZpZXcvV2ViSFRNTFZpZXcubW0JKHJldmlzaW9u IDY3NjY4KQorKysgV2ViS2l0L21hYy9XZWJWaWV3L1dlYkhUTUxWaWV3Lm1tCSh3b3JraW5nIGNv cHkpCkBAIC0yMzc3LDEwICsyMzc3LDYgQEAgLSAodm9pZClzZXRBY2NlcHRzTW91c2VNb3ZlZEV2 ZW50czooQk9PTAogCiAjZW5kaWYKIAotQGludGVyZmFjZSBOU0FycmF5IChXZWJIVE1MVmlldykK LS0gKHZvaWQpX3dlYl9tYWtlUGx1Z2luVmlld3NQZXJmb3JtU2VsZWN0b3I6KFNFTClzZWxlY3Rv ciB3aXRoT2JqZWN0OihpZClvYmplY3Q7Ci1AZW5kCi0KIEBpbXBsZW1lbnRhdGlvbiBXZWJIVE1M VmlldwogCiArICh2b2lkKWluaXRpYWxpemUKQEAgLTMwODAsMTQgKzMwNzYsMzEgQEAgLSAodm9p ZCl2aWV3RGlkTW92ZVRvV2luZG93CiAgICAgfQogfQogCistICh2b2lkKV93ZWJfbWFrZVBsdWdp blN1YnZpZXdzUGVyZm9ybVNlbGVjdG9yOihTRUwpc2VsZWN0b3Igd2l0aE9iamVjdDooaWQpb2Jq ZWN0Cit7CisjaWYgRU5BQkxFKE5FVFNDQVBFX1BMVUdJTl9BUEkpCisgICAgLy8gQ29weSBzdWJ2 aWV3cyBiZWNhdXNlIFtzZWxmIHN1YnZpZXdzXSByZXR1cm5zIHRoZSB2aWV3J3MgbXV0YWJsZSBp bnRlcm5hbCBhcnJheSwKKyAgICAvLyBhbmQgd2UgbXVzdCBhdm9pZCBtdXRhdGluZyB0aGUgYXJy YXkgd2hpbGUgZW51bWVyYXRpbmcgaXQuCisgICAgTlNBcnJheSAqc3Vidmlld3MgPSBbW05TQXJy YXkgYWxsb2NdIGluaXRXaXRoQXJyYXk6W3NlbGYgc3Vidmlld3NdXTsKKyAgICAKKyAgICBOU0Vu dW1lcmF0b3IgKmVudW1lcmF0b3IgPSBbc3Vidmlld3Mgb2JqZWN0RW51bWVyYXRvcl07CisgICAg V2ViTmV0c2NhcGVQbHVnaW5WaWV3ICp2aWV3OworICAgIHdoaWxlICgodmlldyA9IFtlbnVtZXJh dG9yIG5leHRPYmplY3RdKSAhPSBuaWwpCisgICAgICAgIGlmIChbdmlldyBpc0tpbmRPZkNsYXNz OltXZWJCYXNlTmV0c2NhcGVQbHVnaW5WaWV3IGNsYXNzXV0pCisgICAgICAgICAgICBbdmlldyBw ZXJmb3JtU2VsZWN0b3I6c2VsZWN0b3Igd2l0aE9iamVjdDpvYmplY3RdOworICAgIAorICAgIFtz dWJ2aWV3cyByZWxlYXNlXTsKKyNlbmRpZgorfQorCiAtICh2b2lkKXZpZXdXaWxsTW92ZVRvSG9z dFdpbmRvdzooTlNXaW5kb3cgKilob3N0V2luZG93CiB7Ci0gICAgW1tzZWxmIHN1YnZpZXdzXSBf d2ViX21ha2VQbHVnaW5WaWV3c1BlcmZvcm1TZWxlY3RvcjpAc2VsZWN0b3Iodmlld1dpbGxNb3Zl VG9Ib3N0V2luZG93Oikgd2l0aE9iamVjdDpob3N0V2luZG93XTsKKyAgICBbc2VsZiBfd2ViX21h a2VQbHVnaW5TdWJ2aWV3c1BlcmZvcm1TZWxlY3RvcjpAc2VsZWN0b3Iodmlld1dpbGxNb3ZlVG9I b3N0V2luZG93Oikgd2l0aE9iamVjdDpob3N0V2luZG93XTsKIH0KIAogLSAodm9pZCl2aWV3RGlk TW92ZVRvSG9zdFdpbmRvdwogewotICAgIFtbc2VsZiBzdWJ2aWV3c10gX3dlYl9tYWtlUGx1Z2lu Vmlld3NQZXJmb3JtU2VsZWN0b3I6QHNlbGVjdG9yKHZpZXdEaWRNb3ZlVG9Ib3N0V2luZG93KSB3 aXRoT2JqZWN0Om5pbF07CisgICAgW3NlbGYgX3dlYl9tYWtlUGx1Z2luU3Vidmlld3NQZXJmb3Jt U2VsZWN0b3I6QHNlbGVjdG9yKHZpZXdEaWRNb3ZlVG9Ib3N0V2luZG93KSB3aXRoT2JqZWN0Om5p bF07CiB9CiAKIApAQCAtNTA4MiwyMSArNTA5NSw2IEBAIC0gKHZvaWQpb3RoZXJNb3VzZVVwOihO U0V2ZW50ICopZXZlbnQKIAogQGVuZAogCi1AaW1wbGVtZW50YXRpb24gTlNBcnJheSAoV2ViSFRN TFZpZXcpCi0KLS0gKHZvaWQpX3dlYl9tYWtlUGx1Z2luVmlld3NQZXJmb3JtU2VsZWN0b3I6KFNF TClzZWxlY3RvciB3aXRoT2JqZWN0OihpZClvYmplY3QKLXsKLSNpZiBFTkFCTEUoTkVUU0NBUEVf UExVR0lOX0FQSSkKLSAgICBOU0VudW1lcmF0b3IgKmVudW1lcmF0b3IgPSBbc2VsZiBvYmplY3RF bnVtZXJhdG9yXTsKLSAgICBXZWJOZXRzY2FwZVBsdWdpblZpZXcgKnZpZXc7Ci0gICAgd2hpbGUg KCh2aWV3ID0gW2VudW1lcmF0b3IgbmV4dE9iamVjdF0pICE9IG5pbCkKLSAgICAgICAgaWYgKFt2 aWV3IGlzS2luZE9mQ2xhc3M6W1dlYkJhc2VOZXRzY2FwZVBsdWdpblZpZXcgY2xhc3NdXSkKLSAg ICAgICAgICAgIFt2aWV3IHBlcmZvcm1TZWxlY3RvcjpzZWxlY3RvciB3aXRoT2JqZWN0Om9iamVj dF07Ci0jZW5kaWYKLX0KLQotQGVuZAotCiBAaW1wbGVtZW50YXRpb24gV2ViSFRNTFZpZXcgKFdl YkludGVybmFsKQogCiAtICh2b2lkKV9zZWxlY3Rpb25DaGFuZ2VkCg==