Created attachment 131272 [details] Patch

Comment on attachment 131272 [details] Patch Attachment 131272 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11936673

Comment on attachment 131272 [details] Patch Attachment 131272 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/11940684

Comment on attachment 131272 [details] Patch I found this renaming is wrong. As you can see in IDBObjectStore.h and IDBKeyRange.h, WebCore is expecting that OptionsObject.h exists both in JSC and in V8. OptionsObject.h should be a common interface between JSC and V8. So the right fix would be - to implement bindings/js/OptionsObject.h so that it has the same interface as bindings/v8/OptionsObject.h. bindings/js/OptionsObject.h would be just a wrapper of JSDictionary. - to rename OptionsObject.{h,cpp} to Dictionary.{h,cpp}, for clarification. The reason why IDBObjectStore.h and IDBKeyRange.h are not currently causing any build errors is that IndexedDB is enabled in Chromium/V8 only.

(In reply to comment #4) > (From update of attachment 131272 [details]) > I found this renaming is wrong. As you can see in IDBObjectStore.h and IDBKeyRange.h, WebCore is expecting that OptionsObject.h exists both in JSC and in V8. OptionsObject.h should be a common interface between JSC and V8. So the right fix would be Can we instead call it "Dictionary.h"? OptionsObject is a bad name for a generic object like this one because it can be used for things other than options.

(In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 131272 [details] [details]) > > I found this renaming is wrong. As you can see in IDBObjectStore.h and IDBKeyRange.h, WebCore is expecting that OptionsObject.h exists both in JSC and in V8. OptionsObject.h should be a common interface between JSC and V8. So the right fix would be > > Can we instead call it "Dictionary.h"? OptionsObject is a bad name for a generic object like this one because it can be used for things other than options. FYI: https://bugs.webkit.org/show_bug.cgi?id=80207 adds an empty implementation of OptionsObject (patches in that bug are trying to get IDB to compile w/ JSC) +1 to a name evocative of WebIDL/Dictionary.

(In reply to comment #6) > > Can we instead call it "Dictionary.h"? OptionsObject is a bad name for a generic object like this one because it can be used for things other than options. > > FYI: https://bugs.webkit.org/show_bug.cgi?id=80207 adds an empty implementation of OptionsObject (patches in that bug are trying to get IDB to compile w/ JSC) OK, please land the patch for 80207 first, to avoid conflict.

Adding benjamin@ who is working on 80207

Created attachment 131552 [details] Patch

Attachment 131552 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:1421: Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] Total errors found: 1 in 32 files If any of these errors are false positives, please file a bug against check-webkit-style.

Ews-bots look fine. Committed manually to avoid style check errors.