As discussed in IRC, we are migrating our IDL files away from the interface-level "LegacyDefaultOptionalArguments" flag and onto argument-level [Optional] or [Optional=CallWithDefaultValue] flags. This patch migrates all remaining IndexedDB and WebSQLDatabase-related IDL files. It does not change any behavior, i.e. it does not make any arguments required that were previously optional, nor vice-versa. ONE EXCEPTION: it changes IDBTransaction.idl to make the objectStore() method require its single argument, as required by WebIDL and consistent with all the other changes we've made in the recent past to make IndexedDB as strict as WebIDL requires. All existing tests pass.
Created attachment 103032 [details] Patch
Comment on attachment 103032 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=103032&action=review > Source/WebCore/storage/StorageInfo.idl:41 > - [CallWith=ScriptExecutionContext] void queryUsageAndQuota(in unsigned short storageType, in [Callback, Optional] StorageInfoUsageCallback usageCallback, in [Callback, Optional] StorageInfoErrorCallback errorCallback); > - [CallWith=ScriptExecutionContext] void requestQuota(in unsigned short storageType, in unsigned long long newQuotaInBytes, in [Callback, Optional] StorageInfoQuotaCallback quotaCallback, in [Callback, Optional] StorageInfoErrorCallback errorCallback); > + [CallWith=ScriptExecutionContext] void queryUsageAndQuota(in [Optional=CallWithDefaultValue] unsigned short storageType, > + in [Callback, Optional] StorageInfoUsageCallback usageCallback, > + in [Callback, Optional] StorageInfoErrorCallback errorCallback); > + [CallWith=ScriptExecutionContext] void requestQuota(in [Optional=CallWithDefaultValue] unsigned short storageType, > + in [Optional=CallWithDefaultValue] unsigned long long newQuotaInBytes, > + in [Callback, Optional] StorageInfoQuotaCallback quotaCallback, > + in [Callback, Optional] StorageInfoErrorCallback errorCallback); QUOTA is very new. I wouldn't add the optional attributes.
Comment on attachment 103032 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=103032&action=review > Source/WebCore/storage/Storage.idl:42 > + [DontEnum, ConvertNullStringTo=Null] DOMString key(in [Optional=CallWithDefaultValue] unsigned long index); > + [DontEnum, ConvertNullStringTo=Null] DOMString getItem(in [Optional=CallWithDefaultValue] DOMString key); > + [DontEnum] void setItem(in [Optional=CallWithDefaultValue] DOMString key, > + in [Optional=CallWithDefaultValue] DOMString data) > raises(DOMException); > - [DontEnum] void removeItem(in DOMString key); > + [DontEnum] void removeItem(in [Optional=CallWithDefaultValue] DOMString key); I understand that this doesn't change the semantics but do we really want these to be optional?
> I understand that this doesn't change the semantics but do we really want these to be optional? Yeah, they make much more sense as non-optional. The only question is the compatibility risk. Do other browsers require these arguments?
Created attachment 113388 [details] Patch
(In reply to comment #4) > Do other browsers require these arguments? I checked and all these arguments are required in Firefox. I can test IE9 too if you want.
Comment on attachment 113388 [details] Patch Can we add those tests as LayoutTests?
IE9 allows skipping the arguments in Storage.idl even though it makes little sense in doing localStorage.setItem() I'm fine changing this to match IE9 but I feel like we are better of not allowing this. I will add some tests.
Created attachment 113391 [details] Patch
Comment on attachment 113391 [details] Patch What about StorageInfo.idl ? Would you be willing to add tests for those changes too. Note: Quota is very new, so there's not much risk of breaking stuff.
Committed r99130: <http://trac.webkit.org/changeset/99130>