[Webkit-unassigned] [Bug 107207] Support selectively wrapping DOM accesses from certain V8 contexts.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jan 18 10:38:22 PST 2013


https://bugs.webkit.org/show_bug.cgi?id=107207





--- Comment #8 from Adam Barth <abarth at webkit.org>  2013-01-18 10:40:09 PST ---
It's hard for me to understand what this patch is doing for two reasons:

A) All the policy-related code obscures the actual meat of the wrapping and logging mechanism.
B) All the style and memory errors are very distracting.

What I'd recommend is to break this change down into a number of smaller steps:

1) The first patch would contain the wrapping mechanism, but we would not actually wrap anything.
2) The second patch would contain the policy mechanism for controlling the wrapping.

Please also run check-webkit-style before uploading patches.  The reason we have a style guide is to make it easier for people to understand code written by other people.  You'll make it easier for me to understand your code if you follow the style guide.

It's also likely that we'll need to do a few rounds on each patch to clean up the memory errors.  I can understand if you'd like feedback on the general approach before putting in the detailed work to manage memory correctly, but it's difficult for me to see the larger approach with all these errors.  To give a musical analogy, it's like asking someone what they think of a song playing on the radio when the radio has a lot of static.  In principle, you should be able to appreciate the song separately from the static, but the static is really distracting.

For the first patch, I would encourage you to focus on the core wrapping mechanism, assuming that's the part you're most interested in feedback about.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list