Bug 107680 – [EFL][WK2] Use C API inside ewk_favicon_database

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Chris Dumez]

As per Bug 107657, we should start using the C API internally to avoid violating layering.

[Chris Dumez]

Created attachment 184216 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 184216 [details]
Patch

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

> Source/WebKit2/UIProcess/API/C/WKIconDatabase.cpp:77
> +    toImpl(iconDatabaseRef)->synchronousIconURLForPageURL(toWTFString(pageURL), iconURL);

I don't know if this should be exposed via an API.

[Brady Eidson]

Comment on attachment 184216 [details]
Patch

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

>> Source/WebKit2/UIProcess/API/C/WKIconDatabase.cpp:77
>> +    toImpl(iconDatabaseRef)->synchronousIconURLForPageURL(toWTFString(pageURL), iconURL);
> 
> I don't know if this should be exposed via an API.

It should not be.  Can you explain more why you need it?

> Source/WebKit2/UIProcess/API/C/WKIconDatabase.cpp:85
> +bool WKIconDatabaseIsURLImportCompleted(WKIconDatabaseRef iconDatabaseRef)
> +{
> +    return toImpl(iconDatabaseRef)->isUrlImportCompleted();
> +}

The "url import" is an *extremely* fragile implementation detail of the icon database and has no business being exposed as API.

Could you share details why you think you need it?

[Brady Eidson]

(In reply to comment #0)
> As per Bug 107657, we should start using the C API internally to avoid violating layering.

The solution to solving layering problems is not to expose more internals as API.

[Chris Dumez]

Created attachment 184451 [details]
Patch

Do not add C API anymore. Further refactoring will be done in a later patch to stop relying on the few C++ methods that we currently use and have no C API alternative.

[Brady Eidson]

Comment on attachment 184451 [details]
Patch

Looking briefly, I definitely have no reason to r- this patch.

Since it's entirely contained within EFL, I'll let an EFL reviewer take a look.

[Kenneth Rohde Christiansen]

Comment on attachment 184451 [details]
Patch

OK, I am OK with this.

[WebKit Review Bot]

Comment on attachment 184451 [details]
Patch

Rejecting attachment 184451 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-04', 'apply-attachment', '--no-update', '--non-interactive', 184451, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
-queue

Parsed 4 diffs from patch file(s).
patch: **** Can't create file /tmp/pptQe127 : No space left on device
patch: **** Can't create file /tmp/ppIK0MM8 : No space left on device
patch: **** Can't create file /tmp/ppTdTd08 : No space left on device
patch: **** Can't create file /tmp/pp8tZQ8b : No space left on device

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Kenneth Rohde Christiansen']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16073864

[WebKit Review Bot]

Comment on attachment 184451 [details]
Patch

Rejecting attachment 184451 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-04', 'apply-attachment', '--no-update', '--non-interactive', 184451, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
-queue

Parsed 4 diffs from patch file(s).
patch: **** Can't create file /tmp/ppMW9zDm : No space left on device
patch: **** Can't create file /tmp/ppdP3Ahm : No space left on device
patch: **** Can't create file /tmp/ppcKpp5m : No space left on device
patch: **** Can't create file /tmp/ppPJ0nln : No space left on device

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Kenneth Rohde Christiansen']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16078798

[Chris Dumez]

Committed r140692: <http://trac.webkit.org/changeset/140692>