Bug 101477 - [EFL][WK2] Make ewk_context use WebContext instead of WKContext
Summary: [EFL][WK2] Make ewk_context use WebContext instead of WKContext
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: Jinwoo Song
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-07 08:10 PST by Jinwoo Song
Modified: 2012-11-08 01:55 PST (History)
6 users (show)

See Also:


Attachments
Patch (16.19 KB, patch)
2012-11-07 08:26 PST, Jinwoo Song
no flags Details | Formatted Diff | Diff
Patch (19.49 KB, patch)
2012-11-07 09:47 PST, Jinwoo Song
no flags Details | Formatted Diff | Diff
Patch (19.51 KB, patch)
2012-11-07 11:16 PST, Jinwoo Song
no flags Details | Formatted Diff | Diff
Patch (19.31 KB, patch)
2012-11-07 18:41 PST, Jinwoo Song
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jinwoo Song 2012-11-07 08:10:14 PST
Currently ewk_context is using WKContextRef but it would be better to use the WebContext as we use C++ classes.
Comment 1 Jinwoo Song 2012-11-07 08:26:56 PST
Created attachment 172812 [details]
Patch
Comment 2 Jinwoo Song 2012-11-07 09:47:22 PST
Created attachment 172824 [details]
Patch
Comment 3 Chris Dumez 2012-11-07 10:12:12 PST
Comment on attachment 172824 [details]
Patch

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

> Source/WebKit2/ChangeLog:3
> +        [EFL][WK2] Make ewk_context to use WebContext

"Make ewk_context use WebContext instead of WKContext" (without "to") would probably be more correct.

> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:165
> +    if (m_webContext.get()->iconDatabase()->isOpen())

.get() seems useless here.

> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:169
> +    String databasePath = databaseDirectory.isEmpty() ? m_webContext.get()->iconDatabasePath() : pathByAppendingComponent(databaseDirectory, WebCore::IconDatabase::defaultDatabaseFilename());

Ditto.

> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:170
> +    m_webContext.get()->setIconDatabasePath(databasePath);

Ditto.

> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:202
> +    m_webContext.get()->addVisitedLink(visitedURL);

Ditto.

> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:212
> +    return static_cast<Ewk_Cache_Model>(WKContextGetCacheModel(toAPI(m_webContext.get())));

I guess we could call m_webContext->cacheModel() instead of using C API.

> Source/WebKit2/UIProcess/API/efl/ewk_database_manager_private.h:42
> +        return adoptPtr(new Ewk_Database_Manager(context->databaseManagerProxy()));

Would be nice to add an ASSERT(context); before this line.

> Source/WebKit2/UIProcess/API/efl/ewk_storage_manager_private.h:42
> +        return adoptPtr(new Ewk_Storage_Manager(context->keyValueStorageManagerProxy()));

Ditto.
Comment 4 Jinwoo Song 2012-11-07 11:16:29 PST
Created attachment 172842 [details]
Patch
Comment 5 Jinwoo Song 2012-11-07 11:18:18 PST
Comment on attachment 172824 [details]
Patch

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

>> Source/WebKit2/ChangeLog:3
>> +        [EFL][WK2] Make ewk_context to use WebContext
> 
> "Make ewk_context use WebContext instead of WKContext" (without "to") would probably be more correct.

Changed title more correctly.

>> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:165
>> +    if (m_webContext.get()->iconDatabase()->isOpen())
> 
> .get() seems useless here.

Fixed.

>> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:169
>> +    String databasePath = databaseDirectory.isEmpty() ? m_webContext.get()->iconDatabasePath() : pathByAppendingComponent(databaseDirectory, WebCore::IconDatabase::defaultDatabaseFilename());
> 
> Ditto.

Fixed.

>> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:202
>> +    m_webContext.get()->addVisitedLink(visitedURL);
> 
> Ditto.

Fixed.

>> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:212
>> +    return static_cast<Ewk_Cache_Model>(WKContextGetCacheModel(toAPI(m_webContext.get())));
> 
> I guess we could call m_webContext->cacheModel() instead of using C API.

Fixed, thanks.

>> Source/WebKit2/UIProcess/API/efl/ewk_database_manager_private.h:42
>> +        return adoptPtr(new Ewk_Database_Manager(context->databaseManagerProxy()));
> 
> Would be nice to add an ASSERT(context); before this line.

Done.

>> Source/WebKit2/UIProcess/API/efl/ewk_storage_manager_private.h:42
>> +        return adoptPtr(new Ewk_Storage_Manager(context->keyValueStorageManagerProxy()));
> 
> Ditto.

Done.
Comment 6 Gyuyoung Kim 2012-11-07 17:46:21 PST
Comment on attachment 172842 [details]
Patch

LGTM.
Comment 7 WebKit Review Bot 2012-11-07 18:06:05 PST
Comment on attachment 172842 [details]
Patch

Rejecting attachment 172842 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
ile Source/WebKit2/UIProcess/API/efl/ewk_storage_manager.cpp
patching file Source/WebKit2/UIProcess/API/efl/ewk_storage_manager_private.h
patching file Source/WebKit2/UIProcess/API/efl/ewk_view.cpp
patching file Source/WebKit2/UIProcess/efl/DownloadManagerEfl.cpp
patching file Source/WebKit2/UIProcess/efl/RequestManagerClientEfl.cpp

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Gyuyoung K..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/14764327
Comment 8 Jinwoo Song 2012-11-07 18:41:21 PST
Created attachment 172913 [details]
Patch
Comment 9 WebKit Review Bot 2012-11-07 20:38:32 PST
Comment on attachment 172913 [details]
Patch

Clearing flags on attachment: 172913

Committed r133844: <http://trac.webkit.org/changeset/133844>
Comment 10 WebKit Review Bot 2012-11-07 20:38:36 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Mikhail Pozdnyakov 2012-11-08 01:55:04 PST
Comment on attachment 172913 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/ewk_context_private.h:50
> +    static PassRefPtr<EwkContext> create(WebContext* context);

PassRefPtr<WebContext> shall be used here and below, filing a new issue for that, since this patch is landed.

> Source/WebKit2/UIProcess/API/efl/ewk_context_private.h:86
> +    RefPtr<WebContext> m_webContext;

think it should be still m_context