107975 2013-01-25 12:09:16 -0800 Don't use the threaded HTML parser for javascript: URLs 2013-01-28 11:37:17 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 106127 1 tonyg tonyg abarth ojan.autocc webkit.review.bot oldest_to_newest 816827 0 tonyg 2013-01-25 12:09:16 -0800 Don't use the threaded HTML parser for javascript: URLs 816830 1 184792 tonyg 2013-01-25 12:14:24 -0800 Created attachment 184792 Patch 816841 2 184792 eric 2013-01-25 12:25:23 -0800 Comment on attachment 184792 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=184792&action=review > Source/WebCore/ChangeLog:11 > + No new tests because covered by existing tests. Do you have an idea as to how many this causes to pass? > Source/WebCore/html/parser/HTMLDocumentParser.cpp:186 > +bool HTMLDocumentParser::shouldUseThreading() const > +{ > + return m_options.useThreading && !isParsingFragment() && !document()->url().isBlankURL(); > +} We had this exact logic in an earlier version, but removed it in favor of a solution which could handle empty documnts (in addition to about:blank, which just happens to be empty). I'm surprised javascript is counted as a blank url? 816844 3 tonyg 2013-01-25 12:31:13 -0800 (In reply to comment #2) > (From update of attachment 184792 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=184792&action=review > > > Source/WebCore/ChangeLog:11 > > + No new tests because covered by existing tests. > > Do you have an idea as to how many this causes to pass? No. I only ran the tests in loading/ and it caused us to pick up 3 there. Not sure how many other tests like that are out there. > > > Source/WebCore/html/parser/HTMLDocumentParser.cpp:186 > > +bool HTMLDocumentParser::shouldUseThreading() const > > +{ > > + return m_options.useThreading && !isParsingFragment() && !document()->url().isBlankURL(); > > +} > > We had this exact logic in an earlier version, but removed it in favor of a solution which could handle empty documnts (in addition to about:blank, which just happens to be empty). I'm surprised javascript is counted as a blank url? Yeah, JavaScript is counted as a blank URL. The thing that gets piped to append() is the result of the JavaScript execution, not the JavaScript itself and at that time that document has an empty URL. 817070 4 abarth 2013-01-25 17:18:23 -0800 > Yeah, JavaScript is counted as a blank URL. The thing that gets piped to append() is the result of the JavaScript execution, not the JavaScript itself and at that time that document has an empty URL. Maybe we should switch it to using insert instead? 817072 5 184792 abarth 2013-01-25 17:20:16 -0800 Comment on attachment 184792 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=184792&action=review > Source/WebCore/html/parser/HTMLDocumentParser.cpp:185 > + return m_options.useThreading && !isParsingFragment() && !document()->url().isBlankURL(); We should put this logic in HTMLParserOptions so that m_options reflects reality. IMHO, disabling threading for blank URLs makes sense. Given that we've wanted to do this twice, it's probably the right thing to do. 817877 6 185009 tonyg 2013-01-28 10:57:37 -0800 Created attachment 185009 Patch 817878 7 tonyg 2013-01-28 10:58:16 -0800 > > Source/WebCore/html/parser/HTMLDocumentParser.cpp:185 > > + return m_options.useThreading && !isParsingFragment() && !document()->url().isBlankURL(); > > We should put this logic in HTMLParserOptions so that m_options reflects reality. Done 817884 8 185009 abarth 2013-01-28 11:09:29 -0800 Comment on attachment 185009 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185009&action=review > Source/WebCore/html/parser/HTMLParserOptions.cpp:44 > - useThreading = settings && settings->threadedHTMLParser(); > + useThreading = settings && settings->threadedHTMLParser() && (!document || !document->url().isBlankURL()); settings being non-zero implies that document is non-zero, so you can remove the check for !document. 817891 9 185016 tonyg 2013-01-28 11:13:56 -0800 Created attachment 185016 Patch for landing 817916 10 185016 webkit.review.bot 2013-01-28 11:37:13 -0800 Comment on attachment 185016 Patch for landing Clearing flags on attachment: 185016 Committed r140986: <http://trac.webkit.org/changeset/140986> 817918 11 webkit.review.bot 2013-01-28 11:37:17 -0800 All reviewed patches have been landed. Closing bug. 184792 2013-01-25 12:14:24 -0800 2013-01-28 10:57:35 -0800 Patch bug-107975-20130125121115.patch text/plain 2448 tonyg U3VidmVyc2lvbiBSZXZpc2lvbjogMTQwODQ1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMjIyY2YyNzg5YjJmZTM2 NjBmNjVhMzY5NTQ1Y2ZjM2FmZjM0ZjQ3OC4uZDI2ZWM2Njc5YjFhMDM0ZGM2ZGYwZTYxMGUwYTdl OGQ2NDdmMzkzNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDEzLTAxLTI1ICBUb255 IEdlbnRpbGNvcmUgIDx0b255Z0BjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgRG9uJ3QgdXNlIHRo ZSB0aHJlYWRlZCBIVE1MIHBhcnNlciBmb3IgamF2YXNjcmlwdDogVVJMcworICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTA3OTc1CisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgU2V2ZXJhbCBsYXlvdXQgdGVzdHMg ZGVwZW5kIG9uIGphdmFzY3JpcHQ6IFVSTCBpZnJhbWVzIGxvYWRpbmcgc3luY2hyb25vdXNseSBp bmNsdWRpbmcgZmFzdC9sb2FkZXIvamF2YXNjcmlwdC11cmwtZW5jb2RpbmcuaHRtbC4KKyAgICAg ICAgVGhpcyBwYXRjaCBhdm9pZHMgdXNpbmcgdGhlIHRocmVhZGVkIHBhcnNlciBmb3IgdGhvc2Ug VVJMcyBzbyB0aGV5IHdpbGwgY29udGludWUgdG8gYmUgc3luY2hyb25vdXMuCisKKyAgICAgICAg Tm8gbmV3IHRlc3RzIGJlY2F1c2UgY292ZXJlZCBieSBleGlzdGluZyB0ZXN0cy4KKworICAgICAg ICAqIGh0bWwvcGFyc2VyL0hUTUxEb2N1bWVudFBhcnNlci5jcHA6CisgICAgICAgIChXZWJDb3Jl OjpIVE1MRG9jdW1lbnRQYXJzZXI6OnNob3VsZFVzZVRocmVhZGluZyk6CisgICAgICAgIChXZWJD b3JlKToKKyAgICAgICAgKiBodG1sL3BhcnNlci9IVE1MRG9jdW1lbnRQYXJzZXIuaDoKKwogMjAx My0wMS0yNSAgQ2hhbmcgU2h1ICA8Y3NodUB3ZWJraXQub3JnPgogCiAgICAgICAgIEJ1aWxkIGJy b2tlbiB3aGVuIHN2ZyBpcyBkaXNhYmxlZC4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2h0 bWwvcGFyc2VyL0hUTUxEb2N1bWVudFBhcnNlci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL3Bh cnNlci9IVE1MRG9jdW1lbnRQYXJzZXIuY3BwCmluZGV4IDI3OGFhZGE2MjI3OWQzYjhjMDEyMTFh ZWZjYWIyMGM5MjAwM2I4NjcuLjVlNDgyOTAzZDg4MGUxODljM2YxYmEzZGJmZjYwYzAyNDc4NGIw OWEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2h0bWwvcGFyc2VyL0hUTUxEb2N1bWVudFBh cnNlci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvaHRtbC9wYXJzZXIvSFRNTERvY3VtZW50UGFy c2VyLmNwcApAQCAtMTgwLDYgKzE4MCwxMSBAQCB2b2lkIEhUTUxEb2N1bWVudFBhcnNlcjo6cHJl cGFyZVRvU3RvcFBhcnNpbmcoKQogICAgIGF0dGVtcHRUb1J1bkRlZmVycmVkU2NyaXB0c0FuZEVu ZCgpOwogfQogCitib29sIEhUTUxEb2N1bWVudFBhcnNlcjo6c2hvdWxkVXNlVGhyZWFkaW5nKCkg Y29uc3QKK3sKKyAgICByZXR1cm4gbV9vcHRpb25zLnVzZVRocmVhZGluZyAmJiAhaXNQYXJzaW5n RnJhZ21lbnQoKSAmJiAhZG9jdW1lbnQoKS0+dXJsKCkuaXNCbGFua1VSTCgpOworfQorCiBib29s IEhUTUxEb2N1bWVudFBhcnNlcjo6aXNQYXJzaW5nRnJhZ21lbnQoKSBjb25zdAogewogICAgIHJl dHVybiBtX3RyZWVCdWlsZGVyLT5pc1BhcnNpbmdGcmFnbWVudCgpOwpkaWZmIC0tZ2l0IGEvU291 cmNlL1dlYkNvcmUvaHRtbC9wYXJzZXIvSFRNTERvY3VtZW50UGFyc2VyLmggYi9Tb3VyY2UvV2Vi Q29yZS9odG1sL3BhcnNlci9IVE1MRG9jdW1lbnRQYXJzZXIuaAppbmRleCBmMTljYjEyMjk0NWVm NTk2MWYxN2Y3MGI5NWRlZDMzNDg0YjViYzFmLi45NGZmY2M1YTc5ZWE2OTdiOTMxNDJkZDk2ZDA1 ODQzOTA5YjY3MTJkIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9odG1sL3BhcnNlci9IVE1M RG9jdW1lbnRQYXJzZXIuaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL3BhcnNlci9IVE1MRG9j dW1lbnRQYXJzZXIuaApAQCAtMTQ3LDggKzE0Nyw3IEBAIHByaXZhdGU6CiAgICAgdm9pZCBhdHRl bXB0VG9SdW5EZWZlcnJlZFNjcmlwdHNBbmRFbmQoKTsKICAgICB2b2lkIGVuZCgpOwogCi0gICAg Ym9vbCBzaG91bGRVc2VUaHJlYWRpbmcoKSBjb25zdCB7IHJldHVybiBtX29wdGlvbnMudXNlVGhy ZWFkaW5nICYmICFpc1BhcnNpbmdGcmFnbWVudCgpOyB9Ci0KKyAgICBib29sIHNob3VsZFVzZVRo cmVhZGluZygpIGNvbnN0OwogICAgIGJvb2wgaXNQYXJzaW5nRnJhZ21lbnQoKSBjb25zdDsKICAg ICBib29sIGlzU2NoZWR1bGVkRm9yUmVzdW1lKCkgY29uc3Q7CiAgICAgYm9vbCBpblB1bXBTZXNz aW9uKCkgY29uc3QgeyByZXR1cm4gbV9wdW1wU2Vzc2lvbk5lc3RpbmdMZXZlbCA+IDA7IH0K 185009 2013-01-28 10:57:37 -0800 2013-01-28 11:13:54 -0800 Patch bug-107975-20130128105424.patch text/plain 1830 tonyg U3VidmVyc2lvbiBSZXZpc2lvbjogMTQwOTc4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjE3YTM0ZjZlODhkMmNj OWM5MTY5MjgzZDU4NDM5OTQxOWViZDhhOC4uMDllMzNjYWQ0MDZmNmIzM2FkNjlhYWE3ZDUxOTFm OWFhZDkxYTgzZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEzLTAxLTI4ICBUb255 IEdlbnRpbGNvcmUgIDx0b255Z0BjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgRG9uJ3QgdXNlIHRo ZSB0aHJlYWRlZCBIVE1MIHBhcnNlciBmb3IgamF2YXNjcmlwdDogVVJMcworICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTA3OTc1CisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgU2V2ZXJhbCBsYXlvdXQgdGVzdHMg ZGVwZW5kIG9uIGphdmFzY3JpcHQ6IFVSTCBpZnJhbWVzIGxvYWRpbmcgc3luY2hyb25vdXNseSBp bmNsdWRpbmcgZmFzdC9sb2FkZXIvamF2YXNjcmlwdC11cmwtZW5jb2RpbmcuaHRtbC4KKyAgICAg ICAgVGhpcyBwYXRjaCBhdm9pZHMgdXNpbmcgdGhlIHRocmVhZGVkIHBhcnNlciBmb3IgdGhvc2Ug VVJMcyBzbyB0aGV5IHdpbGwgY29udGludWUgdG8gYmUgc3luY2hyb25vdXMuCisKKyAgICAgICAg Tm8gbmV3IHRlc3RzIGJlY2F1c2UgY292ZXJlZCBieSBleGlzdGluZyB0ZXN0cy4KKworICAgICAg ICAqIGh0bWwvcGFyc2VyL0hUTUxQYXJzZXJPcHRpb25zLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OkhUTUxQYXJzZXJPcHRpb25zOjpIVE1MUGFyc2VyT3B0aW9ucyk6CisKIDIwMTMtMDEtMjggIEJl YXIgVHJhdmlzICA8YmV0cmF2aXNAYWRvYmUuY29tPgogCiAgICAgICAgIFtDU1MgRXhjbHVzaW9u c10gUmVmYWN0b3IgRXhjbHVzaW9uU2hhcGVJbnNpZGVJbmZvIHRvIG1vcmUgZ2VuZXJhbCBFeGNs dXNpb25TaGFwZUluZm8KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2h0bWwvcGFyc2VyL0hU TUxQYXJzZXJPcHRpb25zLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvcGFyc2VyL0hUTUxQYXJz ZXJPcHRpb25zLmNwcAppbmRleCAyNTUyNzU4ZTQ5ZWMwMjM2ZTU3NzI5Y2EwZTE3NjIyZDQzM2Ni YTdhLi5lODA0MjNlNTMyNjg5ZDgxMjBlOTIzOTE4MDEwMjQ0MzA2ODhkYWU2IDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViQ29yZS9odG1sL3BhcnNlci9IVE1MUGFyc2VyT3B0aW9ucy5jcHAKKysrIGIv U291cmNlL1dlYkNvcmUvaHRtbC9wYXJzZXIvSFRNTFBhcnNlck9wdGlvbnMuY3BwCkBAIC00MSw3 ICs0MSw3IEBAIEhUTUxQYXJzZXJPcHRpb25zOjpIVE1MUGFyc2VyT3B0aW9ucyhEb2N1bWVudCog ZG9jdW1lbnQpCiAgICAgU2V0dGluZ3MqIHNldHRpbmdzID0gZG9jdW1lbnQgPyBkb2N1bWVudC0+ c2V0dGluZ3MoKSA6IDA7CiAgICAgdXNlUHJlSFRNTDVQYXJzZXJRdWlya3MgPSBzZXR0aW5ncyAm JiBzZXR0aW5ncy0+dXNlUHJlSFRNTDVQYXJzZXJRdWlya3MoKTsKICNpZiBFTkFCTEUoVEhSRUFE RURfSFRNTF9QQVJTRVIpCi0gICAgdXNlVGhyZWFkaW5nID0gc2V0dGluZ3MgJiYgc2V0dGluZ3Mt PnRocmVhZGVkSFRNTFBhcnNlcigpOworICAgIHVzZVRocmVhZGluZyA9IHNldHRpbmdzICYmIHNl dHRpbmdzLT50aHJlYWRlZEhUTUxQYXJzZXIoKSAmJiAoIWRvY3VtZW50IHx8ICFkb2N1bWVudC0+ dXJsKCkuaXNCbGFua1VSTCgpKTsKICNlbHNlCiAgICAgdXNlVGhyZWFkaW5nID0gZmFsc2U7CiAj ZW5kaWYK 185016 2013-01-28 11:13:56 -0800 2013-01-28 11:37:13 -0800 Patch for landing bug-107975-20130128111043.patch text/plain 1811 tonyg U3VidmVyc2lvbiBSZXZpc2lvbjogMTQwOTc4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjE3YTM0ZjZlODhkMmNj OWM5MTY5MjgzZDU4NDM5OTQxOWViZDhhOC4uYWNiZDc5MjdkZjI2OGRiY2VkOTg1ODlmYjllOWZh M2NjZGQ0YjU4YiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEzLTAxLTI4ICBUb255 IEdlbnRpbGNvcmUgIDx0b255Z0BjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgRG9uJ3QgdXNlIHRo ZSB0aHJlYWRlZCBIVE1MIHBhcnNlciBmb3IgamF2YXNjcmlwdDogVVJMcworICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTA3OTc1CisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgQWRhbSBCYXJ0aC4KKworICAgICAgICBTZXZlcmFsIGxheW91dCB0ZXN0cyBkZXBl bmQgb24gamF2YXNjcmlwdDogVVJMIGlmcmFtZXMgbG9hZGluZyBzeW5jaHJvbm91c2x5IGluY2x1 ZGluZyBmYXN0L2xvYWRlci9qYXZhc2NyaXB0LXVybC1lbmNvZGluZy5odG1sLgorICAgICAgICBU aGlzIHBhdGNoIGF2b2lkcyB1c2luZyB0aGUgdGhyZWFkZWQgcGFyc2VyIGZvciB0aG9zZSBVUkxz IHNvIHRoZXkgd2lsbCBjb250aW51ZSB0byBiZSBzeW5jaHJvbm91cy4KKworICAgICAgICBObyBu ZXcgdGVzdHMgYmVjYXVzZSBjb3ZlcmVkIGJ5IGV4aXN0aW5nIHRlc3RzLgorCisgICAgICAgICog aHRtbC9wYXJzZXIvSFRNTFBhcnNlck9wdGlvbnMuY3BwOgorICAgICAgICAoV2ViQ29yZTo6SFRN TFBhcnNlck9wdGlvbnM6OkhUTUxQYXJzZXJPcHRpb25zKToKKwogMjAxMy0wMS0yOCAgQmVhciBU cmF2aXMgIDxiZXRyYXZpc0BhZG9iZS5jb20+CiAKICAgICAgICAgW0NTUyBFeGNsdXNpb25zXSBS ZWZhY3RvciBFeGNsdXNpb25TaGFwZUluc2lkZUluZm8gdG8gbW9yZSBnZW5lcmFsIEV4Y2x1c2lv blNoYXBlSW5mbwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvaHRtbC9wYXJzZXIvSFRNTFBh cnNlck9wdGlvbnMuY3BwIGIvU291cmNlL1dlYkNvcmUvaHRtbC9wYXJzZXIvSFRNTFBhcnNlck9w dGlvbnMuY3BwCmluZGV4IDI1NTI3NThlNDllYzAyMzZlNTc3MjljYTBlMTc2MjJkNDMzY2JhN2Eu LjMyYTQ2NGEwOGMyZTNmYmQ2OTIyNTc0NDhmOTYyZjk1ZmYxN2I4YTEgMTAwNjQ0Ci0tLSBhL1Nv dXJjZS9XZWJDb3JlL2h0bWwvcGFyc2VyL0hUTUxQYXJzZXJPcHRpb25zLmNwcAorKysgYi9Tb3Vy Y2UvV2ViQ29yZS9odG1sL3BhcnNlci9IVE1MUGFyc2VyT3B0aW9ucy5jcHAKQEAgLTQxLDcgKzQx LDcgQEAgSFRNTFBhcnNlck9wdGlvbnM6OkhUTUxQYXJzZXJPcHRpb25zKERvY3VtZW50KiBkb2N1 bWVudCkKICAgICBTZXR0aW5ncyogc2V0dGluZ3MgPSBkb2N1bWVudCA/IGRvY3VtZW50LT5zZXR0 aW5ncygpIDogMDsKICAgICB1c2VQcmVIVE1MNVBhcnNlclF1aXJrcyA9IHNldHRpbmdzICYmIHNl dHRpbmdzLT51c2VQcmVIVE1MNVBhcnNlclF1aXJrcygpOwogI2lmIEVOQUJMRShUSFJFQURFRF9I VE1MX1BBUlNFUikKLSAgICB1c2VUaHJlYWRpbmcgPSBzZXR0aW5ncyAmJiBzZXR0aW5ncy0+dGhy ZWFkZWRIVE1MUGFyc2VyKCk7CisgICAgdXNlVGhyZWFkaW5nID0gc2V0dGluZ3MgJiYgc2V0dGlu Z3MtPnRocmVhZGVkSFRNTFBhcnNlcigpICYmICFkb2N1bWVudC0+dXJsKCkuaXNCbGFua1VSTCgp OwogI2Vsc2UKICAgICB1c2VUaHJlYWRpbmcgPSBmYWxzZTsKICNlbmRpZgo=