40177 2010-06-04 11:38:32 -0700 CachedResourceHandle isn't held long enough to guaranteed CachedScript ownership 2010-06-16 22:23:15 -0700 1 1 1 Unclassified WebKit Page Loading 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 tonyg tonyg abarth ap commit-queue darin eric oldest_to_newest 234309 0 tonyg 2010-06-04 11:38:32 -0700 CachedResourceHandle isn't held long enough to guaranteed CachedScript ownership 234389 1 57918 tonyg 2010-06-04 14:43:28 -0700 Created attachment 57918 Patch 234391 2 tonyg 2010-06-04 14:44:54 -0700 abarth - you brought up this issue in https://bugs.webkit.org/show_bug.cgi?id=40089 234397 3 tonyg 2010-06-04 14:48:00 -0700 By the way, I had a related question. It looks to me like CachedResourceHandle could just go away in favor of RefPtr. Am I missing something? If not, I could look into cleaning that up. 234438 4 darin 2010-06-04 17:39:42 -0700 (In reply to comment #3) > It looks to me like CachedResourceHandle could just go away in favor of RefPtr. Am I missing something? If not, I could look into cleaning that up. You are missing something. Look at m_handlesToRevalidate in CachedResource to see what this does beyond what RefPtr does. 234444 5 tonyg 2010-06-04 17:48:52 -0700 (In reply to comment #4) > (In reply to comment #3) > > It looks to me like CachedResourceHandle could just go away in favor of RefPtr. Am I missing something? If not, I could look into cleaning that up. > > You are missing something. Look at m_handlesToRevalidate in CachedResource to see what this does beyond what RefPtr does. Oh, I see now. Thanks for pointing that out :) 234562 6 ap 2010-06-05 00:25:20 -0700 + No new tests because no new functionality. Do you know what is holding the other reference(s)? If this patch actually fixes a bug, then landing a regression test would be way more important than even fixing it - this area isn't well covered by tests. 234716 7 tonyg 2010-06-06 21:04:24 -0700 (In reply to comment #6) > + No new tests because no new functionality. > > Do you know what is holding the other reference(s)? If this patch actually fixes a bug, then landing a regression test would be way more important than even fixing it - this area isn't well covered by tests. I don't believe there is actually a bug here. It just seems like booby trap that could be triggered by an unrelated change. When I trace through, I always have at least 1 other reference to the CachedScript. It can be from the DocLoader::m_documentResources and/or from the ScriptElement::m_cachedScript. 237451 8 eric 2010-06-12 20:41:25 -0700 Could this be related to the invalid CachedResource* crashes we were seeing for a while with the HTML5 parser? 237459 9 57918 commit-queue 2010-06-12 20:53:09 -0700 Comment on attachment 57918 Patch Rejecting patch 57918 from commit-queue. Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Darin Adler', u'--force']" exit_code: 1 Last 500 characters of output: WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. can't find file to patch at input line 5 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: WebCore/html/HTMLTokenizer.cpp |index 629e7f4187448db7f0516add263995dc5e9e2636..74063ddc5c91bc25c2208bdf52e33d5e557f4919 100644 |--- WebCore/html/HTMLTokenizer.cpp |+++ WebCore/html/HTMLTokenizer.cpp -------------------------- No file to patch. Skipping patch. 2 out of 2 hunks ignored Full output: http://webkit-commit-queue.appspot.com/results/3309015 237622 10 eric 2010-06-13 16:52:27 -0700 Sorry, I renamed *Tokenizer out from under you! 237852 11 58660 tonyg 2010-06-14 09:22:32 -0700 Created attachment 58660 Patch 239223 12 58660 commit-queue 2010-06-16 22:23:10 -0700 Comment on attachment 58660 Patch Clearing flags on attachment: 58660 Committed r61306: <http://trac.webkit.org/changeset/61306> 239224 13 commit-queue 2010-06-16 22:23:15 -0700 All reviewed patches have been landed. Closing bug. 57918 2010-06-04 14:43:28 -0700 2010-06-14 09:22:26 -0700 Patch bug-40177-20100604144327.patch text/plain 2056 tonyg ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg ZDNiYjNmN2MzNjgxNzA5NDk1NmI5OWJjYjZhNzQyNzYzM2QwMTQ1YS4uN2EwOGM3NjJmYWI3NGVl NzY0ZmFhNTM0MDBiYjQ0ODJjODk4YjQzYyAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cK KysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNiBAQAorMjAxMC0wNi0wNCAgVG9u eSBHZW50aWxjb3JlICA8dG9ueWdAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEd1YXJhbnRlZSB0aGF0IHJlZmVyZW5jZXMgYXJl IGhlbGQgZm9yIENhY2hlZFNjcmlwdHMgaW4gSFRNTFRva2VuaXplci4KKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTQwMTc3CisKKyAgICAgICAgTm8gbmV3 IHRlc3RzIGJlY2F1c2Ugbm8gbmV3IGZ1bmN0aW9uYWxpdHkuCisKKyAgICAgICAgKiBodG1sL0hU TUxUb2tlbml6ZXIuY3BwOgorICAgICAgICAoV2ViQ29yZTo6SFRNTFRva2VuaXplcjo6cmVzZXQp OgorICAgICAgICAoV2ViQ29yZTo6SFRNTFRva2VuaXplcjo6ZXhlY3V0ZUV4dGVybmFsU2NyaXB0 c0lmUmVhZHkpOgorCiAyMDEwLTA2LTA0ICBBZGFtIEJhcnRoICA8YWJhcnRoQHdlYmtpdC5vcmc+ CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRXJpYyBTZWlkZWwuCmRpZmYgLS1naXQgYS9XZWJDb3Jl L2h0bWwvSFRNTFRva2VuaXplci5jcHAgYi9XZWJDb3JlL2h0bWwvSFRNTFRva2VuaXplci5jcHAK aW5kZXggNjI5ZTdmNDE4NzQ0OGRiN2YwNTE2YWRkMjYzOTk1ZGM1ZTllMjYzNi4uNzQwNjNkZGM1 YzkxYmMyNWMyMjA4YmRmNTJlMzNkNWU1NTdmNDkxOSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9odG1s L0hUTUxUb2tlbml6ZXIuY3BwCisrKyBiL1dlYkNvcmUvaHRtbC9IVE1MVG9rZW5pemVyLmNwcApA QCAtMjE0LDcgKzIxNCw3IEBAIHZvaWQgSFRNTFRva2VuaXplcjo6cmVzZXQoKQogICAgIEFTU0VS VChtX2V4ZWN1dGluZ1NjcmlwdCA9PSAwKTsKIAogICAgIHdoaWxlICghbV9wZW5kaW5nU2NyaXB0 cy5pc0VtcHR5KCkpIHsKLSAgICAgICAgQ2FjaGVkU2NyaXB0KiBjcyA9IG1fcGVuZGluZ1Njcmlw dHMudGFrZUZpcnN0KCkuZ2V0KCk7CisgICAgICAgIENhY2hlZFJlc291cmNlSGFuZGxlPENhY2hl ZFNjcmlwdD4gY3MgPSBtX3BlbmRpbmdTY3JpcHRzLnRha2VGaXJzdCgpOwogICAgICAgICBBU1NF UlQoY2FjaGUoKS0+ZGlzYWJsZWQoKSB8fCBjcy0+YWNjZXNzQ291bnQoKSA+IDApOwogICAgICAg ICBjcy0+cmVtb3ZlQ2xpZW50KHRoaXMpOwogICAgIH0KQEAgLTIwMTAsMTQgKzIwMTAsMTQgQEAg dm9pZCBIVE1MVG9rZW5pemVyOjpleGVjdXRlRXh0ZXJuYWxTY3JpcHRzSWZSZWFkeSgpCiAgICAg ICAgIGlmICghY29udGludWVFeGVjdXRpbmdFeHRlcm5hbFNjcmlwdHMoc3RhcnRUaW1lKSkKICAg ICAgICAgICAgIGJyZWFrOwogCi0gICAgICAgIENhY2hlZFNjcmlwdCogY3MgPSBtX3BlbmRpbmdT Y3JpcHRzLnRha2VGaXJzdCgpLmdldCgpOworICAgICAgICBDYWNoZWRSZXNvdXJjZUhhbmRsZTxD YWNoZWRTY3JpcHQ+IGNzID0gbV9wZW5kaW5nU2NyaXB0cy50YWtlRmlyc3QoKTsKICAgICAgICAg QVNTRVJUKGNhY2hlKCktPmRpc2FibGVkKCkgfHwgY3MtPmFjY2Vzc0NvdW50KCkgPiAwKTsKIAog ICAgICAgICBzZXRTcmMoU2VnbWVudGVkU3RyaW5nKCkpOwogCiAgICAgICAgIC8vIG1ha2Ugc3Vy ZSB3ZSBmb3JnZXQgYWJvdXQgdGhlIHNjcmlwdCBiZWZvcmUgd2UgZXhlY3V0ZSB0aGUgbmV3IG9u ZQogICAgICAgICAvLyBpbmZpbml0ZSByZWN1cnNpb24gbWlnaHQgaGFwcGVuIG90aGVyd2lzZQot ICAgICAgICBTY3JpcHRTb3VyY2VDb2RlIHNvdXJjZUNvZGUoY3MpOworICAgICAgICBTY3JpcHRT b3VyY2VDb2RlIHNvdXJjZUNvZGUoY3MuZ2V0KCkpOwogICAgICAgICBib29sIGVycm9yT2NjdXJy ZWQgPSBjcy0+ZXJyb3JPY2N1cnJlZCgpOwogICAgICAgICBjcy0+cmVtb3ZlQ2xpZW50KHRoaXMp OwogCg== 58660 2010-06-14 09:22:32 -0700 2010-06-16 22:23:10 -0700 Patch bug-40177-20100614092231.patch text/plain 2126 tonyg ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg OGZmYjEzYWU5MWI0NzUxNThhNjMyYWQ1NjQ4YTExMmM2ZWQ4YTEzNC4uMTBmMjQ5ZDMzM2ZjN2Yw MTkwYzg2OWNlOTg4YTAyYTEzMGJhYjhiMSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cK KysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNiBAQAorMjAxMC0wNi0wNCAgVG9u eSBHZW50aWxjb3JlICA8dG9ueWdAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEd1YXJhbnRlZSB0aGF0IHJlZmVyZW5jZXMgYXJl IGhlbGQgZm9yIENhY2hlZFNjcmlwdHMgaW4gSFRNTERvY3VtZW50UGFyc2VyLgorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDAxNzcKKworICAgICAgICBO byBuZXcgdGVzdHMgYmVjYXVzZSBubyBuZXcgZnVuY3Rpb25hbGl0eS4KKworICAgICAgICAqIGh0 bWwvSFRNTERvY3VtZW50UGFyc2VyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkhUTUxEb2N1bWVu dFBhcnNlcjo6cmVzZXQpOgorICAgICAgICAoV2ViQ29yZTo6SFRNTERvY3VtZW50UGFyc2VyOjpl eGVjdXRlRXh0ZXJuYWxTY3JpcHRzSWZSZWFkeSk6CisKIDIwMTAtMDYtMTMgIFlhZWwgQWhhcm9u ICA8eWFlbC5haGFyb25Abm9raWEuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEtlbm5ldGgg Um9oZGUgQ2hyaXN0aWFuc2VuLgpkaWZmIC0tZ2l0IGEvV2ViQ29yZS9odG1sL0hUTUxEb2N1bWVu dFBhcnNlci5jcHAgYi9XZWJDb3JlL2h0bWwvSFRNTERvY3VtZW50UGFyc2VyLmNwcAppbmRleCAy NGQ2OTg1NGM5ZGU3YmNhOTA5OTlkZWMzYmVkZmI1NjRjZTA2YTViLi5jZTlhNGYwN2E5NTgyYjdm MzU3NWZhYWFjYzJkYjBlNTlhODU1ZWIxIDEwMDY0NAotLS0gYS9XZWJDb3JlL2h0bWwvSFRNTERv Y3VtZW50UGFyc2VyLmNwcAorKysgYi9XZWJDb3JlL2h0bWwvSFRNTERvY3VtZW50UGFyc2VyLmNw cApAQCAtMjEzLDcgKzIxMyw3IEBAIHZvaWQgSFRNTERvY3VtZW50UGFyc2VyOjpyZXNldCgpCiAg ICAgQVNTRVJUKG1fZXhlY3V0aW5nU2NyaXB0ID09IDApOwogCiAgICAgd2hpbGUgKCFtX3BlbmRp bmdTY3JpcHRzLmlzRW1wdHkoKSkgewotICAgICAgICBDYWNoZWRTY3JpcHQqIGNzID0gbV9wZW5k aW5nU2NyaXB0cy50YWtlRmlyc3QoKS5nZXQoKTsKKyAgICAgICAgQ2FjaGVkUmVzb3VyY2VIYW5k bGU8Q2FjaGVkU2NyaXB0PiBjcyA9IG1fcGVuZGluZ1NjcmlwdHMudGFrZUZpcnN0KCk7CiAgICAg ICAgIEFTU0VSVChjYWNoZSgpLT5kaXNhYmxlZCgpIHx8IGNzLT5hY2Nlc3NDb3VudCgpID4gMCk7 CiAgICAgICAgIGNzLT5yZW1vdmVDbGllbnQodGhpcyk7CiAgICAgfQpAQCAtMjAxMCwxNCArMjAx MCwxNCBAQCB2b2lkIEhUTUxEb2N1bWVudFBhcnNlcjo6ZXhlY3V0ZUV4dGVybmFsU2NyaXB0c0lm UmVhZHkoKQogICAgICAgICBpZiAoIWNvbnRpbnVlRXhlY3V0aW5nRXh0ZXJuYWxTY3JpcHRzKHN0 YXJ0VGltZSkpCiAgICAgICAgICAgICBicmVhazsKIAotICAgICAgICBDYWNoZWRTY3JpcHQqIGNz ID0gbV9wZW5kaW5nU2NyaXB0cy50YWtlRmlyc3QoKS5nZXQoKTsKKyAgICAgICAgQ2FjaGVkUmVz b3VyY2VIYW5kbGU8Q2FjaGVkU2NyaXB0PiBjcyA9IG1fcGVuZGluZ1NjcmlwdHMudGFrZUZpcnN0 KCk7CiAgICAgICAgIEFTU0VSVChjYWNoZSgpLT5kaXNhYmxlZCgpIHx8IGNzLT5hY2Nlc3NDb3Vu dCgpID4gMCk7CiAKICAgICAgICAgc2V0U3JjKFNlZ21lbnRlZFN0cmluZygpKTsKIAogICAgICAg ICAvLyBtYWtlIHN1cmUgd2UgZm9yZ2V0IGFib3V0IHRoZSBzY3JpcHQgYmVmb3JlIHdlIGV4ZWN1 dGUgdGhlIG5ldyBvbmUKICAgICAgICAgLy8gaW5maW5pdGUgcmVjdXJzaW9uIG1pZ2h0IGhhcHBl biBvdGhlcndpc2UKLSAgICAgICAgU2NyaXB0U291cmNlQ29kZSBzb3VyY2VDb2RlKGNzKTsKKyAg ICAgICAgU2NyaXB0U291cmNlQ29kZSBzb3VyY2VDb2RlKGNzLmdldCgpKTsKICAgICAgICAgYm9v bCBlcnJvck9jY3VycmVkID0gY3MtPmVycm9yT2NjdXJyZWQoKTsKICAgICAgICAgY3MtPnJlbW92 ZUNsaWVudCh0aGlzKTsKIAo=