59263 2011-04-22 17:54:49 -0700 WebKit2: Web Inspector: Support highlighting page elements 2011-04-25 10:13:13 -0700 1 1 1 Unclassified WebKit Web Inspector (Deprecated) 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 0 bweinstein bweinstein apavlov bweinstein joepeck keishi loislo pfeldman pmuellr rik timothy webkit.review.bot yurys oldest_to_newest 391280 0 bweinstein 2011-04-22 17:54:49 -0700 The Web Inspector in WebKit2 should support highlighting page elements when they are moused-over in the inspector. <rdar://problem/8767659> 391286 1 90819 bweinstein 2011-04-22 17:59:19 -0700 Created attachment 90819 [PATCH] Fix 391291 2 webkit.review.bot 2011-04-22 18:01:24 -0700 Attachment 90819 did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.h:36: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style. 391697 3 90819 aroben 2011-04-24 23:32:23 -0700 Comment on attachment 90819 [PATCH] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=90819&action=review > Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.h:73 > + PageOverlay* m_highlightOverlay; Holding a bare pointer doesn't seem safe. If someone else calls WebPage::installPageOverlay when you aren't expecting it, m_highlightOverlay will then become a dangling pointer. 391779 4 bweinstein 2011-04-25 09:31:21 -0700 (In reply to comment #3) > (From update of attachment 90819 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=90819&action=review > > > Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.h:73 > > + PageOverlay* m_highlightOverlay; > > Holding a bare pointer doesn't seem safe. If someone else calls WebPage::installPageOverlay when you aren't expecting it, m_highlightOverlay will then become a dangling pointer. It looks like FindController does the same thing. Is that code wrong as well? It looks like WebPage.h holds a RefPtr to a single overlay. 391802 5 bweinstein 2011-04-25 10:03:22 -0700 (In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 90819 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=90819&action=review > > > > > Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.h:73 > > > + PageOverlay* m_highlightOverlay; > > > > Holding a bare pointer doesn't seem safe. If someone else calls WebPage::installPageOverlay when you aren't expecting it, m_highlightOverlay will then become a dangling pointer. > > It looks like FindController does the same thing. Is that code wrong as well? It looks like WebPage.h holds a RefPtr to a single overlay. I'm going to land this with Alice's r+, and we can discuss a better way to clean this and FindController up. 391806 6 bweinstein 2011-04-25 10:13:13 -0700 Landed in r84781. Filed http://webkit.org/b/59329 to track the raw PageOverlay pointers in WebInspectorClient and FindController. 90819 2011-04-22 17:59:19 -0700 2011-04-24 23:32:23 -0700 [PATCH] Fix webkit2_highlight.patch text/plain 5094 bweinstein SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDg0NzE2KQorKysgU291cmNlL1dlYktpdDIvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjggQEAKKzIwMTEtMDQtMjIgIEJyaWFuIFdl aW5zdGVpbiAgPGJ3ZWluc3RlaW5AYXBwbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIFdlYktpdDI6IFdlYiBJbnNwZWN0b3I6IFN1cHBvcnQg aGlnaGxpZ2h0aW5nIHBhZ2UgZWxlbWVudHMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTU5MjYzCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS84NzY3NjU5 PgorCisgICAgICAgIFN1cHBvcnQgaGlnaGxpZ2h0aW5nIG9mIHBhZ2UgZWxlbWVudHMgZm9yIHRo ZSBpbnNwZWN0b3IgaW4gV2ViS2l0MiBieQorICAgICAgICBoYXZpbmcgdGhlIFdlYkluc3BlY3Rv ckNsaWVudCBiZSBhIFBhZ2VPdmVybGF5IENsaWVudCBhbmQgYmUgcmVzcG9uc2libGUKKyAgICAg ICAgZm9yIGNhbGxpbmcgSW5zcGVjdG9yQ29udHJvbGxlcjo6ZHJhd05vZGVIaWdobGlnaHQuCisK KyAgICAgICAgKiBXZWJQcm9jZXNzL1dlYkNvcmVTdXBwb3J0L1dlYkluc3BlY3RvckNsaWVudC5j cHA6CisgICAgICAgIChXZWJLaXQ6OldlYkluc3BlY3RvckNsaWVudDo6aGlnaGxpZ2h0KTogQ3Jl YXRlIG91ciBvdmVybGF5IGlmIGl0IGhhc24ndCBiZWVuIGNyZWF0ZWQsCisgICAgICAgICAgICBv ciBjYWxsIHNldE5lZWRzRGlzcGxheSBpZiBpdCBoYXMuCisgICAgICAgIChXZWJLaXQ6OldlYklu c3BlY3RvckNsaWVudDo6aGlkZUhpZ2hsaWdodCk6IFVuaW5zdGFsbCB0aGUgcGFnZSBvdmVybGF5 IGlmIGl0IGV4aXN0cy4KKyAgICAgICAgKFdlYktpdDo6V2ViSW5zcGVjdG9yQ2xpZW50OjpwYWdl T3ZlcmxheURlc3Ryb3llZCk6CisgICAgICAgIChXZWJLaXQ6OldlYkluc3BlY3RvckNsaWVudDo6 d2lsbE1vdmVUb1dlYlBhZ2UpOiBJZiBpdCdzIGJlaW5nIGRlc3Ryb3llZCAobW92aW5nIHRvIGEg bnVsbAorICAgICAgICAgICAgV2ViUGFnZSksIGNsZWFyIHRoZSBtZW1iZXIgdmFyaWFibGUuCisg ICAgICAgIChXZWJLaXQ6OldlYkluc3BlY3RvckNsaWVudDo6ZGlkTW92ZVRvV2ViUGFnZSk6Cisg ICAgICAgIChXZWJLaXQ6OldlYkluc3BlY3RvckNsaWVudDo6ZHJhd1JlY3QpOiBDYWxsIEluc3Bl Y3RvckNvbnRyb2xsZXI6OmRyYXdOb2RlSGlnaGxpZ2h0LgorICAgICAgICAoV2ViS2l0OjpXZWJJ bnNwZWN0b3JDbGllbnQ6Om1vdXNlRXZlbnQpOgorICAgICAgICAqIFdlYlByb2Nlc3MvV2ViQ29y ZVN1cHBvcnQvV2ViSW5zcGVjdG9yQ2xpZW50Lmg6CisgICAgICAgIChXZWJLaXQ6OldlYkluc3Bl Y3RvckNsaWVudDo6V2ViSW5zcGVjdG9yQ2xpZW50KTogQWRkIGEgbmV3IG1lbWJlciB2YXJpYWJs ZS4KKwogMjAxMS0wNC0yMiAgQWxlamFuZHJvIEcuIENhc3RybyAgPGFsZXhAaWdhbGlhLmNvbT4g YW5kIE1hcnRpbiBSb2JpbnNvbiAgPG1yb2JpbnNvbkBpZ2FsaWEuY29tPgogCiAgICAgICAgIFJl dmlld2VkIGJ5IFhhbiBMb3Blei4KSW5kZXg6IFNvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2Vi Q29yZVN1cHBvcnQvV2ViSW5zcGVjdG9yQ2xpZW50LmNwcAo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv V2ViS2l0Mi9XZWJQcm9jZXNzL1dlYkNvcmVTdXBwb3J0L1dlYkluc3BlY3RvckNsaWVudC5jcHAJ KHJldmlzaW9uIDg0NzEyKQorKysgU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJDb3JlU3Vw cG9ydC9XZWJJbnNwZWN0b3JDbGllbnQuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC01NCwxNCArNTQs MjAgQEAgdm9pZCBXZWJJbnNwZWN0b3JDbGllbnQ6Om9wZW5JbnNwZWN0b3JGcgogICAgIGluc3Bl Y3RvclBhZ2UtPmNvcmVQYWdlKCktPmluc3BlY3RvckNvbnRyb2xsZXIoKS0+c2V0SW5zcGVjdG9y RnJvbnRlbmRDbGllbnQoYWRvcHRQdHIobmV3IFdlYkluc3BlY3RvckZyb250ZW5kQ2xpZW50KG1f cGFnZSwgaW5zcGVjdG9yUGFnZSkpKTsKIH0KIAotdm9pZCBXZWJJbnNwZWN0b3JDbGllbnQ6Omhp Z2hsaWdodChOb2RlKikKK3ZvaWQgV2ViSW5zcGVjdG9yQ2xpZW50OjpoaWdobGlnaHQoTm9kZSog bm9kZSkKIHsKLSAgICBub3RJbXBsZW1lbnRlZCgpOworICAgIGlmICghbV9oaWdobGlnaHRPdmVy bGF5KSB7CisgICAgICAgIFJlZlB0cjxQYWdlT3ZlcmxheT4gaGlnaGxpZ2h0T3ZlcmxheSA9IFBh Z2VPdmVybGF5OjpjcmVhdGUodGhpcyk7CisgICAgICAgIG1faGlnaGxpZ2h0T3ZlcmxheSA9IGhp Z2hsaWdodE92ZXJsYXkuZ2V0KCk7CisgICAgICAgIG1fcGFnZS0+aW5zdGFsbFBhZ2VPdmVybGF5 KGhpZ2hsaWdodE92ZXJsYXkucmVsZWFzZSgpKTsKKyAgICB9IGVsc2UKKyAgICAgICAgbV9oaWdo bGlnaHRPdmVybGF5LT5zZXROZWVkc0Rpc3BsYXkoKTsKIH0KIAogdm9pZCBXZWJJbnNwZWN0b3JD bGllbnQ6OmhpZGVIaWdobGlnaHQoKQogewotICAgIG5vdEltcGxlbWVudGVkKCk7CisgICAgaWYg KG1faGlnaGxpZ2h0T3ZlcmxheSkKKyAgICAgICAgbV9wYWdlLT51bmluc3RhbGxQYWdlT3Zlcmxh eShtX2hpZ2hsaWdodE92ZXJsYXksIGZhbHNlKTsKIH0KIAogdm9pZCBXZWJJbnNwZWN0b3JDbGll bnQ6OnBvcHVsYXRlU2V0dGluZyhjb25zdCBTdHJpbmcmIGtleSwgU3RyaW5nKikKQEAgLTg1LDYg KzkxLDM0IEBAIGJvb2wgV2ViSW5zcGVjdG9yQ2xpZW50OjpzZW5kTWVzc2FnZVRvRnIKICAgICBy ZXR1cm4gZG9EaXNwYXRjaE1lc3NhZ2VPbkZyb250ZW5kUGFnZShpbnNwZWN0b3JQYWdlLT5jb3Jl UGFnZSgpLCBtZXNzYWdlKTsKIH0KIAordm9pZCBXZWJJbnNwZWN0b3JDbGllbnQ6OnBhZ2VPdmVy bGF5RGVzdHJveWVkKFBhZ2VPdmVybGF5KikKK3sKK30KKwordm9pZCBXZWJJbnNwZWN0b3JDbGll bnQ6OndpbGxNb3ZlVG9XZWJQYWdlKFBhZ2VPdmVybGF5KiwgV2ViUGFnZSogd2ViUGFnZSkKK3sK KyAgICBpZiAod2ViUGFnZSkKKyAgICAgICAgcmV0dXJuOworCisgICAgLy8gVGhlIHBhZ2Ugb3Zl cmxheSBpcyBtb3ZpbmcgYXdheSBmcm9tIHRoZSB3ZWIgcGFnZSwgcmVzZXQgaXQuCisgICAgQVNT RVJUKG1faGlnaGxpZ2h0T3ZlcmxheSk7CisgICAgbV9oaWdobGlnaHRPdmVybGF5ID0gMDsKK30K Kwordm9pZCBXZWJJbnNwZWN0b3JDbGllbnQ6OmRpZE1vdmVUb1dlYlBhZ2UoUGFnZU92ZXJsYXkq LCBXZWJQYWdlKikKK3sKK30KKwordm9pZCBXZWJJbnNwZWN0b3JDbGllbnQ6OmRyYXdSZWN0KFBh Z2VPdmVybGF5KiBvdmVybGF5LCBXZWJDb3JlOjpHcmFwaGljc0NvbnRleHQmIGNvbnRleHQsIGNv bnN0IFdlYkNvcmU6OkludFJlY3QmIGRpcnR5UmVjdCkKK3sKKyAgICBtX3BhZ2UtPmNvcmVQYWdl KCktPmluc3BlY3RvckNvbnRyb2xsZXIoKS0+ZHJhd05vZGVIaWdobGlnaHQoY29udGV4dCk7Cit9 CisKK2Jvb2wgV2ViSW5zcGVjdG9yQ2xpZW50Ojptb3VzZUV2ZW50KFBhZ2VPdmVybGF5KiwgY29u c3QgV2ViTW91c2VFdmVudCYpCit7CisgICAgcmV0dXJuIGZhbHNlOworfQorCiB9IC8vIG5hbWVz cGFjZSBXZWJLaXQKIAogI2VuZGlmIC8vIEVOQUJMRShJTlNQRUNUT1IpCkluZGV4OiBTb3VyY2Uv V2ViS2l0Mi9XZWJQcm9jZXNzL1dlYkNvcmVTdXBwb3J0L1dlYkluc3BlY3RvckNsaWVudC5oCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2ViQ29yZVN1cHBvcnQvV2Vi SW5zcGVjdG9yQ2xpZW50LmgJKHJldmlzaW9uIDg0NzEyKQorKysgU291cmNlL1dlYktpdDIvV2Vi UHJvY2Vzcy9XZWJDb3JlU3VwcG9ydC9XZWJJbnNwZWN0b3JDbGllbnQuaAkod29ya2luZyBjb3B5 KQpAQCAtMjgsMTYgKzI4LDI0IEBACiAKICNpZiBFTkFCTEUoSU5TUEVDVE9SKQogCisjaW5jbHVk ZSAiUGFnZU92ZXJsYXkuaCIKKwogI2luY2x1ZGUgPFdlYkNvcmUvSW5zcGVjdG9yQ2xpZW50Lmg+ CiAKK25hbWVzcGFjZSBXZWJDb3JlIHsKKyAgICBjbGFzcyBHcmFwaGljc0NvbnRleHQ7CisgICAg Y2xhc3MgSW50UmVjdDsKK30KKwogbmFtZXNwYWNlIFdlYktpdCB7CiAKIGNsYXNzIFdlYlBhZ2U7 CiAKLWNsYXNzIFdlYkluc3BlY3RvckNsaWVudCA6IHB1YmxpYyBXZWJDb3JlOjpJbnNwZWN0b3JD bGllbnQgeworY2xhc3MgV2ViSW5zcGVjdG9yQ2xpZW50IDogcHVibGljIFdlYkNvcmU6Okluc3Bl Y3RvckNsaWVudCwgcHJpdmF0ZSBQYWdlT3ZlcmxheTo6Q2xpZW50IHsKIHB1YmxpYzoKICAgICBX ZWJJbnNwZWN0b3JDbGllbnQoV2ViUGFnZSogcGFnZSkKICAgICAgICAgOiBtX3BhZ2UocGFnZSkK KyAgICAgICAgLCBtX2hpZ2hsaWdodE92ZXJsYXkoMCkKICAgICB7CiAgICAgfQogCkBAIC01NCw3 ICs2MiwxNSBAQCBwcml2YXRlOgogCiAgICAgdmlydHVhbCBib29sIHNlbmRNZXNzYWdlVG9Gcm9u dGVuZChjb25zdCBTdHJpbmcmKTsKIAorICAgIC8vIFBhZ2VPdmVybGF5OjpDbGllbnQKKyAgICB2 aXJ0dWFsIHZvaWQgcGFnZU92ZXJsYXlEZXN0cm95ZWQoUGFnZU92ZXJsYXkqKTsKKyAgICB2aXJ0 dWFsIHZvaWQgd2lsbE1vdmVUb1dlYlBhZ2UoUGFnZU92ZXJsYXkqLCBXZWJQYWdlKik7CisgICAg dmlydHVhbCB2b2lkIGRpZE1vdmVUb1dlYlBhZ2UoUGFnZU92ZXJsYXkqLCBXZWJQYWdlKik7Cisg ICAgdmlydHVhbCB2b2lkIGRyYXdSZWN0KFBhZ2VPdmVybGF5KiwgV2ViQ29yZTo6R3JhcGhpY3ND b250ZXh0JiwgY29uc3QgV2ViQ29yZTo6SW50UmVjdCYpOworICAgIHZpcnR1YWwgYm9vbCBtb3Vz ZUV2ZW50KFBhZ2VPdmVybGF5KiwgY29uc3QgV2ViTW91c2VFdmVudCYpOworCiAgICAgV2ViUGFn ZSogbV9wYWdlOworICAgIFBhZ2VPdmVybGF5KiBtX2hpZ2hsaWdodE92ZXJsYXk7CiB9OwogCiB9 IC8vIG5hbWVzcGFjZSBXZWJLaXQK