Created attachment 315185 [details] [review]
file-helpers: Add ephy_default_dot_dir()

Created attachment 315186 [details] [review]
web-app: Use always the default dot dir instead of the one for current mode

Created attachment 315187 [details] [review]
file-helpers: Only write the migration version when creating the dot dir

Created attachment 315188 [details] [review]
web-app: Add support for system-wide installed web apps

This is the initial support, but there are still some details. System-wide apps don't appear in about:applications because I'm not sure what to do in some cases:

 - Should we show all installed web apps? or only the ones for which the user already has a profile dir (user run it at least once).

 - Should we allow to delete system-wide apps? In this case we remove the profile dir, of course, the desktop file can't be removed. Fortunately we use "delete" instead of "uninstall" in about:applications. 

Maybe now we can use uninstall for user created apps and delete for system-wide ones, since in the case os user web apps, we are actually unsintalling them (they no longer appear in the apps menus). This depends on the first question, though. If we only show the apps already run by the user, it make sense to allow the user to delete them, otherwise we should use delete data or move it to personal data dialog.

Review of attachment 315186 [details] [review]:

Alternatively ephy_default_dot_dir() could return a const char * and you could free it in an exit handler, then it would be easier to use. But exit handlers aren't nice.

Review of attachment 315188 [details] [review]:

Few things to fix before you push....

Typo in commit message: "shuld" -> "should"

::: lib/ephy-web-app-utils.c
@@ +381,3 @@
+  }
+
+  /* The address should be the last command line argument */

I would clarify the comment: "the last command line argument in the desktop file"

@@ +429,3 @@
+   * options, which inits GTK+ and sets this as a side effect.
+   */
+  gdk_set_program_class (app_name);

Pretty sure we don't need this anymore, since it always follows the call to g_set_prgname().

@@ +472,3 @@
+  wm_class = g_desktop_app_info_get_startup_wm_class (desktop_info);
+  if (wm_class)
+    gdk_set_program_class (wm_class);

Ditto.

::: lib/ephy-web-app-utils.h
@@ +45,3 @@
 char    *ephy_web_application_create (const char *address, const char *name, GdkPixbuf *icon);
 
+char    *ephy_web_application_ensure_for_app_info (GAppInfo *app_info);

Not thrilled that an "ensure" function returns a string, since it's not clear at all from the signature what it's returning. I would probably do this with a out parameter if I wrote this myself, even though it's clunkier to use. On the other hand, you're just following the existing style of ephy_web_application_create(), which has the same problem.

::: src/ephy-main.c
@@ +349,3 @@
+
+  if (application_mode && !profile_directory) {
+    const char* desktop_file_path = g_getenv("GIO_LAUNCHED_DESKTOP_FILE");

Bad copy/paste from WebKit or something? The * should nest to desktop_file_path rather than char, and there's a missing space after g_getenv.

@@ +355,3 @@
+
+      if (desktop_info)
+        profile_directory = ephy_web_application_ensure_for_app_info (desktop_info);

Need to add a cast here:

ephy-main.c:357:71: warning: passing argument 1 of ‘ephy_web_application_ensure_for_app_info’ from incompatible pointer type [-Wincompatible-pointer-types]
         profile_directory = ephy_web_application_ensure_for_app_info (desktop_info);
                                                                       ^
In file included from ephy-main.c:34:0:
../lib/ephy-web-app-utils.h:47:7: note: expected ‘GAppInfo * {aka struct _GAppInfo *}’ but argument is of type ‘GDesktopAppInfo * {aka struct _GDesktopAppInfo *}’
 char    *ephy_web_application_ensure_for_app_info (GAppInfo *app_info);
       ^

(In reply to Michael Catanzaro from comment #7)
> Review of attachment 315188 [details] [review] [review]:
> 
> Few things to fix before you push....
> 
> Typo in commit message: "shuld" -> "should"

Oops.

> ::: lib/ephy-web-app-utils.c
> @@ +381,3 @@
> +  }
> +
> +  /* The address should be the last command line argument */
> 
> I would clarify the comment: "the last command line argument in the desktop
> file"

Sure.

> @@ +429,3 @@
> +   * options, which inits GTK+ and sets this as a side effect.
> +   */
> +  gdk_set_program_class (app_name);
> 
> Pretty sure we don't need this anymore, since it always follows the call to
> g_set_prgname().

I'm not so sure, because gdk has already been initialized at this point, so g_set_prgname() doesn't change anything.

> @@ +472,3 @@
> +  wm_class = g_desktop_app_info_get_startup_wm_class (desktop_info);
> +  if (wm_class)
> +    gdk_set_program_class (wm_class);
> 
> Ditto.

Ditto.

> ::: lib/ephy-web-app-utils.h
> @@ +45,3 @@
>  char    *ephy_web_application_create (const char *address, const char
> *name, GdkPixbuf *icon);
>  
> +char    *ephy_web_application_ensure_for_app_info (GAppInfo *app_info);
> 
> Not thrilled that an "ensure" function returns a string, since it's not
> clear at all from the signature what it's returning. I would probably do
> this with a out parameter if I wrote this myself, even though it's clunkier
> to use. On the other hand, you're just following the existing style of
> ephy_web_application_create(), which has the same problem.

Yes, I followed the style, we can change it on both cases if we really want it as a separate issue.

> ::: src/ephy-main.c
> @@ +349,3 @@
> +
> +  if (application_mode && !profile_directory) {
> +    const char* desktop_file_path = g_getenv("GIO_LAUNCHED_DESKTOP_FILE");
> 
> Bad copy/paste from WebKit or something? The * should nest to
> desktop_file_path rather than char, and there's a missing space after
> g_getenv.

Not copy paste but fast context switching, I guess.

> @@ +355,3 @@
> +
> +      if (desktop_info)
> +        profile_directory = ephy_web_application_ensure_for_app_info
> (desktop_info);
> 
> Need to add a cast here:
> 
> ephy-main.c:357:71: warning: passing argument 1 of
> ‘ephy_web_application_ensure_for_app_info’ from incompatible pointer type
> [-Wincompatible-pointer-types]
>          profile_directory = ephy_web_application_ensure_for_app_info
> (desktop_info);
>                                                                        ^
> In file included from ephy-main.c:34:0:
> ../lib/ephy-web-app-utils.h:47:7: note: expected ‘GAppInfo * {aka struct
> _GAppInfo *}’ but argument is of type ‘GDesktopAppInfo * {aka struct
> _GDesktopAppInfo *}’
>  char    *ephy_web_application_ensure_for_app_info (GAppInfo *app_info);
>        ^

Right. Thanks!

Patches pushed, we still need to decide what to do with about:applications

(In reply to Carlos Garcia Campos from comment #9)
> Patches pushed, we still need to decide what to do with about:applications

In another bug, it's too confusing to have bugs open for stuff fixed years ago.