Re: [webkit-dev] JS bindings: Adding EventTarget to the prototype chain

[Dominic Cooney]

Thread necromancy! To summarize the original post: Currently WebKit’s JS
binding implements EventTarget as a mixin—all event targets have
addEventListener, removeEventListener and dispatchEvent methods. I propose
WebKit put EventTarget on the prototype chain.

There were two basic questions in response: (1) Why? and (2) Where are the
specs going?

(1) Why? To summarize the rationale from off-list replies, spec mailing list
threads, and webapps bug threads:

Making event targets inherit from EventTarget simplifies hooking that
functionality from a single choke-point. Otherwise, the web developer has to
track down all those separate objects hook each one. Why is EventTarget
special? What about other mixins? At the TC39 meeting in November, during a
discussion about removing multiple inheritance, EventTarget was brought up
as the mixin what would probably want to have global hooking done to it the
most.

(2) Where are the specs going?

The momentum for specifying EventTarget as an interface is building. In my
original post I noted that many specs (DOM Core, Notifications, Indexed DB,
SVG, XHR) already specified EventTarget as an interface; now the specs Hixie
edits (HTML5, Web RTC, Workers, Web Sockets) and FileReader do too.

Furthermore, there’s a comment from Jonas Sicking <
http://www.w3.org/Bugs/Public/show_bug.cgi?id=12574#c3> indicating that
Firefox is going to implement EventTarget this way, and from Travis
Leithead  endorsing
implementing EventTarget this way.

Which by my rough reckoning of specs implemented by WebKit, leaves just
FileWriter and Web Audio. Both of those specs are in a funny place where
they don’t specify EventTarget at all, but are event targets in WebKit. I
will follow up with Eric Urhane and Chris Rogers about these.

As I mentioned in my original post, the performance impact of this change is
minimal. So now that the specs are lined up, are there any objections to
making this change? If not I am going to work on reworking the prototype I
used to measure performance into a proper patch.

Dominic

On Fri, Jun 10, 2011 at 7:53 AM, Maciej Stachowiak  wrote:

>
> I don't have a personal opinion on which way is technically better myself.
> But I think the key is getting our code aligned with where standards are
> going, wether by changing he code or the standards. EventTarget in the
> prototype chain seems neither especially awesome nor especially terrible to
> me, it is not really clear to me why editor's drafts of the listed specs
> decided to change.
>
> Cheers,
> Maciej
>
>
>
> On Jun 9, 2011, at 1:14 PM, Sam Weinig wrote:
>
> I don't think we should do this.  EventTarget is really just an abstract
> interface, and changing its implementation globally is of limited utility.
>
> -Sam
>
> On Jun 8, 2011, at 5:54 PM, Dominic Cooney wrote:
>
> [If you don't care about JSC or V8 bindings you can safely ignore
> this.]
>
> TL;DR I want to change the JavaScript bindings to put EventTarget on
> the prototype chain so it is easier to work with event targets from
> JavaScript. What do you think?
>
> Here is the prototype chain for a button today:
>
> HTMLButtonElement-HTMLElement-Element-Node-Object
> (add/removeEventListener and dispatchEvent are on Node.)
>
> Here is how I think we should make it look:
>
> HTMLButtonElement-HTMLElement-Element-Node-EventTarget-Object
> (addEventListener etc. are on EventTarget.)
>
> Here’s why I think we should do this:
>
> - Specs are moving towards specifying EventTarget as living on the
> prototype chain. DOM Core*, Notifications, Indexed DB, SVG and XHR
> already do it this way. (* Editor’s draft.)
>
> - Frameworks that want to hook add/removeEventListener will be able to
> do it in one place: on EventTarget.prototype. In WebKit today they
> have to hook the prototypes of window, Node, XMLHttpRequest, etc.
> individually (Because WebKit implements EventTarget as a mixin
> everywhere, there are 20+ different kinds of event targets to hook if
> you want to be comprehensive.) If we make this change, it gets easier
> to tell if a given object is an EventTarget; just do
> EventTarget.prototype.isPrototypeOf(something).
>
> - It will modestly simplify WebKit’s IDLs and bindings. Instead of
> declaring addEventListener in two dozen places in IDLs, it will be in
> one place; instead of calling visitJSEventListeners in dozens of
> places for JSC GC, it will be called in one place; instead of assuming
> that EventTarget parameters are all Node* under the covers for V8 code
> generation, we can treat EventTargets uniformly; instead of
> redundantly specifying EventTarget on Document and Node inheritance
> will do what you want; etc.
>
> Will doing this break the web? I don’t think so:
>
> Anyone who calls or hooks addEventListener, etc. will continue to
> work, just their foo.addEventListener might be resolved at one level
> higher up the prototype chain than it is today. To really observe the
> different p

Re: [webkit-dev] Current legacy argument refactoring -- behavior changes?

[Adam Barth]

To clarify:

This message isn't meant to say "if you don't like any of these, speak
up" (although you should feel free to speak up, as always).  It's
meant to encourage folks to review the forthcomming patches with the
following standard for the underlying behavior change (i.e., not
"just" as patches that add some tests):

othermaciej: in other words, for each change, there should be a good
reason to do it, not lack of a good reason not to do it
othermaciej: so if anyone does care to review the matter further, they
should be looking at the justifications, not just the behavior
changes, and thinking "is this a good reason to change", not just "do
I know of some other problem caused by this"

Thanks,
Adam


On Fri, Aug 5, 2011 at 3:04 PM, Adam Barth  wrote:
> Maciej and I discussed this in IRC.  We agree that it's important to
> discuss the changes more broadly within the project, but we disagree
> about the best format for that discussion.
>
> If you're interested in these sorts of changes, please feel free to
> add yourself to the CC list of this bug.  Mark and I will be writing
> tests for all of the behavior changes and blocking those bugs against
> this one:
>
> https://bugs.webkit.org/show_bug.cgi?id=65787
>
> The patches containing the tests will provide a forum for discussing
> whether the behavior change is beneficial.  If you don't think it's
> beneficial, please either let us know or comment on the appropriate
> bug as it goes past.  Maciej is worried about the existing changes
> having "incumbency," so we'll try to avoid treating the current
> behavior as incumbent in those discussions.
>
> Adam
>
>
> On Fri, Aug 5, 2011 at 2:16 PM, Maciej Stachowiak  wrote:
>>
>> On Aug 5, 2011, at 1:39 PM, Adam Barth wrote:
>>
>>>
 My proposal is to revert all the patches that changed behavior without 
 incorporating tests, and we can decide how to proceed from there. If it's 
 hard to even tell which those are, then we should just revert the whole 
 patch set. Then we can do this right.
>>>
>>> I'm not sure that's necessary.  We can look over the list of changed
>>> APIs just as easily without reverting the patches.  Are there any in
>>> the list that Mark set out that you'd like to discuss further?
>>
>> I can't actually tell what the behavior changes were from the patches.
>>
>> For example, Mark said that the following patch changes 
>> HTMLAnchorElement.getParameter:
>>
>> https://bugs.webkit.org/attachment.cgi?id=102840&action=review
>>
>> How am I supposed to know from that diff that HTMLAnchorElement.getParameter 
>> changed, or how it changed? How am I supposed to verify that Mark's list of 
>> methods whose behavior changed is complete and correct?
>>
>> It's basically impossible to meaningfully discuss the changes.
>>
>>
>> That is why, for behavior-changing patches, the test should be reviewed 
>> *with* the behavior-changing code change, especially when the behavior 
>> change is very subtle in the code. Adding tests after the fact does not give 
>> people a meaningful opportunity to comment on the changes.
>>
>>
>> In fact, I am skeptical that you meaningfully reviewed the behavior changes 
>> caused by the above patch. I don't see any mention of the functions that 
>> changed behavior in the ChangeLog, in the patch, in the bug, or in your 
>> review comments. Even though the changes are called intentional, they give 
>> every appearance of being accidental and give no appearance of having been 
>> carefully considered and reviewed.
>>
>>
>>
>> By contrast, here is an example of a much better patch: 
>> https://bugs.webkit.org/attachment.cgi?id=98024&action=review
>>
>> It has a test and mentions the behavior change in ChangeLog and in the bug. 
>> There is at least an implied justification of "match the spec" My one 
>> complaint about this one is that it also changes behavior of .getKey() as 
>> well as .get(), but the ChangeLog does not mention this (though it is 
>> covered by the test).
>>
>>
>>
>> I don't think it's good process to say that changes which were never 
>> justified or reviewed are now our new baseline, and anyone objecting to one 
>> of these changes has to come up with a reason. Even though it's impossible 
>> to tell from the diffs what the changes are. Our default should be to have 
>> *no* behavior changes, and any behavior change should be explicitly 
>> justified.
>>
>>
>> Therefore I still think revert the correct action, for patches that came 
>> with no test or explanation and justification of the changes.. These changes 
>> should be accompanied with a test and a justification before they are 
>> reviewed. This goes double for interfaces that are in wide use. I don't 
>> agree that just because these changes slipped in, the burden of proof is on 
>> others to rebut them, rather than on advocates of those changes to justify 
>> them.
>>
>>
>> Regards,
>> Maciej
>>
>>
>
___
webkit-dev mailing list
webkit-dev@li

Re: [webkit-dev] Current legacy argument refactoring -- behavior changes?

[Adam Barth]

Maciej and I discussed this in IRC.  We agree that it's important to
discuss the changes more broadly within the project, but we disagree
about the best format for that discussion.

If you're interested in these sorts of changes, please feel free to
add yourself to the CC list of this bug.  Mark and I will be writing
tests for all of the behavior changes and blocking those bugs against
this one:

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

The patches containing the tests will provide a forum for discussing
whether the behavior change is beneficial.  If you don't think it's
beneficial, please either let us know or comment on the appropriate
bug as it goes past.  Maciej is worried about the existing changes
having "incumbency," so we'll try to avoid treating the current
behavior as incumbent in those discussions.

Adam


On Fri, Aug 5, 2011 at 2:16 PM, Maciej Stachowiak  wrote:
>
> On Aug 5, 2011, at 1:39 PM, Adam Barth wrote:
>
>>
>>> My proposal is to revert all the patches that changed behavior without 
>>> incorporating tests, and we can decide how to proceed from there. If it's 
>>> hard to even tell which those are, then we should just revert the whole 
>>> patch set. Then we can do this right.
>>
>> I'm not sure that's necessary.  We can look over the list of changed
>> APIs just as easily without reverting the patches.  Are there any in
>> the list that Mark set out that you'd like to discuss further?
>
> I can't actually tell what the behavior changes were from the patches.
>
> For example, Mark said that the following patch changes 
> HTMLAnchorElement.getParameter:
>
> https://bugs.webkit.org/attachment.cgi?id=102840&action=review
>
> How am I supposed to know from that diff that HTMLAnchorElement.getParameter 
> changed, or how it changed? How am I supposed to verify that Mark's list of 
> methods whose behavior changed is complete and correct?
>
> It's basically impossible to meaningfully discuss the changes.
>
>
> That is why, for behavior-changing patches, the test should be reviewed 
> *with* the behavior-changing code change, especially when the behavior change 
> is very subtle in the code. Adding tests after the fact does not give people 
> a meaningful opportunity to comment on the changes.
>
>
> In fact, I am skeptical that you meaningfully reviewed the behavior changes 
> caused by the above patch. I don't see any mention of the functions that 
> changed behavior in the ChangeLog, in the patch, in the bug, or in your 
> review comments. Even though the changes are called intentional, they give 
> every appearance of being accidental and give no appearance of having been 
> carefully considered and reviewed.
>
>
>
> By contrast, here is an example of a much better patch: 
> https://bugs.webkit.org/attachment.cgi?id=98024&action=review
>
> It has a test and mentions the behavior change in ChangeLog and in the bug. 
> There is at least an implied justification of "match the spec" My one 
> complaint about this one is that it also changes behavior of .getKey() as 
> well as .get(), but the ChangeLog does not mention this (though it is covered 
> by the test).
>
>
>
> I don't think it's good process to say that changes which were never 
> justified or reviewed are now our new baseline, and anyone objecting to one 
> of these changes has to come up with a reason. Even though it's impossible to 
> tell from the diffs what the changes are. Our default should be to have *no* 
> behavior changes, and any behavior change should be explicitly justified.
>
>
> Therefore I still think revert the correct action, for patches that came with 
> no test or explanation and justification of the changes.. These changes 
> should be accompanied with a test and a justification before they are 
> reviewed. This goes double for interfaces that are in wide use. I don't agree 
> that just because these changes slipped in, the burden of proof is on others 
> to rebut them, rather than on advocates of those changes to justify them.
>
>
> Regards,
> Maciej
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Current legacy argument refactoring -- behavior changes?

[Mark Pilgrim]

On Fri, Aug 5, 2011 at 5:16 PM, Maciej Stachowiak  wrote:
> For example, Mark said that the following patch changes 
> HTMLAnchorElement.getParameter:
>
> https://bugs.webkit.org/attachment.cgi?id=102840&action=review
>
> [...] I don't see any mention of the functions that changed behavior in the 
> ChangeLog, in the patch, in the bug, or in your review comments.

https://bugs.webkit.org/show_bug.cgi?id=65338#c2

"You can skip this one.  getParameter is very new."
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Current legacy argument refactoring -- behavior changes?

[Maciej Stachowiak]

On Aug 5, 2011, at 1:39 PM, Adam Barth wrote:

> 
>> My proposal is to revert all the patches that changed behavior without 
>> incorporating tests, and we can decide how to proceed from there. If it's 
>> hard to even tell which those are, then we should just revert the whole 
>> patch set. Then we can do this right.
> 
> I'm not sure that's necessary.  We can look over the list of changed
> APIs just as easily without reverting the patches.  Are there any in
> the list that Mark set out that you'd like to discuss further?

I can't actually tell what the behavior changes were from the patches. 

For example, Mark said that the following patch changes 
HTMLAnchorElement.getParameter:

https://bugs.webkit.org/attachment.cgi?id=102840&action=review

How am I supposed to know from that diff that HTMLAnchorElement.getParameter 
changed, or how it changed? How am I supposed to verify that Mark's list of 
methods whose behavior changed is complete and correct? 

It's basically impossible to meaningfully discuss the changes.


That is why, for behavior-changing patches, the test should be reviewed *with* 
the behavior-changing code change, especially when the behavior change is very 
subtle in the code. Adding tests after the fact does not give people a 
meaningful opportunity to comment on the changes.


In fact, I am skeptical that you meaningfully reviewed the behavior changes 
caused by the above patch. I don't see any mention of the functions that 
changed behavior in the ChangeLog, in the patch, in the bug, or in your review 
comments. Even though the changes are called intentional, they give every 
appearance of being accidental and give no appearance of having been carefully 
considered and reviewed. 



By contrast, here is an example of a much better patch: 
https://bugs.webkit.org/attachment.cgi?id=98024&action=review

It has a test and mentions the behavior change in ChangeLog and in the bug. 
There is at least an implied justification of "match the spec" My one complaint 
about this one is that it also changes behavior of .getKey() as well as .get(), 
but the ChangeLog does not mention this (though it is covered by the test).



I don't think it's good process to say that changes which were never justified 
or reviewed are now our new baseline, and anyone objecting to one of these 
changes has to come up with a reason. Even though it's impossible to tell from 
the diffs what the changes are. Our default should be to have *no* behavior 
changes, and any behavior change should be explicitly justified.


Therefore I still think revert the correct action, for patches that came with 
no test or explanation and justification of the changes.. These changes should 
be accompanied with a test and a justification before they are reviewed. This 
goes double for interfaces that are in wide use. I don't agree that just 
because these changes slipped in, the burden of proof is on others to rebut 
them, rather than on advocates of those changes to justify them.


Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Current legacy argument refactoring -- behavior changes?

[Adam Barth]

I don't fully understand what's going on with those APIs.  My understanding
prior to this morning is that they were special cased by the code
generator.  It's on my list of things to investigate today.

Adam
 On Aug 5, 2011 1:48 PM, "Sam Weinig"  wrote:
> This list doesn't seem to include the changes in behavior to
addEventListener and removeEventListerner (from patches like
http://trac.webkit.org/changeset/92469
http://trac.webkit.org/changeset/92433). Are those intentionally left out?
>
> -Sam
>
>
> On Aug 5, 2011, at 12:44 PM, Mark Pilgrim wrote:
>
>> Here is the complete list of intentional behavior changes in patches
>> I've submitted in the past few months, along with bug URLs to show the
>> discussion / rationale behind each change:
>>
>>
>> https://bugs.webkit.org/show_bug.cgi?id=63065
>>
>> IDBFactory.open()
>>
>> https://bugs.webkit.org/show_bug.cgi?id=63079
>>
>> IDBIndex.get()
>> IDBIndex.getKey()
>>
>> https://bugs.webkit.org/show_bug.cgi?id=63085
>>
>> IDBKeyRange.only()
>> IDBKeyRange.lowerBound()
>> IDBKeyRange.upperBound()
>> IDBKeyRange.bound()
>>
>> https://bugs.webkit.org/show_bug.cgi?id=63087
>>
>> IDBObjectStore.put()
>> IDBObjectStore.add()
>> IDBObjectStore.delete()
>> IDBObjectStore.get()
>> IDBObjectStore.createIndex()
>> IDBObjectStore.index()
>> IDBObjectStore.deleteIndex()
>>
>> https://bugs.webkit.org/show_bug.cgi?id=63140
>>
>> IDBDatabase.createObjectStore()
>> IDBDatabase.deleteObjectStore()
>> IDBDatabase.setVersion()
>> IDBDatabase.transaction()
>>
>> https://bugs.webkit.org/show_bug.cgi?id=64539
>>
>> all arguments in File API-related methods not already marked [Optional]
>>
>> https://bugs.webkit.org/show_bug.cgi?id=64549
>>
>> all arguments in WebGL-related methods not already marked [Optional]
>>
>> https://bugs.webkit.org/show_bug.cgi?id=65338
>>
>> DOMTokenList.item()
>> DOMTokenList.contains()
>> DOMTokenList.add()
>> DOMTokenList.remove()
>> DOMTokenList.toggle()
>> DOMURL.createObjectURL()
>> DOMURL.revokeObjectURL()
>> HTMLAnchorElement.getParameter()
>> HTMLButtonElement.setCustomValidity()
>> HTMLFieldSetElement.setCustomValidity()
>> HTMLInputElement.setCustomValidity()
>> HTMLKeygenElement.setCustomValidity()
>> HTMLMediaElement.canPlayType()
>> TimeRanges.start()
>> TimeRanges.end()
>> HTMLAllCollection.namedItem()
>> HTMLAllCollection.tags()
>>
>> https://bugs.webkit.org/show_bug.cgi?id=65353
>>
>> DOMWindow.webkitRequestFileSystem()
>> DOMWindow.webkitResolveLocalFileSystemURL()
>> DOMWindow.webkitRequestAnimationFrame() (reverted in
>> https://bugs.webkit.org/show_bug.cgi?id=65698 )
>> DOMWindow.webkitCancelRequestAnimationFrame() (reverted in
>> https://bugs.webkit.org/show_bug.cgi?id=65698 )
>>
>> https://bugs.webkit.org/show_bug.cgi?id=65355
>>
>> Geolocation.clearWatch()
>> PositionCallback.handleEvent()
>> PositionErrorCallback.handleEvent()
>>
>> https://bugs.webkit.org/show_bug.cgi?id=65370
>>
>> Navigator.registerProtocolHandler()
>>
>> https://bugs.webkit.org/show_bug.cgi?id=65570
>>
>> SpeechInputResultList.item()
>>
>> https://bugs.webkit.org/show_bug.cgi?id=65571
>>
>> WebKitAnimationList.item()
>>
>> https://bugs.webkit.org/show_bug.cgi?id=65715
>>
>> MediaStreamList.item()
>> MediaStreamTrackList.item()
>> TouchList.item()
>>
>> https://bugs.webkit.org/show_bug.cgi?id=65750
>>
>> AudioBufferSourceNode.noteOn()
>> AudioBufferSourceNode.noteGrainOn()
>> AudioBufferSourceNode.noteOff()
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Current legacy argument refactoring -- behavior changes?

[Sam Weinig]

This list doesn't seem to include the changes in behavior to addEventListener 
and removeEventListerner (from patches like 
http://trac.webkit.org/changeset/92469 http://trac.webkit.org/changeset/92433). 
 Are those intentionally left out?

-Sam


On Aug 5, 2011, at 12:44 PM, Mark Pilgrim wrote:

> Here is the complete list of intentional behavior changes in patches
> I've submitted in the past few months, along with bug URLs to show the
> discussion / rationale behind each change:
> 
> 
> https://bugs.webkit.org/show_bug.cgi?id=63065
> 
> IDBFactory.open()
> 
> https://bugs.webkit.org/show_bug.cgi?id=63079
> 
> IDBIndex.get()
> IDBIndex.getKey()
> 
> https://bugs.webkit.org/show_bug.cgi?id=63085
> 
> IDBKeyRange.only()
> IDBKeyRange.lowerBound()
> IDBKeyRange.upperBound()
> IDBKeyRange.bound()
> 
> https://bugs.webkit.org/show_bug.cgi?id=63087
> 
> IDBObjectStore.put()
> IDBObjectStore.add()
> IDBObjectStore.delete()
> IDBObjectStore.get()
> IDBObjectStore.createIndex()
> IDBObjectStore.index()
> IDBObjectStore.deleteIndex()
> 
> https://bugs.webkit.org/show_bug.cgi?id=63140
> 
> IDBDatabase.createObjectStore()
> IDBDatabase.deleteObjectStore()
> IDBDatabase.setVersion()
> IDBDatabase.transaction()
> 
> https://bugs.webkit.org/show_bug.cgi?id=64539
> 
> all arguments in File API-related methods not already marked [Optional]
> 
> https://bugs.webkit.org/show_bug.cgi?id=64549
> 
> all arguments in WebGL-related methods not already marked [Optional]
> 
> https://bugs.webkit.org/show_bug.cgi?id=65338
> 
> DOMTokenList.item()
> DOMTokenList.contains()
> DOMTokenList.add()
> DOMTokenList.remove()
> DOMTokenList.toggle()
> DOMURL.createObjectURL()
> DOMURL.revokeObjectURL()
> HTMLAnchorElement.getParameter()
> HTMLButtonElement.setCustomValidity()
> HTMLFieldSetElement.setCustomValidity()
> HTMLInputElement.setCustomValidity()
> HTMLKeygenElement.setCustomValidity()
> HTMLMediaElement.canPlayType()
> TimeRanges.start()
> TimeRanges.end()
> HTMLAllCollection.namedItem()
> HTMLAllCollection.tags()
> 
> https://bugs.webkit.org/show_bug.cgi?id=65353
> 
> DOMWindow.webkitRequestFileSystem()
> DOMWindow.webkitResolveLocalFileSystemURL()
> DOMWindow.webkitRequestAnimationFrame() (reverted in
> https://bugs.webkit.org/show_bug.cgi?id=65698 )
> DOMWindow.webkitCancelRequestAnimationFrame() (reverted in
> https://bugs.webkit.org/show_bug.cgi?id=65698 )
> 
> https://bugs.webkit.org/show_bug.cgi?id=65355
> 
> Geolocation.clearWatch()
> PositionCallback.handleEvent()
> PositionErrorCallback.handleEvent()
> 
> https://bugs.webkit.org/show_bug.cgi?id=65370
> 
> Navigator.registerProtocolHandler()
> 
> https://bugs.webkit.org/show_bug.cgi?id=65570
> 
> SpeechInputResultList.item()
> 
> https://bugs.webkit.org/show_bug.cgi?id=65571
> 
> WebKitAnimationList.item()
> 
> https://bugs.webkit.org/show_bug.cgi?id=65715
> 
> MediaStreamList.item()
> MediaStreamTrackList.item()
> TouchList.item()
> 
> https://bugs.webkit.org/show_bug.cgi?id=65750
> 
> AudioBufferSourceNode.noteOn()
> AudioBufferSourceNode.noteGrainOn()
> AudioBufferSourceNode.noteOff()
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Current legacy argument refactoring -- behavior changes?

[Adam Barth]

Hi Maciej,

On Fri, Aug 5, 2011 at 1:16 PM, Maciej Stachowiak  wrote:
> I have a hard time reading this wall of text, but it seems to me there were 
> at least three things wrong with what happened:

I didn't mean to write a wall of test.  I wanted to give Darin
complete answers to his questions because it seems like the main issue
here is under communication.

> 1) Patches mixing refactoring and behavior changes. This is bad. We should 
> aim whenever possible to separate behavior changes from refactoring. 
> Reviewers should point this out and ask for behavior change to be separated 
> with refactoring.

Yes, I agree that I should have asked Mark to separate all the
behavior-changing patches from the non-behavior changing parts.
Originally I had thought that proceeding by IDL file would have that
effect, but that wasn't the case for 100% of the IDL files because
some, like DOMWindow, include APIs from a number of different
features.

> 2) It seems like this change was not discussed enough for such a large 
> change. You say that "everyone was on board", but from the fact that people 
> are surprised and alarmed by this change, that was clearly not the case.

Yes, I misunderstood the extent to which folks were on board.  For
example, some important parts of the discussion occurred on
 in 2009 and 2010.  We
should have re-confirmed with more of the relevant folks more
recently.

> 3) Patches changed behavior without having test cases, and in some cases in 
> the process broke existing tests. This is a clear violation of project 
> policies should have been an automatic r-.

I agree that some of the patches should have included more tests.
Thankfully, that's easy to fix after the fact.

As for breaking existing tests, that's something we should do better
on both for these patches and in the project more generally.

> My proposal is to revert all the patches that changed behavior without 
> incorporating tests, and we can decide how to proceed from there. If it's 
> hard to even tell which those are, then we should just revert the whole patch 
> set. Then we can do this right.

I'm not sure that's necessary.  We can look over the list of changed
APIs just as easily without reverting the patches.  Are there any in
the list that Mark set out that you'd like to discuss further?

(As for adding the missing tests, we can also do that without
reverting and re-applying the IDL changes.)

Adam


> On Aug 5, 2011, at 12:05 PM, Adam Barth wrote:
>
>> On Fri, Aug 5, 2011 at 11:12 AM, Darin Adler  wrote:
>>> I assume that the legacy argument refactoring you are doing right now has a 
>>> goal of no behavior changes. But I’ve noticed some cases where arguments 
>>> are not marked optional in a few of the patches. Possibly I misunderstood 
>>> the patch.
>>
>> The project has proceeded in 4 phases:
>>
>> 1) Change the default behavior of the code generator and tag every IDL
>> file with LegacyDefaultOptionalArguments (no behavior change).
>> 2) Remove LegacyDefaultOptionalArguments from IDL files where it
>> doesn't affect the generated code (no behavior change).
>> 3) Remove LegacyDefaultOptionalArguments from "newer" APIs where we
>> wish to have the stricter behavior (behavior change, with tests).
>> 4) Remove LegacyDefaultOptionalArguments and tag each individual
>> optional argument with the [Optional=CallWithDefaultArgument]
>> attribute (no behavior change).
>>
>> Here's an example of a patch from phase (3):
>>
>> http://trac.webkit.org/changeset/89377
>>
>> My understanding was that everyone was on board with tightening up our
>> behavior for newer APIs.  We discussed this on some W3C mailing lists
>> and got Jonas to agree to loosen Firefox in some areas so that we can
>> converge in behavior.
>>
>> My view is that it's important for the long-term health of the web
>> that browser vendors converge on these sorts of behaviors, but it's
>> not overly important whether they converge to strictness or looseness.
>> We picked "strict" for the default to match WebIDL, which aligns with
>> out long-term goal of having WebKit IDL be as close to WebIDL as
>> possible.
>>
>>> One example was arguments in canvas rendering context functions.
>>
>> I believe we changed WebGL to be strict (i.e., matching Firefox) but
>> left 2D canvas loose (as there's a lof of WebKit-specific code that
>> uses canvas).  Is there a specific patch you have in mind?
>>
>> Here's are the recent phase (4) patches in the
>> Source/WebCore/html/canvas directory:
>>
>> http://trac.webkit.org/changeset/92443/trunk/Source/WebCore/html/canvas
>> http://trac.webkit.org/changeset/91617/trunk/Source/WebCore/html/canvas
>>
>> Neither of those are related to canvas rendering context functions,
>> and all of them should preserve existing behavior.
>>
>>> If you we want to make behavior changes, I think it would be done in a 
>>> patch without other refactoring. I’m sure we’ll want to change at least 
>>> some, but

Re: [webkit-dev] Current legacy argument refactoring -- behavior changes?

[Adam Barth]

On Fri, Aug 5, 2011 at 1:06 PM, Darin Adler  wrote:
> Mark, Adam, thanks for your replies.
>
> It seems obviously OK to do this for new APIs that have not been around long, 
> especially ones that never shipped in a production browser, for the same 
> reason that we decided to do this for new functions from this point forward. 
> I have no quibble with that.
>
> The way we checked in these changes makes it a challenge to figure out what 
> changed. The list you mailed is easier to understand than the changes 
> themselves, but it’s a pretty long list. Normally we’d want to triage, 
> quickly land completely non-controversial changes, and discuss the more 
> interesting cases at greater length.
>
> Best practices for WebKit would be to make sure there are patches where we 
> can see easily the functions affected in each patch. We could accomplish this 
> by first checking in with the [Optional] in a refactoring patch, then 
> removing [Optional] in a behavior change patch along with test cases and, as 
> appropriate, a bit of further discussion discussion.
>
> The reason these are best practices is that they help others involved in the 
> project understand the changes and even participate in decision making. The 
> way we’ve been doing it up until now makes it hard for anyone to follow 
> along. The reason I have reviewed so few of these patches is that it’s very 
> hard for me to figure out which ones include behavior changes.

Yeah, we should have more clearly separated phase (3) and phase (4).
Ideally, we should have sent out a plan to webkit-dev and then used
meta bugs to separate out the different phases so that folks could CC
themselves on the bugs related to whichever phase they were most
interested in.

For the major features that moved to the newer behavior, we discussed
each personally with at one of the folks who is a major contributor to
the feature.  Not all those discussions are recorded in the bug
tracker because some of them happened in person or on #webkit.

> What about my test coverage question?
>
> Normally we require tests when changing WebKit behavior. Is there a reason 
> these changes are an exception? It seems that in most of these cases creating 
> a test would be straightforward and it would be useful for the project to 
> have such tests.

I've filed https://bugs.webkit.org/show_bug.cgi?id=65787 about adding
the missing tests.

Thanks,
Adam
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Current legacy argument refactoring -- behavior changes?

[Maciej Stachowiak]

Hi Adam,

I have a hard time reading this wall of text, but it seems to me there were at 
least three things wrong with what happened:

1) Patches mixing refactoring and behavior changes. This is bad. We should aim 
whenever possible to separate behavior changes from refactoring. Reviewers 
should point this out and ask for behavior change to be separated with 
refactoring.

2) It seems like this change was not discussed enough for such a large change. 
You say that "everyone was on board", but from the fact that people are 
surprised and alarmed by this change, that was clearly not the case.

3) Patches changed behavior without having test cases, and in some cases in the 
process broke existing tests. This is a clear violation of project policies 
should have been an automatic r-.


My proposal is to revert all the patches that changed behavior without 
incorporating tests, and we can decide how to proceed from there. If it's hard 
to even tell which those are, then we should just revert the whole patch set. 
Then we can do this right.


Regards,
Maciej



On Aug 5, 2011, at 12:05 PM, Adam Barth wrote:

> On Fri, Aug 5, 2011 at 11:12 AM, Darin Adler  wrote:
>> I assume that the legacy argument refactoring you are doing right now has a 
>> goal of no behavior changes. But I’ve noticed some cases where arguments are 
>> not marked optional in a few of the patches. Possibly I misunderstood the 
>> patch.
> 
> The project has proceeded in 4 phases:
> 
> 1) Change the default behavior of the code generator and tag every IDL
> file with LegacyDefaultOptionalArguments (no behavior change).
> 2) Remove LegacyDefaultOptionalArguments from IDL files where it
> doesn't affect the generated code (no behavior change).
> 3) Remove LegacyDefaultOptionalArguments from "newer" APIs where we
> wish to have the stricter behavior (behavior change, with tests).
> 4) Remove LegacyDefaultOptionalArguments and tag each individual
> optional argument with the [Optional=CallWithDefaultArgument]
> attribute (no behavior change).
> 
> Here's an example of a patch from phase (3):
> 
> http://trac.webkit.org/changeset/89377
> 
> My understanding was that everyone was on board with tightening up our
> behavior for newer APIs.  We discussed this on some W3C mailing lists
> and got Jonas to agree to loosen Firefox in some areas so that we can
> converge in behavior.
> 
> My view is that it's important for the long-term health of the web
> that browser vendors converge on these sorts of behaviors, but it's
> not overly important whether they converge to strictness or looseness.
> We picked "strict" for the default to match WebIDL, which aligns with
> out long-term goal of having WebKit IDL be as close to WebIDL as
> possible.
> 
>> One example was arguments in canvas rendering context functions.
> 
> I believe we changed WebGL to be strict (i.e., matching Firefox) but
> left 2D canvas loose (as there's a lof of WebKit-specific code that
> uses canvas).  Is there a specific patch you have in mind?
> 
> Here's are the recent phase (4) patches in the
> Source/WebCore/html/canvas directory:
> 
> http://trac.webkit.org/changeset/92443/trunk/Source/WebCore/html/canvas
> http://trac.webkit.org/changeset/91617/trunk/Source/WebCore/html/canvas
> 
> Neither of those are related to canvas rendering context functions,
> and all of them should preserve existing behavior.
> 
>> If you we want to make behavior changes, I think it would be done in a patch 
>> without other refactoring. I’m sure we’ll want to change at least some, but 
>> not mixed in with the refactoring under “accidental cover of darkness”.
> 
> There has been some patches that combined phase (3) and phase (4)
> changes in a single patch.  For example, there are some IDL files that
> include both old APIs (where we want to preserve behavior) and new
> APIs (e.g., FileSystem, where we want the strict behavior).  We've
> been proceeding by IDL files, but it's certainly possible I should
> have asked Mark to separate those changes into separate patches.  I
> probably also should have been more pedantic about asking Mark to
> write more tests as we go through these.
> 
>> I have two questions:
>> - Did any of the recent refactoring patches change behavior?
> 
> Yes.  For example,
> http://trac.webkit.org/changeset/92313/trunk/Source/WebCore/page/DOMWindow.idl
> mixes some phase (3) and phase (4) changes.  The vast majority of the
> APIs were left unchanged, but we did make the following APIs stricter:
> 
> 1) webkitRequestAnimationFrame
> 2) webkitCancelRequestAnimationFrame
> 3) webkitRequestFileSystem
> 4) webkitResolveLocalFileSystemURL
> 
> My reasoning in asking Mark to do make these changes is that these
> APIs are relatively new (therefore the compat risk of making them
> match other browsers is low), and they're vendor-prefixed (which puts
> developers on notice that we might update them as the spec evolves).
> 
> That change did break something.  It broke some callers of
> webkitR

Re: [webkit-dev] Current legacy argument refactoring -- behavior changes?

[Darin Adler]

Mark, Adam, thanks for your replies.

It seems obviously OK to do this for new APIs that have not been around long, 
especially ones that never shipped in a production browser, for the same reason 
that we decided to do this for new functions from this point forward. I have no 
quibble with that.

The way we checked in these changes makes it a challenge to figure out what 
changed. The list you mailed is easier to understand than the changes 
themselves, but it’s a pretty long list. Normally we’d want to triage, quickly 
land completely non-controversial changes, and discuss the more interesting 
cases at greater length.

Best practices for WebKit would be to make sure there are patches where we can 
see easily the functions affected in each patch. We could accomplish this by 
first checking in with the [Optional] in a refactoring patch, then removing 
[Optional] in a behavior change patch along with test cases and, as 
appropriate, a bit of further discussion discussion.

The reason these are best practices is that they help others involved in the 
project understand the changes and even participate in decision making. The way 
we’ve been doing it up until now makes it hard for anyone to follow along. The 
reason I have reviewed so few of these patches is that it’s very hard for me to 
figure out which ones include behavior changes.

What about my test coverage question?

Normally we require tests when changing WebKit behavior. Is there a reason 
these changes are an exception? It seems that in most of these cases creating a 
test would be straightforward and it would be useful for the project to have 
such tests.

-- Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] DOMCrypt

[Michael Nordman]

I think bytes and blobs would be sufficient.

+f...@google.com

On Fri, Aug 5, 2011 at 12:58 PM, Adam Barth  wrote:

> Bytes and (likely) blobs are types we're planning to do in DOMCrypt.
> Hashing strings is slightly more delicate because you need to pick an
> encoding.  Do you have a sense, if we did bytes and blobs, would that
> be enough, or are strings really important also?
>
> Thanks,
> Adam
>
>
> On Fri, Aug 5, 2011 at 12:43 PM, Michael Nordman 
> wrote:
> > Yes, hashing blobs. Here's the last line of the relevant meeting notes...
> > "In the end, we all agreed that the main thing with the highest utility
> > would be a native hashing implementation that could accept strings,
> bytes,
> > or BLOBs."
> >
> > On Fri, Aug 5, 2011 at 12:32 PM, Adam Barth  wrote:
> >>
> >> On Fri, Aug 5, 2011 at 12:09 PM, Michael Nordman 
> >> wrote:
> >> >> For example, the CryptoHash
> >> >> interface can be implemented independently of the rest of the API and
> >> >> provides value by itself.
> >> >
> >> > Moving forward on that part first sounds reasonable. I've been asked
> >> > about
> >> > that specifically by some app developers that really aren't interested
> >> > in
> >> > the other parts of the larger proposal.
> >>
> >> Are they specifically interested in hashing blobs?  David and I have
> >> been discussing what sort of types these functions should handle.
> >>
> >> Adam
> >>
> >>
> >> > On Wed, Jul 27, 2011 at 10:06 AM, Sam Weinig 
> wrote:
> >> >>
> >> >> I think we should let the spec mature a bit before diving in.
> >> >>
> >> >> -Sam
> >> >>
> >> >> On Jul 26, 2011, at 10:53 PM, Adam Barth wrote:
> >> >>
> >> >> > Hi webkit-dev,
> >> >> >
> >> >> > As some of you are probably aware, Mozilla is experimenting with
> >> >> > exposing some basic cryptographic primitives to web applications:
> >> >> >
> >> >> > https://wiki.mozilla.org/Privacy/Features/DOMCryptAPISpec/Latest
> >> >> >
> >> >> > I wanted to get a sense from the WebKit community about how
> >> >> > interested
> >> >> > we are in implementing this feature.  My sense is that this API is
> >> >> > fairly early in it's lifecycle, so one perspective is that we
> should
> >> >> > wait for Mozilla to experiment for a bit longer and revisit this
> >> >> > question once the design is further along (e.g., submitted to the
> W3C
> >> >> > standards process).
> >> >> >
> >> >> > Another perspective is that there are some simple parts of the API
> >> >> > that we should implement now, and we can grow into the more
> involved
> >> >> > parts of the API as they mature.  For example, the CryptoHash
> >> >> > interface can be implemented independently of the rest of the API
> and
> >> >> > provides value by itself.
> >> >> >
> >> >> > Thoughts?
> >> >> >
> >> >> > Adam
> >> >> > ___
> >> >> > webkit-dev mailing list
> >> >> > webkit-dev@lists.webkit.org
> >> >> > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
> >> >>
> >> >> ___
> >> >> webkit-dev mailing list
> >> >> webkit-dev@lists.webkit.org
> >> >> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
> >> >
> >> >
> >
> >
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] DOMCrypt

[Adam Barth]

Bytes and (likely) blobs are types we're planning to do in DOMCrypt.
Hashing strings is slightly more delicate because you need to pick an
encoding.  Do you have a sense, if we did bytes and blobs, would that
be enough, or are strings really important also?

Thanks,
Adam


On Fri, Aug 5, 2011 at 12:43 PM, Michael Nordman  wrote:
> Yes, hashing blobs. Here's the last line of the relevant meeting notes...
> "In the end, we all agreed that the main thing with the highest utility
> would be a native hashing implementation that could accept strings, bytes,
> or BLOBs."
>
> On Fri, Aug 5, 2011 at 12:32 PM, Adam Barth  wrote:
>>
>> On Fri, Aug 5, 2011 at 12:09 PM, Michael Nordman 
>> wrote:
>> >> For example, the CryptoHash
>> >> interface can be implemented independently of the rest of the API and
>> >> provides value by itself.
>> >
>> > Moving forward on that part first sounds reasonable. I've been asked
>> > about
>> > that specifically by some app developers that really aren't interested
>> > in
>> > the other parts of the larger proposal.
>>
>> Are they specifically interested in hashing blobs?  David and I have
>> been discussing what sort of types these functions should handle.
>>
>> Adam
>>
>>
>> > On Wed, Jul 27, 2011 at 10:06 AM, Sam Weinig  wrote:
>> >>
>> >> I think we should let the spec mature a bit before diving in.
>> >>
>> >> -Sam
>> >>
>> >> On Jul 26, 2011, at 10:53 PM, Adam Barth wrote:
>> >>
>> >> > Hi webkit-dev,
>> >> >
>> >> > As some of you are probably aware, Mozilla is experimenting with
>> >> > exposing some basic cryptographic primitives to web applications:
>> >> >
>> >> > https://wiki.mozilla.org/Privacy/Features/DOMCryptAPISpec/Latest
>> >> >
>> >> > I wanted to get a sense from the WebKit community about how
>> >> > interested
>> >> > we are in implementing this feature.  My sense is that this API is
>> >> > fairly early in it's lifecycle, so one perspective is that we should
>> >> > wait for Mozilla to experiment for a bit longer and revisit this
>> >> > question once the design is further along (e.g., submitted to the W3C
>> >> > standards process).
>> >> >
>> >> > Another perspective is that there are some simple parts of the API
>> >> > that we should implement now, and we can grow into the more involved
>> >> > parts of the API as they mature.  For example, the CryptoHash
>> >> > interface can be implemented independently of the rest of the API and
>> >> > provides value by itself.
>> >> >
>> >> > Thoughts?
>> >> >
>> >> > Adam
>> >> > ___
>> >> > webkit-dev mailing list
>> >> > webkit-dev@lists.webkit.org
>> >> > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>> >>
>> >> ___
>> >> webkit-dev mailing list
>> >> webkit-dev@lists.webkit.org
>> >> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>> >
>> >
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Current legacy argument refactoring -- behavior changes?

[Mark Pilgrim]

Here is the complete list of intentional behavior changes in patches
I've submitted in the past few months, along with bug URLs to show the
discussion / rationale behind each change:


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

IDBFactory.open()

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

IDBIndex.get()
IDBIndex.getKey()

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

IDBKeyRange.only()
IDBKeyRange.lowerBound()
IDBKeyRange.upperBound()
IDBKeyRange.bound()

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

IDBObjectStore.put()
IDBObjectStore.add()
IDBObjectStore.delete()
IDBObjectStore.get()
IDBObjectStore.createIndex()
IDBObjectStore.index()
IDBObjectStore.deleteIndex()

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

IDBDatabase.createObjectStore()
IDBDatabase.deleteObjectStore()
IDBDatabase.setVersion()
IDBDatabase.transaction()

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

all arguments in File API-related methods not already marked [Optional]

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

all arguments in WebGL-related methods not already marked [Optional]

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

DOMTokenList.item()
DOMTokenList.contains()
DOMTokenList.add()
DOMTokenList.remove()
DOMTokenList.toggle()
DOMURL.createObjectURL()
DOMURL.revokeObjectURL()
HTMLAnchorElement.getParameter()
HTMLButtonElement.setCustomValidity()
HTMLFieldSetElement.setCustomValidity()
HTMLInputElement.setCustomValidity()
HTMLKeygenElement.setCustomValidity()
HTMLMediaElement.canPlayType()
TimeRanges.start()
TimeRanges.end()
HTMLAllCollection.namedItem()
HTMLAllCollection.tags()

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

DOMWindow.webkitRequestFileSystem()
DOMWindow.webkitResolveLocalFileSystemURL()
DOMWindow.webkitRequestAnimationFrame() (reverted in
https://bugs.webkit.org/show_bug.cgi?id=65698 )
DOMWindow.webkitCancelRequestAnimationFrame() (reverted in
https://bugs.webkit.org/show_bug.cgi?id=65698 )

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

Geolocation.clearWatch()
PositionCallback.handleEvent()
PositionErrorCallback.handleEvent()

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

Navigator.registerProtocolHandler()

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

SpeechInputResultList.item()

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

WebKitAnimationList.item()

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

MediaStreamList.item()
MediaStreamTrackList.item()
TouchList.item()

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

AudioBufferSourceNode.noteOn()
AudioBufferSourceNode.noteGrainOn()
AudioBufferSourceNode.noteOff()
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] DOMCrypt

[Michael Nordman]

Yes, hashing blobs. Here's the last line of the relevant meeting notes...

"In the end, we all agreed that the main thing with the highest utility
would be a native hashing implementation that could accept strings, bytes,
or BLOBs."

On Fri, Aug 5, 2011 at 12:32 PM, Adam Barth  wrote:

> On Fri, Aug 5, 2011 at 12:09 PM, Michael Nordman 
> wrote:
> >> For example, the CryptoHash
> >> interface can be implemented independently of the rest of the API and
> >> provides value by itself.
> >
> > Moving forward on that part first sounds reasonable. I've been asked
> about
> > that specifically by some app developers that really aren't interested in
> > the other parts of the larger proposal.
>
> Are they specifically interested in hashing blobs?  David and I have
> been discussing what sort of types these functions should handle.
>
> Adam
>
>
> > On Wed, Jul 27, 2011 at 10:06 AM, Sam Weinig  wrote:
> >>
> >> I think we should let the spec mature a bit before diving in.
> >>
> >> -Sam
> >>
> >> On Jul 26, 2011, at 10:53 PM, Adam Barth wrote:
> >>
> >> > Hi webkit-dev,
> >> >
> >> > As some of you are probably aware, Mozilla is experimenting with
> >> > exposing some basic cryptographic primitives to web applications:
> >> >
> >> > https://wiki.mozilla.org/Privacy/Features/DOMCryptAPISpec/Latest
> >> >
> >> > I wanted to get a sense from the WebKit community about how interested
> >> > we are in implementing this feature.  My sense is that this API is
> >> > fairly early in it's lifecycle, so one perspective is that we should
> >> > wait for Mozilla to experiment for a bit longer and revisit this
> >> > question once the design is further along (e.g., submitted to the W3C
> >> > standards process).
> >> >
> >> > Another perspective is that there are some simple parts of the API
> >> > that we should implement now, and we can grow into the more involved
> >> > parts of the API as they mature.  For example, the CryptoHash
> >> > interface can be implemented independently of the rest of the API and
> >> > provides value by itself.
> >> >
> >> > Thoughts?
> >> >
> >> > Adam
> >> > ___
> >> > webkit-dev mailing list
> >> > webkit-dev@lists.webkit.org
> >> > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
> >>
> >> ___
> >> webkit-dev mailing list
> >> webkit-dev@lists.webkit.org
> >> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
> >
> >
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] DOMCrypt

[Adam Barth]

On Fri, Aug 5, 2011 at 12:09 PM, Michael Nordman  wrote:
>> For example, the CryptoHash
>> interface can be implemented independently of the rest of the API and
>> provides value by itself.
>
> Moving forward on that part first sounds reasonable. I've been asked about
> that specifically by some app developers that really aren't interested in
> the other parts of the larger proposal.

Are they specifically interested in hashing blobs?  David and I have
been discussing what sort of types these functions should handle.

Adam


> On Wed, Jul 27, 2011 at 10:06 AM, Sam Weinig  wrote:
>>
>> I think we should let the spec mature a bit before diving in.
>>
>> -Sam
>>
>> On Jul 26, 2011, at 10:53 PM, Adam Barth wrote:
>>
>> > Hi webkit-dev,
>> >
>> > As some of you are probably aware, Mozilla is experimenting with
>> > exposing some basic cryptographic primitives to web applications:
>> >
>> > https://wiki.mozilla.org/Privacy/Features/DOMCryptAPISpec/Latest
>> >
>> > I wanted to get a sense from the WebKit community about how interested
>> > we are in implementing this feature.  My sense is that this API is
>> > fairly early in it's lifecycle, so one perspective is that we should
>> > wait for Mozilla to experiment for a bit longer and revisit this
>> > question once the design is further along (e.g., submitted to the W3C
>> > standards process).
>> >
>> > Another perspective is that there are some simple parts of the API
>> > that we should implement now, and we can grow into the more involved
>> > parts of the API as they mature.  For example, the CryptoHash
>> > interface can be implemented independently of the rest of the API and
>> > provides value by itself.
>> >
>> > Thoughts?
>> >
>> > Adam
>> > ___
>> > webkit-dev mailing list
>> > webkit-dev@lists.webkit.org
>> > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] DOMCrypt

[Michael Nordman]

> For example, the CryptoHash
> interface can be implemented independently of the rest of the API and
> provides value by itself.

Moving forward on that part first sounds reasonable. I've been asked about
that specifically by some app developers that really aren't interested in
the other parts of the larger proposal.

On Wed, Jul 27, 2011 at 10:06 AM, Sam Weinig  wrote:

> I think we should let the spec mature a bit before diving in.
>
> -Sam
>
> On Jul 26, 2011, at 10:53 PM, Adam Barth wrote:
>
> > Hi webkit-dev,
> >
> > As some of you are probably aware, Mozilla is experimenting with
> > exposing some basic cryptographic primitives to web applications:
> >
> > https://wiki.mozilla.org/Privacy/Features/DOMCryptAPISpec/Latest
> >
> > I wanted to get a sense from the WebKit community about how interested
> > we are in implementing this feature.  My sense is that this API is
> > fairly early in it's lifecycle, so one perspective is that we should
> > wait for Mozilla to experiment for a bit longer and revisit this
> > question once the design is further along (e.g., submitted to the W3C
> > standards process).
> >
> > Another perspective is that there are some simple parts of the API
> > that we should implement now, and we can grow into the more involved
> > parts of the API as they mature.  For example, the CryptoHash
> > interface can be implemented independently of the rest of the API and
> > provides value by itself.
> >
> > Thoughts?
> >
> > Adam
> > ___
> > webkit-dev mailing list
> > webkit-dev@lists.webkit.org
> > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Current legacy argument refactoring -- behavior changes?

[Adam Barth]

On Fri, Aug 5, 2011 at 11:12 AM, Darin Adler  wrote:
> I assume that the legacy argument refactoring you are doing right now has a 
> goal of no behavior changes. But I’ve noticed some cases where arguments are 
> not marked optional in a few of the patches. Possibly I misunderstood the 
> patch.

The project has proceeded in 4 phases:

1) Change the default behavior of the code generator and tag every IDL
file with LegacyDefaultOptionalArguments (no behavior change).
2) Remove LegacyDefaultOptionalArguments from IDL files where it
doesn't affect the generated code (no behavior change).
3) Remove LegacyDefaultOptionalArguments from "newer" APIs where we
wish to have the stricter behavior (behavior change, with tests).
4) Remove LegacyDefaultOptionalArguments and tag each individual
optional argument with the [Optional=CallWithDefaultArgument]
attribute (no behavior change).

Here's an example of a patch from phase (3):

http://trac.webkit.org/changeset/89377

My understanding was that everyone was on board with tightening up our
behavior for newer APIs.  We discussed this on some W3C mailing lists
and got Jonas to agree to loosen Firefox in some areas so that we can
converge in behavior.

My view is that it's important for the long-term health of the web
that browser vendors converge on these sorts of behaviors, but it's
not overly important whether they converge to strictness or looseness.
 We picked "strict" for the default to match WebIDL, which aligns with
out long-term goal of having WebKit IDL be as close to WebIDL as
possible.

> One example was arguments in canvas rendering context functions.

I believe we changed WebGL to be strict (i.e., matching Firefox) but
left 2D canvas loose (as there's a lof of WebKit-specific code that
uses canvas).  Is there a specific patch you have in mind?

Here's are the recent phase (4) patches in the
Source/WebCore/html/canvas directory:

http://trac.webkit.org/changeset/92443/trunk/Source/WebCore/html/canvas
http://trac.webkit.org/changeset/91617/trunk/Source/WebCore/html/canvas

Neither of those are related to canvas rendering context functions,
and all of them should preserve existing behavior.

> If you we want to make behavior changes, I think it would be done in a patch 
> without other refactoring. I’m sure we’ll want to change at least some, but 
> not mixed in with the refactoring under “accidental cover of darkness”.

There has been some patches that combined phase (3) and phase (4)
changes in a single patch.  For example, there are some IDL files that
include both old APIs (where we want to preserve behavior) and new
APIs (e.g., FileSystem, where we want the strict behavior).  We've
been proceeding by IDL files, but it's certainly possible I should
have asked Mark to separate those changes into separate patches.  I
probably also should have been more pedantic about asking Mark to
write more tests as we go through these.

> I have two questions:
> - Did any of the recent refactoring patches change behavior?

Yes.  For example,
http://trac.webkit.org/changeset/92313/trunk/Source/WebCore/page/DOMWindow.idl
mixes some phase (3) and phase (4) changes.  The vast majority of the
APIs were left unchanged, but we did make the following APIs stricter:

1) webkitRequestAnimationFrame
2) webkitCancelRequestAnimationFrame
3) webkitRequestFileSystem
4) webkitResolveLocalFileSystemURL

My reasoning in asking Mark to do make these changes is that these
APIs are relatively new (therefore the compat risk of making them
match other browsers is low), and they're vendor-prefixed (which puts
developers on notice that we might update them as the spec evolves).

That change did break something.  It broke some callers of
webkitRequestAnimationFrame.  That issue was fixed in
, which also didn't contain
tests.  This time I did ask for tests
, which will happen
in .

> - Do you know if have test coverage for the optional arguments?

We've added more test coverage in this area, but I doubt our coverage
is 100%, even in the places where we've changed behavior.  I agree
that we probably should have added more tests for the phase (3)
stragglers that overlapped with phase (4).  That's my fault, as a
reviewer, for not asking for these tests.

Adam
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] Current legacy argument refactoring -- behavior changes?

[Darin Adler]

Hi Mark.

I assume that the legacy argument refactoring you are doing right now has a 
goal of no behavior changes. But I’ve noticed some cases where arguments are 
not marked optional in a few of the patches. Possibly I misunderstood the 
patch. One example was arguments in canvas rendering context functions.

If you we want to make behavior changes, I think it would be done in a patch 
without other refactoring. I’m sure we’ll want to change at least some, but not 
mixed in with the refactoring under “accidental cover of darkness”.

I have two questions:
- Did any of the recent refactoring patches change behavior?
- Do you know if have test coverage for the optional arguments?

-- Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] More feature flags for input types (Re: New Feature Flag Proposal: INPUT_COLOR)

[Darin Adler]

On Aug 4, 2011, at 6:40 PM, TAMURA, Kent wrote:

> Like INPUT_COLOR, I'd like to introduce feature flags for date, datetime, 
> datetime-local, month, time, and week types.
> * We're going to implement dedicated UIs for them, and will want to disable 
> it temporarily.
> * Their current implementations are insufficient and one might not want to 
> ship them in browsers, but they were already shipped in some browsers.

These are both good reasons to have a feature flag and I think it’s critical we 
do that. But is there any grouping we can do? Does each need a separate feature 
flag?

-- Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev