Bug 132176 - [EFL] Remove indexedDB's dependency on leveldb and disable indexedDB
Summary: [EFL] Remove indexedDB's dependency on leveldb and disable indexedDB
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-25 01:02 PDT by Joonghun Park
Modified: 2014-04-27 23:16 PDT (History)
8 users (show)

See Also:


Attachments
Patch (1.11 KB, patch)
2014-04-25 01:08 PDT, Joonghun Park
no flags Details | Formatted Diff | Diff
Patch (3.11 KB, patch)
2014-04-27 21:13 PDT, Joonghun Park
no flags Details | Formatted Diff | Diff
Patch (3.40 KB, patch)
2014-04-27 22:06 PDT, Joonghun Park
gyuyoung.kim: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>