Bug 101477 – [EFL][WK2] Make ewk_context use WebContext instead of WKContext

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Jinwoo Song]

Currently ewk_context is using WKContextRef but it would be better to use the WebContext as we use C++ classes.

[Jinwoo Song]

Created attachment 172812 [details]
Patch

[Jinwoo Song]

Created attachment 172824 [details]
Patch

[Chris Dumez]

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.

[Jinwoo Song]

Created attachment 172842 [details]
Patch

[Jinwoo Song]

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.

[Gyuyoung Kim]

Comment on attachment 172842 [details]
Patch

LGTM.

[WebKit Review Bot]

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

[Jinwoo Song]

Created attachment 172913 [details]
Patch

[WebKit Review Bot]

Comment on attachment 172913 [details]
Patch

Clearing flags on attachment: 172913

Committed r133844: <http://trac.webkit.org/changeset/133844>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Mikhail Pozdnyakov]

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