Bug 109455 - [WK2] Allow using the value of WebContext::platformDefaultIconDatabasePath from the C API
Summary: [WK2] Allow using the value of WebContext::platformDefaultIconDatabasePath fr...
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jocelyn Turcotte
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-11 09:27 PST by Jocelyn Turcotte
Modified: 2013-03-05 07:20 PST (History)
4 users (show)

See Also:


Attachments
Patch (7.99 KB, patch)
2013-02-11 09:43 PST, Jocelyn Turcotte
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jocelyn Turcotte 2013-02-11 09:27:54 PST
[WK2] Allow using the value of WebContext::platformDefaultIconDatabasePath from the C API
Comment 1 Jocelyn Turcotte 2013-02-11 09:43:36 PST
Created attachment 187601 [details]
Patch
Comment 2 Allan Sandfeld Jensen 2013-02-12 06:12:26 PST
What happens if you set an override path on an already open icon-database?
Comment 3 Jocelyn Turcotte 2013-02-12 06:18:23 PST
(In reply to comment #2)
> What happens if you set an override path on an already open icon-database?

It won't have any effect unless it's closed and re-opened. The same as when calling setLocalStorageDirectory, setCookieStorageDirectory, etc. after the web process was started.
Comment 4 Jocelyn Turcotte 2013-02-19 07:27:13 PST
Brady, could you please have a look at the patch? I think that this needs to go through you.
Comment 5 Brady Eidson 2013-03-01 11:03:36 PST
Comment on attachment 187601 [details]
Patch

I'm confused as to why this is important.

It's increasing the API surface to accomplish something that is already possible today - opening the icondatabase at the platform default path.
Comment 6 Jocelyn Turcotte 2013-03-01 11:21:24 PST
(In reply to comment #5)

If I'm not missing anything, it's only possible to open it from outside the current C API with:
- WebIconDatabase::setDatabasePath(const String& path)
- From WebContext::setIconDatabasePath(const String& path)
- Finally from WKContextSetIconDatabasePath(WKContextRef contextRef, WKStringRef iconDatabasePath)

This code path doesn't use the value returned by WebContext::platformDefaultIconDatabasePath() the same way it is possible with platformDefaultDatabaseDirectory(), platformDefaultLocalStorageDirectory() and platformDefaultCookieStorageDirectory().

Another way would be to set all of them through the C API client and not use the platformDefault* methods at all, if you think it would be cleaner.
Comment 7 Brady Eidson 2013-03-01 12:16:35 PST
(In reply to comment #6)
> (In reply to comment #5)
> 
> If I'm not missing anything, it's only possible to open it from outside the current C API with:
> - WebIconDatabase::setDatabasePath(const String& path)
> - From WebContext::setIconDatabasePath(const String& path)
> - Finally from WKContextSetIconDatabasePath(WKContextRef contextRef, WKStringRef iconDatabasePath)
> 
> This code path doesn't use the value returned by WebContext::platformDefaultIconDatabasePath() the same way it is possible with platformDefaultDatabaseDirectory(), platformDefaultLocalStorageDirectory() and platformDefaultCookieStorageDirectory().
> 
> Another way would be to set all of them through the C API client and not use the platformDefault* methods at all, if you think it would be cleaner.

WKContextSetIconDatabasePath is the call in the current C API that opens the icon database.

You can call it with the default path to open the database at the default path.

The notion of a "default path" was a mistake.  Clients should always have to explicitly provide a path.
Comment 8 Jocelyn Turcotte 2013-03-05 07:20:06 PST
Comment on attachment 187601 [details]
Patch

(In reply to comment #7)
> WKContextSetIconDatabasePath is the call in the current C API that opens the icon database.
> 
> You can call it with the default path to open the database at the default path.
> 
> The notion of a "default path" was a mistake.  Clients should always have to explicitly provide a path.

Alright, that makes sense.
I've gone that way with the patch for bug 111435, thanks for the input.