Bug 132176

Summary: [EFL] Remove indexedDB's dependency on leveldb and disable indexedDB
Product: WebKit Reporter: Joonghun Park <jh718.park>
Component: WebKit EFLAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bunhere, commit-queue, dbates, gyuyoung.kim, lucas.de.marchi, rakuco, ryuan.choi, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch gyuyoung.kim: review+

Description Joonghun Park 2014-04-25 01:02:31 PDT
disable leveldb used for indexedDB in webkit1 of EFL port.
Comment 1 Joonghun Park 2014-04-25 01:08:43 PDT
Created attachment 230150 [details]
Patch
Comment 2 Ryuan Choi 2014-04-25 03:20:38 PDT
Comment on attachment 230150 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=230150&action=review

> Source/cmake/OptionsEfl.cmake:295
>  if (ENABLE_INDEXED_DATABASE)
> -    set(WTF_USE_LEVELDB 1)
> -    add_definitions(-DWTF_USE_LEVELDB=1)
> +    set(WTF_USE_LEVELDB 0)
> +    add_definitions(-DWTF_USE_LEVELDB=0)
>  endif ()

Why don't you just remove them.
Comment 3 Gyuyoung Kim 2014-04-25 03:54:42 PDT
Comment on attachment 230150 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=230150&action=review

> ChangeLog:9
> +        Disable leveldb used for indexedDB in WEBKIT1 of EFL port.

Useless description because title is same with this comment. Why do you want to disable it ?
Comment 4 Joonghun Park 2014-04-27 19:32:05 PDT
(In reply to comment #2)
> (From update of attachment 230150 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=230150&action=review
> 
> > Source/cmake/OptionsEfl.cmake:295
> >  if (ENABLE_INDEXED_DATABASE)
> > -    set(WTF_USE_LEVELDB 1)
> > -    add_definitions(-DWTF_USE_LEVELDB=1)
> > +    set(WTF_USE_LEVELDB 0)
> > +    add_definitions(-DWTF_USE_LEVELDB=0)
> >  endif ()
> 
> Why don't you just remove them.

In addition to remove the statements above, I would also disable indexedDB because in current status efl port webkit is not using levelDB nor SQlite.
Comment 5 Joonghun Park 2014-04-27 20:23:33 PDT
(In reply to comment #3)
> (From update of attachment 230150 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=230150&action=review
> 
> > ChangeLog:9
> > +        Disable leveldb used for indexedDB in WEBKIT1 of EFL port.
> 
> Useless description because title is same with this comment. Why do you want to disable it ?

I want to remove indexeddb's implemental dependency on leveldb and change indexeddb to use sqlite in WebKit1. It leads to the status in which EFL webkit1 and webkit2 has no implementation of indexeddb currently. So I turn off indexed database feature until we have sqlite implementation for it.
Comment 6 Joonghun Park 2014-04-27 21:13:10 PDT
Created attachment 230278 [details]
Patch
Comment 7 Gyuyoung Kim 2014-04-27 21:36:57 PDT
Comment on attachment 230278 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=230278&action=review

LGTM. Please land this patch after adding my suggestion.

> Tools/ChangeLog:7
> +

It would be good if you mention why EFL port remove indexedDB dependency in ChangeLog. For example,

"EFL port will use sqlite to support indexedDB. So, this patch removes levelDB dependency for now."
Comment 8 Joonghun Park 2014-04-27 22:06:46 PDT
Created attachment 230279 [details]
Patch
Comment 9 Gyuyoung Kim 2014-04-27 22:10:38 PDT
Comment on attachment 230279 [details]
Patch

You don't need to request review again if you already got r+ed.
Comment 10 Ryuan Choi 2014-04-27 23:16:55 PDT
Committed r167872: <http://trac.webkit.org/changeset/167872>