Bug 43993

Created attachment 64380 [details] Patch

Comment on attachment 64380 [details] Patch WebCore/ChangeLog:14 + * WebCore.xcodeproj/project.pbxproj: you need to add these new files to a whole lot more build files (and possibly JSDerivedSources too): see http://trac.webkit.org/changeset/61531 WebCore/storage/DirectoryEntry.cpp:54 + // FIXME: to be implemented. might be worth adding ASSERT_NOT_REACHED() to all methods that are not yet implemented. WebCore/storage/DirectoryEntry.idl:38 + void getFile(in DOMString path, in [Optional] Flags options, in [Optional, Callback] EntryCallback successCallback, in [Optional, Callback] ErrorCallback errorCallback); will "[Optional] Flags options" generate "RefPtr<Flags> options;" or just "Flags* options"? r- mostly because of the changes required to other build files.

Created attachment 64526 [details] Patch

Thanks for reviewing. (In reply to comment #2) > (From update of attachment 64380 [details]) > WebCore/ChangeLog:14 > + * WebCore.xcodeproj/project.pbxproj: > you need to add these new files to a whole lot more build files (and possibly JSDerivedSources too): see http://trac.webkit.org/changeset/61531 Fixed. I also added entries for basic idls/classes added in changeset 64414. I was assuming we wouldn't need to include them on other platforms yet but on the second thought not having them on some platforms might be confusing. > WebCore/storage/DirectoryEntry.cpp:54 > + // FIXME: to be implemented. > might be worth adding ASSERT_NOT_REACHED() to all methods that are not yet implemented. Added. > WebCore/storage/DirectoryEntry.idl:38 > + void getFile(in DOMString path, in [Optional] Flags options, in [Optional, Callback] EntryCallback successCallback, in [Optional, Callback] ErrorCallback errorCallback); > will "[Optional] Flags options" generate "RefPtr<Flags> options;" or just "Flags* options"? Yes. > r- mostly because of the changes required to other build files.

(In reply to comment #4) > Thanks for reviewing. > > (In reply to comment #2) > > (From update of attachment 64380 [details] [details]) > > WebCore/ChangeLog:14 > > + * WebCore.xcodeproj/project.pbxproj: > > you need to add these new files to a whole lot more build files (and possibly JSDerivedSources too): see http://trac.webkit.org/changeset/61531 > > Fixed. I also added entries for basic idls/classes added in changeset 64414. > I was assuming we wouldn't need to include them on other platforms yet but on the second thought not having them on some platforms might be confusing. > > > WebCore/storage/DirectoryEntry.cpp:54 > > + // FIXME: to be implemented. > > might be worth adding ASSERT_NOT_REACHED() to all methods that are not yet implemented. > > Added. > > > WebCore/storage/DirectoryEntry.idl:38 > > + void getFile(in DOMString path, in [Optional] Flags options, in [Optional, Callback] EntryCallback successCallback, in [Optional, Callback] ErrorCallback errorCallback); > > will "[Optional] Flags options" generate "RefPtr<Flags> options;" or just "Flags* options"? > > Yes. Oops, I meant: it generates RefPtr<Flags> options (PassRefPtr for args). Do you mean creating ref-counted objects for such flags might be too much? Currently I don't have a good idea how we should treat those super lightweight (but not directly mapped to a native type) interfaces.

(In reply to comment #5) > Oops, I meant: it generates RefPtr<Flags> options (PassRefPtr for args). > Do you mean creating ref-counted objects for such flags might be too much? Currently I don't have a good idea how we should treat those super lightweight (but not directly mapped to a native type) interfaces. I saw your .cpp code takes PassRefPtr<Flags> parameters, and was worried that the auto-generated bindings could create raw pointers instead of RefPtr<Flags> and pass those in, but apparently that's not the case. Looking at the patch.

Comment on attachment 64526 [details] Patch r=me. please wait for the chromium EWS bot to turn green before submitting.

(In reply to comment #6) > (In reply to comment #5) > > Oops, I meant: it generates RefPtr<Flags> options (PassRefPtr for args). > > Do you mean creating ref-counted objects for such flags might be too much? Currently I don't have a good idea how we should treat those super lightweight (but not directly mapped to a native type) interfaces. > > I saw your .cpp code takes PassRefPtr<Flags> parameters, and was worried that the auto-generated bindings could create raw pointers instead of RefPtr<Flags> and pass those in, but apparently that's not the case. I see, thanks for taking care of it. Going to submit after checking the builds on bots.

Committed r65474: <http://trac.webkit.org/changeset/65474>

(In reply to comment #9) > Committed r65474: <http://trac.webkit.org/changeset/65474> Submitted except for WebCore/Android.derived.{jsc,v8}bindings.mk changes.