30747 2009-10-24 11:57:33 -0700 [v8] Do not retrieve proxy when fetching context 2009-10-25 21:33:13 -0700 1 1 1 Unclassified WebKit WebKit Misc. 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 0 antonm webkit-unassigned abarth commit-queue dglazkov oldest_to_newest 157461 0 antonm 2009-10-24 11:57:33 -0700 On some paths caller already has a proxy, so there is no need to retrieve it another time when fetching context. 157464 1 41787 antonm 2009-10-24 12:00:34 -0700 Created attachment 41787 First take Adam, it looks safe, but I'd appreciate if you had another look at it. 157476 2 41787 abarth 2009-10-24 13:19:07 -0700 Comment on attachment 41787 First take It doesn't make sense to have a static method that takes effectively a |this| pointer as an argument. Can you re-phrase this code using instance methods? 157486 3 dglazkov 2009-10-24 13:39:43 -0700 +1. Also, while working on event listeners recently, I was baffled and not amused at how loose and weird our ScriptExecutionContext <-> ScriptController <--> V8Proxy <--> v8::Context relationship graph looks. We should really try to simplify this. I know peeps want performance and efficiency, but I bet there's lots of skeletons buried along this path. (I am stopping short, to avoid becoming unpositive over existence of WorkerProxyExecutionContext and how it's a "look at me, I am a V8Proxy", except its not, ... alrighty then.). 157489 4 abarth 2009-10-24 13:49:48 -0700 The correct relationship is as follows: ScriptExecutionContext == Document == DOMWindow V8Proxy == ScriptController == Frame V8Proxy and ScriptController are actually the same thing, just split apart for no reason. v8::Context == GlobalObject, which has a many-to-one relationship with DOMWindows because of IsolatedWorlds. Not sure if that was helpful. 157497 5 antonm 2009-10-24 15:08:23 -0700 (In reply to comment #2) > (From update of attachment 41787 [details]) > It doesn't make sense to have a static method that takes effectively a |this| > pointer as an argument. Can you re-phrase this code using instance methods? Very good point, thank you. Even more---following you advice I found there is already the method I need. Doing a new build (which takes a lot of time on my Mac) and sending a new patch. 157498 6 antonm 2009-10-24 15:14:51 -0700 (In reply to comment #4) > The correct relationship is as follows: > > ScriptExecutionContext == Document == DOMWindow > V8Proxy == ScriptController == Frame > > V8Proxy and ScriptController are actually the same thing, just split apart for > no reason. v8::Context == GlobalObject, which has a many-to-one relationship > with DOMWindows because of IsolatedWorlds. > > Not sure if that was helpful. That was, at least for me. Adam, Dmitry, maybe we should refactor it? For example for me as a new comer, I see that there is a proxy and frame and they look exactly the way Adam describes, but are separate. Now I am not sure if they are always the same or there are some edge cases/cases I amnot aware of when they are not. 157502 7 abarth 2009-10-24 15:31:50 -0700 > That was, at least for me. Adam, Dmitry, maybe we should refactor it? I support combining ScriptController and V8Proxy. If there's a clean way to do this, please go for it (in a separate patch). 157624 8 41830 antonm 2009-10-25 15:10:51 -0700 Created attachment 41830 2nd try 157669 9 41830 abarth 2009-10-25 21:23:28 -0700 Comment on attachment 41830 2nd try A patch of beauty! 157671 10 41830 commit-queue 2009-10-25 21:33:09 -0700 Comment on attachment 41830 2nd try Clearing flags on attachment: 41830 Committed r50049: <http://trac.webkit.org/changeset/50049> 157672 11 commit-queue 2009-10-25 21:33:13 -0700 All reviewed patches have been landed. Closing bug. 41787 2009-10-24 12:00:34 -0700 2009-10-25 15:10:51 -0700 First take WebCore.patch text/plain 4674 antonm SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1MDAyNykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMjAgQEAKKzIwMDktMTAtMjQgIEFudG9uIE11aGluICA8YW50b25tQGNocm9taXVt Lm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBQ YXNzIGEgcHJveHkgaW50byBtZXRob2QgdG8gcmV0cmlldmUgYSBjb250ZXh0IGZvciB0aGUgZnJh bWUuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zMDc0 NworCisgICAgICAgIE5vIG5ldyB0ZXN0cy4gKE9PUFMhKQorCisgICAgICAgICogYmluZGluZ3Mv djgvVjhET01XcmFwcGVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlY4RE9NV3JhcHBlcjo6Y29u dmVydE5ld05vZGVUb1Y4T2JqZWN0KToKKyAgICAgICAgKiBiaW5kaW5ncy92OC9WOFByb3h5LmNw cDoKKyAgICAgICAgKFdlYkNvcmU6OlY4UHJveHk6OmZpbHRlclRocm91Z2hJc29sYXRlZFdvcmxk cyk6CisgICAgICAgICogYmluZGluZ3MvdjgvVjhQcm94eS5oOgorICAgICAgICAoV2ViQ29yZTo6 VjhQcm94eTo6Y29udGV4dCk6CisgICAgICAgIChXZWJDb3JlOjpWOFByb3h5OjptYWluV29ybGRD b250ZXh0KToKKwogMjAwOS0xMC0yNCAgRGltaXRyaSBHbGF6a292ICA8ZGdsYXprb3ZAY2hyb21p dW0ub3JnPgogCiAgICAgICAgIFVucmV2aWV3ZWQsIGJ1aWxkIGZpeC4KSW5kZXg6IFdlYkNvcmUv YmluZGluZ3MvdjgvVjhET01XcmFwcGVyLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL2JpbmRp bmdzL3Y4L1Y4RE9NV3JhcHBlci5jcHAJKHJldmlzaW9uIDQ5OTYxKQorKysgV2ViQ29yZS9iaW5k aW5ncy92OC9WOERPTVdyYXBwZXIuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xMjc0LDcgKzEyNzQs NyBAQAogCiAgICAgdjg6OkhhbmRsZTx2ODo6Q29udGV4dD4gY29udGV4dDsKICAgICBpZiAocHJv eHkpCi0gICAgICAgIGNvbnRleHQgPSBWOFByb3h5Ojpjb250ZXh0KHByb3h5LT5mcmFtZSgpKTsK KyAgICAgICAgY29udGV4dCA9IFY4UHJveHk6OmNvbnRleHQocHJveHktPmZyYW1lKCksIHByb3h5 KTsKIAogICAgIC8vIEVudGVyIHRoZSBub2RlJ3MgY29udGV4dCBhbmQgY3JlYXRlIHRoZSB3cmFw cGVyIGluIHRoYXQgY29udGV4dC4KICAgICBpZiAoIWNvbnRleHQuSXNFbXB0eSgpKQpJbmRleDog V2ViQ29yZS9iaW5kaW5ncy92OC9WOFByb3h5LmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9iaW5k aW5ncy92OC9WOFByb3h5LmgJKHJldmlzaW9uIDQ5OTYxKQorKysgV2ViQ29yZS9iaW5kaW5ncy92 OC9WOFByb3h5LmgJKHdvcmtpbmcgY29weSkKQEAgLTI1Niw4ICsyNTYsMzQgQEAKIAogICAgICAg ICAvLyBSZXR1cm5zIFY4IENvbnRleHQgb2YgYSBmcmFtZS4gSWYgbm9uZSBleGlzdHMsIGNyZWF0 ZXMKICAgICAgICAgLy8gYSBuZXcgY29udGV4dC4gSXQgaXMgcG90ZW50aWFsbHkgc2xvdyBhbmQg Y29uc3VtZXMgbWVtb3J5LgotICAgICAgICBzdGF0aWMgdjg6OkxvY2FsPHY4OjpDb250ZXh0PiBj b250ZXh0KEZyYW1lKik7Ci0gICAgICAgIHN0YXRpYyB2ODo6TG9jYWw8djg6OkNvbnRleHQ+IG1h aW5Xb3JsZENvbnRleHQoRnJhbWUqKTsKKyAgICAgICAgc3RhdGljIHY4OjpMb2NhbDx2ODo6Q29u dGV4dD4gY29udGV4dChGcmFtZSogZnJhbWUpCisgICAgICAgIHsKKyAgICAgICAgICAgIHY4OjpM b2NhbDx2ODo6Q29udGV4dD4gY29udGV4dCA9IFY4UHJveHk6Om1haW5Xb3JsZENvbnRleHQoZnJh bWUpOworICAgICAgICAgICAgaWYgKGNvbnRleHQuSXNFbXB0eSgpKQorICAgICAgICAgICAgICAg IHJldHVybiB2ODo6TG9jYWw8djg6OkNvbnRleHQ+KCk7CisgICAgICAgICAgICByZXR1cm4gZmls dGVyVGhyb3VnaElzb2xhdGVkV29ybGRzKGZyYW1lLCBjb250ZXh0KTsKKyAgICAgICAgfQorICAg ICAgICBzdGF0aWMgdjg6OkxvY2FsPHY4OjpDb250ZXh0PiBjb250ZXh0KEZyYW1lKiBmcmFtZSwg VjhQcm94eSogcHJveHkpCisgICAgICAgIHsKKyAgICAgICAgICAgIHY4OjpMb2NhbDx2ODo6Q29u dGV4dD4gY29udGV4dCA9IFY4UHJveHk6Om1haW5Xb3JsZENvbnRleHQocHJveHkpOworICAgICAg ICAgICAgaWYgKGNvbnRleHQuSXNFbXB0eSgpKQorICAgICAgICAgICAgICAgIHJldHVybiB2ODo6 TG9jYWw8djg6OkNvbnRleHQ+KCk7CisgICAgICAgICAgICByZXR1cm4gZmlsdGVyVGhyb3VnaElz b2xhdGVkV29ybGRzKGZyYW1lLCBjb250ZXh0KTsKKyAgICAgICAgfQorICAgICAgICBzdGF0aWMg djg6OkxvY2FsPHY4OjpDb250ZXh0PiBtYWluV29ybGRDb250ZXh0KEZyYW1lKiBmcmFtZSkKKyAg ICAgICAgeworICAgICAgICAgICAgVjhQcm94eSogcHJveHkgPSByZXRyaWV2ZShmcmFtZSk7Cisg ICAgICAgICAgICBpZiAoIXByb3h5KQorICAgICAgICAgICAgICAgIHJldHVybiB2ODo6TG9jYWw8 djg6OkNvbnRleHQ+KCk7CisKKyAgICAgICAgICAgIHJldHVybiBtYWluV29ybGRDb250ZXh0KHBy b3h5KTsKKyAgICAgICAgfQorICAgICAgICBzdGF0aWMgdjg6OkxvY2FsPHY4OjpDb250ZXh0PiBt YWluV29ybGRDb250ZXh0KFY4UHJveHkqIHByb3h5KQorICAgICAgICB7CisgICAgICAgICAgICBB U1NFUlQocHJveHkpOworICAgICAgICAgICAgcHJveHktPmluaXRDb250ZXh0SWZOZWVkZWQoKTsK KyAgICAgICAgICAgIHJldHVybiB2ODo6TG9jYWw8djg6OkNvbnRleHQ+OjpOZXcocHJveHktPm1f Y29udGV4dCk7CisgICAgICAgIH0KICAgICAgICAgc3RhdGljIHY4OjpMb2NhbDx2ODo6Q29udGV4 dD4gY3VycmVudENvbnRleHQoKTsKIAogICAgICAgICAvLyBJZiB0aGUgY3VycmVudCBjb250ZXh0 IGNhdXNlcyBvdXQgb2YgbWVtb3J5LCBKYXZhU2NyaXB0IHNldHRpbmcKQEAgLTM4MCw2ICs0MDYs OCBAQAogICAgICAgICAgICAgcmV0dXJuIHY4OjpMb2NhbDx2ODo6Q29udGV4dD46Ok5ldyhtX3V0 aWxpdHlDb250ZXh0KTsKICAgICAgICAgfQogCisgICAgICAgIHN0YXRpYyB2ODo6TG9jYWw8djg6 OkNvbnRleHQ+IGZpbHRlclRocm91Z2hJc29sYXRlZFdvcmxkcyhGcmFtZSogZnJhbWUsIHY4OjpM b2NhbDx2ODo6Q29udGV4dD4gY29udGV4dCk7CisKICAgICAgICAgdjg6OkxvY2FsPHY4OjpPYmpl Y3Q+IGNyZWF0ZVdyYXBwZXJGcm9tQ2FjaGVTbG93Q2FzZShWOENsYXNzSW5kZXg6OlY4V3JhcHBl clR5cGUpOwogCiAgICAgICAgIHN0YXRpYyB2b2lkIHJlZ2lzdGVyRXh0ZW5zaW9uV2l0aFY4KHY4 OjpFeHRlbnNpb24qKTsKSW5kZXg6IFdlYkNvcmUvYmluZGluZ3MvdjgvVjhQcm94eS5jcHAKPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gV2ViQ29yZS9iaW5kaW5ncy92OC9WOFByb3h5LmNwcAkocmV2aXNpb24gNDk5 NjEpCisrKyBXZWJDb3JlL2JpbmRpbmdzL3Y4L1Y4UHJveHkuY3BwCSh3b3JraW5nIGNvcHkpCkBA IC0xMTcxLDEyICsxMTcxLDggQEAKICAgICB9CiB9CiAKLXY4OjpMb2NhbDx2ODo6Q29udGV4dD4g VjhQcm94eTo6Y29udGV4dChGcmFtZSogZnJhbWUpCit2ODo6TG9jYWw8djg6OkNvbnRleHQ+IFY4 UHJveHk6OmZpbHRlclRocm91Z2hJc29sYXRlZFdvcmxkcyhGcmFtZSogZnJhbWUsIHY4OjpMb2Nh bDx2ODo6Q29udGV4dD4gY29udGV4dCkKIHsKLSAgICB2ODo6TG9jYWw8djg6OkNvbnRleHQ+IGNv bnRleHQgPSBWOFByb3h5OjptYWluV29ybGRDb250ZXh0KGZyYW1lKTsKLSAgICBpZiAoY29udGV4 dC5Jc0VtcHR5KCkpCi0gICAgICAgIHJldHVybiB2ODo6TG9jYWw8djg6OkNvbnRleHQ+KCk7Ci0K ICAgICBpZiAoVjhJc29sYXRlZFdvcmxkKiB3b3JsZCA9IFY4SXNvbGF0ZWRXb3JsZDo6Z2V0RW50 ZXJlZCgpKSB7CiAgICAgICAgIGNvbnRleHQgPSB2ODo6TG9jYWw8djg6OkNvbnRleHQ+OjpOZXco d29ybGQtPmNvbnRleHQoKSk7CiAgICAgICAgIGlmIChmcmFtZSAhPSBWOFByb3h5OjpyZXRyaWV2 ZUZyYW1lKGNvbnRleHQpKQpAQCAtMTE5OCwxNiArMTE5NCw2IEBACiAgICAgcmV0dXJuIHY4OjpM b2NhbDx2ODo6Q29udGV4dD46Ok5ldyhtX2NvbnRleHQpOzsKIH0KIAotdjg6OkxvY2FsPHY4OjpD b250ZXh0PiBWOFByb3h5OjptYWluV29ybGRDb250ZXh0KEZyYW1lKiBmcmFtZSkKLXsKLSAgICBW OFByb3h5KiBwcm94eSA9IHJldHJpZXZlKGZyYW1lKTsKLSAgICBpZiAoIXByb3h5KQotICAgICAg ICByZXR1cm4gdjg6OkxvY2FsPHY4OjpDb250ZXh0PigpOwotCi0gICAgcHJveHktPmluaXRDb250 ZXh0SWZOZWVkZWQoKTsKLSAgICByZXR1cm4gdjg6OkxvY2FsPHY4OjpDb250ZXh0Pjo6TmV3KHBy b3h5LT5tX2NvbnRleHQpOwotfQotCiB2ODo6TG9jYWw8djg6OkNvbnRleHQ+IFY4UHJveHk6OmN1 cnJlbnRDb250ZXh0KCkKIHsKICAgICByZXR1cm4gdjg6OkNvbnRleHQ6OkdldEN1cnJlbnQoKTsK 41830 2009-10-25 15:10:51 -0700 2009-10-25 21:33:09 -0700 2nd try WebCore.patch text/plain 1059 antonm SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1MDAyNykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTMgQEAKKzIwMDktMTAtMjQgIEFudG9uIE11aGluICA8YW50b25tQGNocm9taXVt Lm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBS ZXVzZSBhbHJlYWR5IGZldGNoZWQgcHJveHkuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0zMDc0NworCisgICAgICAgICogYmluZGluZ3MvdjgvVjhET01X cmFwcGVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlY4RE9NV3JhcHBlcjo6Y29udmVydE5ld05v ZGVUb1Y4T2JqZWN0KToKKwogMjAwOS0xMC0yNCAgRGltaXRyaSBHbGF6a292ICA8ZGdsYXprb3ZA Y2hyb21pdW0ub3JnPgogCiAgICAgICAgIFVucmV2aWV3ZWQsIGJ1aWxkIGZpeC4KSW5kZXg6IFdl YkNvcmUvYmluZGluZ3MvdjgvVjhET01XcmFwcGVyLmNwcAo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3Jl L2JpbmRpbmdzL3Y4L1Y4RE9NV3JhcHBlci5jcHAJKHJldmlzaW9uIDQ5OTYxKQorKysgV2ViQ29y ZS9iaW5kaW5ncy92OC9WOERPTVdyYXBwZXIuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xMjc0LDcg KzEyNzQsNyBAQAogCiAgICAgdjg6OkhhbmRsZTx2ODo6Q29udGV4dD4gY29udGV4dDsKICAgICBp ZiAocHJveHkpCi0gICAgICAgIGNvbnRleHQgPSBWOFByb3h5Ojpjb250ZXh0KHByb3h5LT5mcmFt ZSgpKTsKKyAgICAgICAgY29udGV4dCA9IHByb3h5LT5jb250ZXh0KCk7CiAKICAgICAvLyBFbnRl ciB0aGUgbm9kZSdzIGNvbnRleHQgYW5kIGNyZWF0ZSB0aGUgd3JhcHBlciBpbiB0aGF0IGNvbnRl eHQuCiAgICAgaWYgKCFjb250ZXh0LklzRW1wdHkoKSkK