Bug 59178 - Removed a use of markDOMObjectWrapper: Element and Document
Summary: Removed a use of markDOMObjectWrapper: Element and Document
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-21 23:36 PDT by Geoffrey Garen
Modified: 2011-04-22 12:57 PDT (History)
3 users (show)

See Also:


Attachments
Patch (29.05 KB, patch)
2011-04-21 23:49 PDT, Geoffrey Garen
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2011-04-21 23:36:58 PDT
Removed a use of markDOMObjectWrapper: Element and Document
Comment 1 Geoffrey Garen 2011-04-21 23:49:07 PDT
Created attachment 90674 [details]
Patch
Comment 2 WebKit Review Bot 2011-04-22 00:12:52 PDT
Attachment 90674 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8498245
Comment 3 WebKit Review Bot 2011-04-22 01:41:03 PDT
Attachment 90674 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8497280
Comment 4 Sam Weinig 2011-04-22 08:10:26 PDT
Comment on attachment 90674 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=90674&action=review

> LayoutTests/fast/dom/script-tests/dataset-gc.js:26
>  var dataset = d.dataset;
> +dataset.__proto__.customProperty = 1; // Add a property to our prototype, so it won't forward to our element.
> +dataset.customProperty = 1; // Now set a property on ourselves.
> +shouldBe("d.dataset.customProperty", "1");
>  
> -// Ensure there is a __proto__ before test.
> -shouldBeNonNull("d.dataset.__proto__");
> -
> -// Set __proto__ to non-starting value. (Must be null do to __proto__ restrictions).
> -d.dataset.__proto__ = null;
> -shouldBeNull("d.dataset.__proto__");
> -
> -// Null out reference to the dataset. 
>  dataset = null;
>  
>  gc();
>  
> -// Test that the null persisted the GC.
> -shouldBeNull("d.dataset.__proto__");
> +// Test that the custom property persisted the GC.
> +shouldBe("d.dataset.customProperty", "1");

If you are going to change this test to not use __proto__, you should ensure/assert that setting dataset.customProperty does not add an attribute to the underlying element by checking for the attribute explicitly.
Comment 5 Oliver Hunt 2011-04-22 11:17:35 PDT
Comment on attachment 90674 [details]
Patch

r=me, but fix the test as sam said, and dont break chrome
Comment 6 Geoffrey Garen 2011-04-22 12:57:28 PDT
Committed r84665: <http://trac.webkit.org/changeset/84665>