Bug 175322 – [WPE] Implement WebsiteDataStore::defaultApplicationCacheDirectory() and friends

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

[Adrian Perez]

These are unimplemented, “APIWebsiteDataStore.cpp” has guards

  #if !PLATFORM(COCOA) && !PLATFORM(GTK)

which means that WPE is using the empty implementations. I think the
best way to go about this is renaming the GTK+ port implementation to
“APIWebsiteDataStoreGLib.cpp” file. That would be used both by the GTK+
and WPE ports, with a preprocessor switch inside the file to choose
the name of the base directory being used. Something in the lines of:

  #if PLATFORM(GTK)
  # define PLATFORM_DIR_BASE "webkitgtk"
  #elif PLATFORM(WPE)
  # define PLATFORM_DIR_BASE "wpe"
  #else
  # error Invalid platform
  #endif 

and then use the definition in the functions.

[Adrian Perez]

Created attachment 317568 [details]
Patch

[Adrian Perez]

Created attachment 317571 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 317571 [details]
Patch

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

> Source/WebKit/UIProcess/API/APIWebsiteDataStore.cpp:91
> -#if !PLATFORM(COCOA) && !PLATFORM(GTK)
> +#if !PLATFORM(COCOA) && !PLATFORM(GTK) && !PLATFORM(WPE)
>  WebKit::WebsiteDataStore::Configuration WebsiteDataStore::defaultDataStoreConfiguration()

This is now dead code then, I think we can just remove it.

> Source/WebKit/UIProcess/API/glib/APIWebsiteDataStoreGLib.cpp:32
> +# define PLATFORM_DIR_BASE "webkitgtk" G_DIR_SEPARATOR_S

Remove the space between # and define. I would call this BASE_DIRECTORY.

> Source/WebKit/UIProcess/API/glib/APIWebsiteDataStoreGLib.cpp:55
> +    return websiteDataDirectoryFileSystemRepresentation(PLATFORM_DIR_BASE "databases" G_DIR_SEPARATOR_S "indexeddb");

This look weird now, I think it's better not to include G_DIR_SEPARATOR_S as part of BASE_DIRECTORY macro

[Adrian Perez]

Created attachment 317574 [details]
Patch

[Adrian Perez]

(In reply to Carlos Garcia Campos from comment #3)
> Comment on attachment 317571 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=317571&action=review
> 
> > Source/WebKit/UIProcess/API/APIWebsiteDataStore.cpp:91
> > -#if !PLATFORM(COCOA) && !PLATFORM(GTK)
> > +#if !PLATFORM(COCOA) && !PLATFORM(GTK) && !PLATFORM(WPE)
> >  WebKit::WebsiteDataStore::Configuration WebsiteDataStore::defaultDataStoreConfiguration()
> 
> This is now dead code then, I think we can just remove it.

I think you are right, the new version of the patch also removes it.

At some point later on probably we would want to implement the missing
“WebsiteDataStore::defaultMediaCacheDirectory()” (that's bug #156369),
or is it unused in the GTK+ and WPE ports?

> > Source/WebKit/UIProcess/API/glib/APIWebsiteDataStoreGLib.cpp:32
> > +# define PLATFORM_DIR_BASE "webkitgtk" G_DIR_SEPARATOR_S
> 
> Remove the space between # and define. I would call this BASE_DIRECTORY.

Done.
 
> > Source/WebKit/UIProcess/API/glib/APIWebsiteDataStoreGLib.cpp:55
> > +    return websiteDataDirectoryFileSystemRepresentation(PLATFORM_DIR_BASE "databases" G_DIR_SEPARATOR_S "indexeddb");
> 
> This look weird now, I think it's better not to include G_DIR_SEPARATOR_S as
> part of BASE_DIRECTORY macro

This is changed in the updated patch, too.

[Carlos Garcia Campos]

Comment on attachment 317574 [details]
Patch

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

> Source/WebKit/ChangeLog:22
> [phone]  Adrian Perez de Castro  <[e-mail]>

Double changelog.

> Source/WebKit/UIProcess/API/glib/APIWebsiteDataStoreGLib.cpp:37
> +#else
> +#error Invalid platform
> +#endif

forgot to say before, sorry, but don't do this, it will fail to compile anyway.

[Adrian Perez]

Committed r220408: <http://trac.webkit.org/changeset/220408>