[epiphany/pgriffis/web-extension/commands: 3/3] WebExtensions: Refactor commands API

[Patrick Griffis <pgriffis src gnome org>]

commit 794249739bd7b58f510fa069c555abf093f44cbb
Author: Patrick Griffis <pgriffis igalia com>
Date:   Wed Jul 6 22:35:29 2022 -0500

    WebExtensions: Refactor commands API
    
    This simplifies things a bit, removing a duplicated Command struct.
    
    Fixes a handful of leaks.
    
    Namespaces the Action name so its private-per-extension.
    
    Adds strict accelerator parsing.
    
    Ensures accels are added on load and removed on unload.

 src/webextension/api/commands.c               | 229 ++++++++++++-------------
 src/webextension/api/commands.h               |   3 +
 src/webextension/ephy-web-extension-manager.c |   3 +
 src/webextension/ephy-web-extension.c         | 234 ++++++++++++++------------
 src/webextension/ephy-web-extension.h         |  25 ++-
 5 files changed, 258 insertions(+), 236 deletions(-)
---
diff --git a/src/webextension/api/commands.c b/src/webextension/api/commands.c
index e12287ec5..9302d371b 100644
--- a/src/webextension/api/commands.c
+++ b/src/webextension/api/commands.c
@@ -24,141 +24,91 @@
 #include "api-utils.h"
 #include "commands.h"
 
-/* Local command struct. */
-typedef struct {
-  EphyWebExtension *web_extension; /* Parent object */
-  char *name;
-  char *accelerator;
-  char *description;
-} Command;
-
-static void
-command_destroy (Command *command)
-{
-  g_free (command->name);
-  g_free (command->accelerator);
-  g_free (command->description);
-  g_free (command);
-}
-
 static void
 on_command_activated (GAction  *action,
                       GVariant *parameter,
                       gpointer  user_data)
 {
+  EphyWebExtension *self = user_data;
   EphyWebExtensionManager *manager = ephy_web_extension_manager_get_default ();
-  Command *cmd = user_data;
-  JsonNode *node;
-  node = json_node_init_string (json_node_alloc (), cmd->name);
 
   ephy_web_extension_manager_emit_in_extension_views (manager,
-                                                      cmd->web_extension,
+                                                      self,
                                                       "commands.onCommand",
-                                                      json_to_string (node, FALSE));
+                                                      g_object_get_data (G_OBJECT (action), 
"command-name-json"));
+}
+
+char *
+get_accel_action_name (EphyWebExtension    *self,
+                 WebExtensionCommand *command)
+{
+  return g_strdup_printf ("app.webextension-command-%s-%s",
+                           ephy_web_extension_get_guid (self),
+                           command->name);
+}
+
+char *
+get_action_name (EphyWebExtension    *self,
+                 WebExtensionCommand *command)
+{
+  return g_strdup_printf ("webextension-command-%s-%s",
+                           ephy_web_extension_get_guid (self),
+                           command->name);
 }
 
 static void
-setup_actions (Command *cmd)
+destroy_action (EphyWebExtension    *self,
+                WebExtensionCommand *command)
 {
-  GSimpleAction *action = g_simple_action_new (cmd->name, NULL);
-  g_signal_connect (action, "activate", G_CALLBACK (on_command_activated), cmd);
-  g_action_map_add_action (G_ACTION_MAP (ephy_shell_get_default ()), (GAction *)action);
+  g_autofree char *action_name = get_action_name (self, command);
+  g_autofree char *accel_action_name = get_accel_action_name (self, command);
+  const char * const empty_accels[] = { NULL };
   gtk_application_set_accels_for_action (GTK_APPLICATION (ephy_shell_get_default ()),
-                                         g_strdup_printf ("app.%s", cmd->name),
-                                         (const char *[]) {
-    cmd->accelerator,
-    NULL,
-  });
+                                         accel_action_name,
+                                         empty_accels);
+  g_action_map_remove_action (G_ACTION_MAP (ephy_shell_get_default ()), action_name);
 }
 
-static Command *
-create_command (EphyWebExtension *self,
-                guint             cmd)
+static void
+setup_action (EphyWebExtension    *self,
+              WebExtensionCommand *command)
 {
-  char *shortcut;
-  char *suggested_key;
-  char *description;
-  Command *command = g_new0 (Command, 1);
-
-  ephy_web_extension_get_command_data_from_index (self,
-                                                  cmd,
-                                                  &shortcut,
-                                                  &suggested_key,
-                                                  &description);
+  const char * const accels[] = { command->accelerator, NULL };
+  g_autofree char *action_name = get_action_name (self, command);
+  g_autofree char *accel_action_name = get_accel_action_name (self, command);
+  g_autoptr (GSimpleAction) action = g_simple_action_new (action_name, NULL);
 
-  command->web_extension = self;
-  command->name = shortcut;
-  command->accelerator = suggested_key;
-  command->description = description;
+  g_action_map_add_action (G_ACTION_MAP (ephy_shell_get_default ()), G_ACTION (action));
 
-  setup_actions (command);
+  gtk_application_set_accels_for_action (GTK_APPLICATION (ephy_shell_get_default ()),
+                                         accel_action_name,
+                                         accels);
 
-  return command;
+  /* Lazy way to pass this info to on_command_activated(). */
+  g_object_set_data_full (G_OBJECT (action), "command-name-json", g_strdup_printf ("\"%s\"", command->name), 
g_free);
 }
 
 static GHashTable *
 get_commands (EphyWebExtension *self)
 {
-  GList *commands = ephy_web_extension_get_commands (self);
-  Command *cmd = NULL;
-
-  GHashTable *cmds = g_object_get_data (G_OBJECT (self), "commands");
-  if (cmds)
-    return cmds;
-
-  cmds = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, (GDestroyNotify)command_destroy);
-  g_object_set_data_full (G_OBJECT (self), "commands", cmds, (GDestroyNotify)g_hash_table_destroy);
-
-  for (GList *list = commands; list && list->data; list = list->next) {
-    cmd = create_command (self, g_list_index (list, list->data));
-
-    g_hash_table_replace (cmds, cmd->name, cmd);
-  }
-  return cmds;
+  return g_object_get_data (G_OBJECT (self), "commands");
 }
 
 static JsonNode *
-command_to_node (Command *cmd)
+command_to_node (WebExtensionCommand *command)
 {
   JsonNode *node;
   JsonObject *obj;
 
-  if (!cmd)
-    return NULL;
-
   node = json_node_init_object (json_node_alloc (), json_object_new ());
   obj = json_node_get_object (node);
-  json_object_set_string_member (obj, "name", cmd->name);
-  json_object_set_string_member (obj, "shortcut", cmd->accelerator);
-  json_object_set_string_member (obj, "description", cmd->description);
+  json_object_set_string_member (obj, "name", command->name);
+  json_object_set_string_member (obj, "shortcut", command->shortcut);
+  json_object_set_string_member (obj, "description", command->description);
 
   return node;
 }
 
-static char *
-create_accelerator (const char *orig_string)
-{
-  char **accelerator_keys = NULL;
-  char *accelerator = "";
-
-  /* FIXME: Stricter validation. */
-  if (strchr (orig_string, '<') != NULL || strchr (orig_string, '>') != NULL)
-    return NULL;
-
-  accelerator_keys = g_strsplit ((const gchar *)orig_string, "+", 0);
-
-  for (int i = 0; accelerator_keys[i]; i++) {
-    /* We have to use 2 here, as F# keys are treated like normal keys. */
-    if (strlen (accelerator_keys[i]) > 3) {
-      accelerator = g_strdup_printf ("%s<%s>", accelerator, accelerator_keys[i]);
-    } else {
-      accelerator = g_strdup_printf ("%s%s", accelerator, accelerator_keys[i]);
-    }
-  }
-
-  return accelerator;
-}
-
 static void
 commands_handler_get_all (EphyWebExtensionSender *sender,
                           const char             *method_name,
@@ -169,11 +119,11 @@ commands_handler_get_all (EphyWebExtensionSender *sender,
   g_autoptr (JsonNode) node = json_node_init_array (json_node_alloc (), json_array_new ());
   JsonArray *rel = json_node_get_array (node);
   GHashTableIter iter;
-  Command *cmd;
+  WebExtensionCommand *command;
 
   g_hash_table_iter_init (&iter, commands);
-  while (g_hash_table_iter_next (&iter, NULL, (gpointer *)&cmd))
-    json_array_add_element (rel, command_to_node (cmd));
+  while (g_hash_table_iter_next (&iter, NULL, (gpointer *)&command))
+    json_array_add_element (rel, command_to_node (command));
 
   g_task_return_pointer (task, json_to_string (node, FALSE), g_free);
 }
@@ -184,9 +134,11 @@ commands_handler_reset (EphyWebExtensionSender *sender,
                         JsonArray              *args,
                         GTask                  *task)
 {
-  GHashTable *commands = get_commands (sender->extension);
+  GHashTable *default_commands = ephy_web_extension_get_commands (sender->extension);
+  GHashTable *active_commands = get_commands (sender->extension);
   const char *name = ephy_json_array_get_string (args, 0);
-  Command *command;
+  WebExtensionCommand *command;
+  WebExtensionCommand *default_command;
   g_autofree char *action_name = NULL;
   g_autofree char *shortcut = NULL;
   g_autofree char *suggested_key = NULL;
@@ -197,27 +149,26 @@ commands_handler_reset (EphyWebExtensionSender *sender,
     return;
   }
 
-  command = g_hash_table_lookup (commands, name);
+  command = g_hash_table_lookup (active_commands, name);
   if (!command) {
     g_task_return_new_error (task, WEB_EXTENSION_ERROR, WEB_EXTENSION_ERROR_INVALID_ARGUMENT, 
"commands.reset(): Did not find command by name %s", name);
     return;
   }
 
-  ephy_web_extension_get_command_data_from_name (sender->extension,
-                                                 name,
-                                                 &shortcut,
-                                                 &suggested_key,
-                                                 &description);
+  g_assert (default_commands);
+  default_command = g_hash_table_lookup (default_commands, name);
+  g_assert (default_command);
+
+  destroy_action (sender->extension, command);
 
-  g_free (command->accelerator);
   g_free (command->description);
-  command->description = g_steal_pointer (&description);
-  command->accelerator = create_accelerator (suggested_key);
+  g_free (command->accelerator);
+  g_free (command->shortcut);
+  command->description = g_strdup (default_command->description);
+  command->accelerator = g_strdup (default_command->accelerator);
+  command->shortcut = g_strdup (default_command->shortcut);
 
-  action_name = g_strdup_printf ("app.%s", command->name);
-  gtk_application_set_accels_for_action (GTK_APPLICATION (ephy_shell_get_default ()),
-                                         action_name,
-                                         (const char *[]) { command->accelerator, NULL });
+  setup_action (sender->extension, command);
 
   g_task_return_pointer (task, NULL, NULL);
 }
@@ -230,7 +181,7 @@ commands_handler_update (EphyWebExtensionSender *sender,
 {
   GHashTable *commands = get_commands (sender->extension);
   JsonObject *details = ephy_json_array_get_object (args, 0);
-  Command *command;
+  WebExtensionCommand *command;
   g_autofree char *action_name = NULL;
   const char *name;
   const char *description;
@@ -263,9 +214,11 @@ commands_handler_update (EphyWebExtensionSender *sender,
   if (shortcut && !*shortcut) {
     /* Empty string is set to nothing. */
     g_free (command->accelerator);
+    g_free (command->shortcut);
     command->accelerator = NULL;
+    command->shortcut = NULL;
   } else if (shortcut) {
-    g_autofree char *new_accelerator = create_accelerator (shortcut);
+    g_autofree char *new_accelerator = ephy_web_extension_parse_command_key (shortcut);
 
     if (!new_accelerator) {
       g_task_return_new_error (task, WEB_EXTENSION_ERROR, WEB_EXTENSION_ERROR_INVALID_ARGUMENT, 
"commands.update(): Shortcut was invalid: '%s'", shortcut);
@@ -273,11 +226,13 @@ commands_handler_update (EphyWebExtensionSender *sender,
     }
 
     g_free (command->accelerator);
+    g_free (command->shortcut);
     command->accelerator = g_steal_pointer (&new_accelerator);
+    command->shortcut = g_strdup (shortcut);
   }
 
   if (shortcut) {
-    action_name = g_strdup_printf ("app.%s", command->name);
+    action_name = get_accel_action_name (sender->extension, command);
     gtk_application_set_accels_for_action (GTK_APPLICATION (ephy_shell_get_default ()),
                                            action_name,
                                            (const char *[]) { command->accelerator, NULL });
@@ -310,3 +265,41 @@ ephy_web_extension_api_commands_handler (EphyWebExtensionSender *sender,
   g_warning ("%s(): '%s' not implemented by Epiphany!", __FUNCTION__, method_name);
   g_task_return_new_error (task, WEB_EXTENSION_ERROR, WEB_EXTENSION_ERROR_NOT_IMPLEMENTED, "Not 
Implemented");
 }
+
+void
+ephy_web_extension_api_commands_init (EphyWebExtension *self)
+{
+  GHashTable *default_commands = ephy_web_extension_get_commands (self);
+  GHashTable *active_commands = g_hash_table_new_similar (default_commands);
+  GHashTableIter iter;
+  WebExtensionCommand *command = NULL;
+
+  /* We load the default commands from the manifest and set them up here. */
+  if (default_commands) {
+    g_hash_table_iter_init (&iter, default_commands);
+    while (g_hash_table_iter_next (&iter, NULL, (gpointer *)&command)) {
+      WebExtensionCommand *new_command;
+      new_command = web_extension_command_copy (command);
+      g_hash_table_replace (active_commands, new_command->name, new_command);
+      setup_action (self, new_command);
+    }
+  }
+
+  g_object_set_data_full (G_OBJECT (self), "commands", active_commands, (GDestroyNotify)g_hash_table_unref);
+}
+
+void
+ephy_web_extension_api_commands_dispose (EphyWebExtension *self)
+{
+  GHashTable *active_commands = get_commands (self);
+  GHashTableIter iter;
+  WebExtensionCommand *command = NULL;
+
+  /* We load the default commands from the manifest and set them up here. */
+  g_hash_table_iter_init (&iter, active_commands);
+  while (g_hash_table_iter_next (&iter, NULL, (gpointer *)&command)) {
+    destroy_action (self, command);
+  }
+
+  g_object_set_data (G_OBJECT (self), "commands", NULL);
+}
diff --git a/src/webextension/api/commands.h b/src/webextension/api/commands.h
index 93f5cf2bf..dc8a45daf 100644
--- a/src/webextension/api/commands.h
+++ b/src/webextension/api/commands.h
@@ -32,4 +32,7 @@ void ephy_web_extension_api_commands_handler (EphyWebExtensionSender *sender,
                                               JsonArray              *args,
                                               GTask                  *task);
 
+void ephy_web_extension_api_commands_init    (EphyWebExtension *self);
+void ephy_web_extension_api_commands_dispose (EphyWebExtension *self);
+
 G_END_DECLS
diff --git a/src/webextension/ephy-web-extension-manager.c b/src/webextension/ephy-web-extension-manager.c
index 40ded52c9..f5a026d60 100644
--- a/src/webextension/ephy-web-extension-manager.c
+++ b/src/webextension/ephy-web-extension-manager.c
@@ -1381,12 +1381,15 @@ ephy_web_extension_manager_set_active (EphyWebExtensionManager *self,
 
     if (ephy_web_extension_has_background_web_view (web_extension))
       run_background_script (self, web_extension);
+
+    ephy_web_extension_api_commands_init (web_extension);
   } else {
     g_signal_handlers_disconnect_by_data (shell, web_extension);
 
     g_hash_table_remove (self->browser_action_map, web_extension);
     g_hash_table_remove (self->background_web_views, web_extension);
     g_object_set_data (G_OBJECT (web_extension), "alarms", NULL); /* Set in alarms.c */
+    ephy_web_extension_api_commands_dispose (web_extension);
   }
 }
 
diff --git a/src/webextension/ephy-web-extension.c b/src/webextension/ephy-web-extension.c
index fda0a77f3..4693c2b9f 100644
--- a/src/webextension/ephy-web-extension.c
+++ b/src/webextension/ephy-web-extension.c
@@ -75,12 +75,6 @@ typedef struct {
   WebKitUserStyleSheet *style;
 } WebExtensionCustomCSS;
 
-typedef struct {
-  char *shortcut;
-  char *suggested_key;
-  char *description;
-} WebExtensionCommand;
-
 struct _EphyWebExtension {
   GObject parent_instance;
 
@@ -111,7 +105,7 @@ struct _EphyWebExtension {
   char *local_storage_path;
   JsonNode *local_storage;
   GHashTable *web_accessible_resources;
-  GList *commands;
+  GHashTable *commands;
 };
 
 G_DEFINE_QUARK (web - extension - error - quark, web_extension_error)
@@ -759,129 +753,158 @@ web_extension_add_web_accessible_resource (JsonArray *array,
   g_hash_table_add (self->web_accessible_resources, g_strdup (web_accessible_resource));
 }
 
-char *
-ephy_web_extension_get_command_accelerator (JsonObject *suggested_key_object)
+static gboolean
+is_valid_key (const char *key)
 {
-  const char *orig_string = NULL;
-  char **accelerator_keys = NULL;
-  char *accelerator = "";
-
-  if (!ephy_json_object_get_string (suggested_key_object, "default") &&
-      !ephy_json_object_get_string (suggested_key_object, "linux")) {
-    return NULL;
-  } else if (ephy_json_object_get_string (suggested_key_object, "default")) {
-    orig_string = ephy_json_object_get_string (suggested_key_object, "default");
-  } else if (ephy_json_object_get_string (suggested_key_object, "linux")) {
-    orig_string = ephy_json_object_get_string (suggested_key_object, "linux");
-  }
+  static const char * const allowed_keys[] = { "Comma", "Period", "Home", "End", "PageUp", "PageDown", 
"Space", "Insert", "Delete", "Up", "Down", "Left", "Right", NULL};
 
-  accelerator_keys = g_strsplit ((const gchar *)orig_string, "+", 0);
+  if (strlen (key) == 1 && ((*key >= 'A' && *key <= 'Z') || (*key >= '0' && *key <= '9')))
+    return TRUE;
 
-  for (int i = 0; accelerator_keys[i]; i++) {
-    /* We have to use 2 here, as F# keys are treated like normal keys. */
-    if (strlen (accelerator_keys[i]) > 2) {
-      accelerator = g_strdup_printf ("%s<%s>", accelerator, accelerator_keys[i]);
-    } else {
-      accelerator = g_strdup_printf ("%s%s", accelerator, accelerator_keys[i]);
-    }
-  }
+  if (strlen (key) == 2 && key[0] == 'F' && (key[1] >= '0' && key[1] <= '9'))
+    return TRUE;
 
-  return accelerator;
+  return g_strv_contains (allowed_keys, key);
 }
 
-static void
-web_extension_parse_commands (EphyWebExtension *self,
-                              JsonObject       *object)
-{
-  JsonNode *node = NULL;
-  JsonObject *cmd_object = NULL;
-  JsonObject *key_object = NULL;
-  WebExtensionCommand *cmd = g_malloc0 (sizeof (WebExtensionCommand));
-  const char *description;
-  const char *shortcut;
-  const char *suggested_key;
-
-  for (GList *list = json_object_get_members (object); list && list->data; list = list->next) {
-    node = json_object_get_member (object, (gchar *)list->data);
-    cmd_object = ephy_json_node_get_object (node);
-    key_object = ephy_json_object_get_object (cmd_object, "suggested_key");
-
-    if (!cmd_object) {
-      LOG ("Skipping command as value is invalid");
-      return;
+/*
+ * ephy_web_extension_parse_command_key:
+ * @key: A command key
+ *
+ * This parses a key in the format specified on 
[MDN](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/commands).
+ *
+ * It returns a string in Gtk's accelerator format.
+ *
+ * Returns: (transfer full): An accelerator or %NULL if invalid
+ */
+char *
+ephy_web_extension_parse_command_key (const char *suggested_key)
+{
+  g_autoptr (GString) accelerator = g_string_sized_new (strlen (suggested_key) + 5);
+  g_auto (GStrv) keys = NULL;
+  guint i;
+  gboolean has_modifier = FALSE;
+  gboolean has_key = FALSE;
+
+  if (strcmp (suggested_key, "MediaNextTrack") == 0)
+    return g_strdup ("XF86AudioNext");
+  else if (strcmp (suggested_key, "MediaPlayPause") == 0)
+    return g_strdup ("XF86AudioPlay");
+  else if (strcmp (suggested_key, "MediaPrevTrack") == 0)
+    return g_strdup ("XF86AudioPrev");
+  else if (strcmp (suggested_key, "MediaStop") == 0)
+    return g_strdup ("XF86AudioStop");
+
+  keys = g_strsplit (suggested_key, "+", 3);
+  for (i = 0; keys[i]; i++) {
+    const char *key = keys[i];
+    /* First two are potentially modifiers. We should check for duplicates but its probably harmless. */
+    if (i == 0 || i == 1) {
+      if (strcmp (key, "Ctrl") == 0 || strcmp (key, "Alt") == 0 || (i == 1 && strcmp (key, "Shift") == 0))
+        g_string_append_printf (accelerator, "<%s>", key);
+      else if (strcmp (key, "Command") == 0 || strcmp (key, "MacCtrl") == 0)
+        g_string_append (accelerator, "<Ctrl>");
+      else {
+        g_debug ("Invalid modifier at index %u: %s", i, key);
+        return NULL;
+      }
+      has_modifier = TRUE;
+      continue;
     }
-
-    description = ephy_json_object_get_string (cmd_object, "description");
-
-    shortcut = (gchar *)list->data;
-    if (!shortcut) {
-      LOG ("Skipping command as value is invalid");
-      return;
+    /* Second two are potentially keys. */
+    else if (i == 1 || i == 2) {
+      if (has_key) {
+        g_debug ("Command key has two keys: %s", suggested_key);
+        return NULL;
+      }
+      if (!is_valid_key (key)) {
+        g_debug ("Command key has invalid_key: %s", key);
+        return NULL;
+      }
+      g_string_append (accelerator, key);
+      has_key = TRUE;
     }
+  }
 
-    cmd->description = g_strdup (description);
-    cmd->shortcut = g_strdup (shortcut);
-
-    suggested_key = ephy_web_extension_get_command_accelerator (key_object);
-
-    if (!g_strcmp0 (suggested_key, "") == 0)
-      cmd->suggested_key = g_strdup (suggested_key);
-
-    self->commands = g_list_append (self->commands, cmd);
+  if (!has_modifier && !has_key) {
+    g_debug ("Command key requires a modifier and a key: %s", suggested_key);
+    return NULL;
   }
+
+  return g_steal_pointer (&accelerator->str);
 }
 
-void
-ephy_web_extension_get_command_data_from_index (EphyWebExtension  *self,
-                                                guint              command,
-                                                char             **shortcut,
-                                                char             **suggested_key,
-                                                char             **description)
+GHashTable *
+ephy_web_extension_get_commands (EphyWebExtension *self)
 {
-  WebExtensionCommand *cmd = g_list_nth_data (self->commands, command);
-
-  *shortcut = g_strdup (cmd->shortcut);
-  *suggested_key = g_strdup (cmd->suggested_key);
-  *description = g_strdup (cmd->description);
-
-  return;
+  return self->commands;
 }
 
-void *
-ephy_web_extension_get_command_data_from_name (EphyWebExtension  *self,
-                                               const char        *name,
-                                               char             **shortcut,
-                                               char             **suggested_key,
-                                               char             **description)
+static WebExtensionCommand *
+web_extension_command_new (const char *name,
+                           const char *description,
+                           const char *accelerator,
+                           const char *shortcut)
 {
-  WebExtensionCommand *cmd = NULL;
+  WebExtensionCommand *command = g_new (WebExtensionCommand, 1);
 
-  for (GList *list = self->commands; list && list->data; list = list->next) {
-    cmd = list->data;
+  command->name = g_strdup (name);
+  command->description = g_strdup (description);
+  command->accelerator = g_strdup (accelerator);
+  command->shortcut = g_strdup (shortcut);
 
-    if (strcmp (cmd->shortcut, name) == 0) {
-      *shortcut = strdup (cmd->shortcut);
-      *suggested_key = strdup (cmd->suggested_key);
-      *description = strdup (cmd->description);
-    }
-  }
+  return command;
+}
 
-  return NULL;
+static void
+web_extension_command_free (WebExtensionCommand *command)
+{
+  g_clear_pointer (&command->name, g_free);
+  g_clear_pointer (&command->description, g_free);
+  g_clear_pointer (&command->accelerator, g_free);
+  g_clear_pointer (&command->shortcut, g_free);
+  g_free (command);
 }
 
-GList *
-ephy_web_extension_get_commands (EphyWebExtension *self)
+WebExtensionCommand *
+web_extension_command_copy (WebExtensionCommand *command)
 {
-  return self->commands;
+  return web_extension_command_new (command->name, command->description, command->accelerator, 
command->shortcut);
 }
 
 static void
-web_extension_commands_free (WebExtensionCommand *cmd)
+web_extension_parse_commands (EphyWebExtension *self,
+                              JsonObject       *object)
 {
-  g_clear_pointer (&cmd->shortcut, g_free);
-  g_clear_pointer (&cmd->suggested_key, g_free);
-  g_clear_pointer (&cmd->description, g_free);
-  g_free (cmd);
+  for (GList *list = json_object_get_members (object); list; list = list->next) {
+    JsonObject *command_object;
+    JsonObject *key_object;
+    const char *suggested_key;
+    WebExtensionCommand *command;
+    g_autofree char *accelerator = NULL;
+    g_autofree char *description = NULL;
+    const char *name = list->data;
+
+    command_object = ephy_json_object_get_object (object, name);
+    if (!command_object) {
+      LOG ("Skipping command as value is invalid");
+      continue;
+    }
+
+    description = ephy_web_extension_manifest_get_localized_string (self, command_object, "description");
+    key_object = ephy_json_object_get_object (command_object, "suggested_key");
+
+    if (key_object) {
+      if ((suggested_key = ephy_json_object_get_string (key_object, "linux")))
+        accelerator = ephy_web_extension_parse_command_key (suggested_key);
+      else if ((suggested_key = ephy_json_object_get_string (key_object, "default")))
+        accelerator = ephy_web_extension_parse_command_key (suggested_key);
+    }
+
+    command = web_extension_command_new (name, description, accelerator, suggested_key);
+    g_message ("Inserting command %s (%s)", command->name, command->accelerator);
+    g_hash_table_replace (self->commands, command->name, command);
+  }
 }
 
 static void
@@ -909,11 +932,11 @@ ephy_web_extension_dispose (GObject *object)
   g_clear_pointer (&self->host_permissions, g_ptr_array_unref);
   g_clear_pointer (&self->local_storage, json_node_unref);
   g_clear_pointer (&self->web_accessible_resources, g_hash_table_unref);
+  g_clear_pointer (&self->commands, g_hash_table_unref);
 
   g_clear_pointer (&self->page_action, web_extension_page_action_free);
   g_clear_pointer (&self->browser_action, web_extension_browser_action_free);
   g_clear_list (&self->custom_css, (GDestroyNotify)webkit_user_style_sheet_unref);
-  g_clear_list (&self->commands, (GDestroyNotify)web_extension_commands_free);
 
   g_hash_table_destroy (self->page_action_map);
 
@@ -934,6 +957,7 @@ ephy_web_extension_init (EphyWebExtension *self)
   self->page_action_map = g_hash_table_new (NULL, NULL);
   self->permissions = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
   self->host_permissions = g_ptr_array_new_full (2, g_free);
+  self->commands = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, 
(GDestroyNotify)web_extension_command_free);
 
   self->guid = g_uuid_string_random ();
 
diff --git a/src/webextension/ephy-web-extension.h b/src/webextension/ephy-web-extension.h
index 4586ce3e7..d4461dc02 100644
--- a/src/webextension/ephy-web-extension.h
+++ b/src/webextension/ephy-web-extension.h
@@ -66,6 +66,15 @@ typedef enum {
   WEB_EXTENSION_ERROR_INVALID_HOST = 1006,
 } WebExtensionErrorCode;
 
+typedef struct {
+  char *name;
+  char *description;
+  char *accelerator;
+  char *shortcut;
+} WebExtensionCommand;
+
+WebExtensionCommand *web_extension_command_copy (WebExtensionCommand *command);
+
 typedef struct {
   char *name;
   EphyApiExecuteFunc execute;
@@ -148,19 +157,7 @@ const char            *ephy_web_extension_get_option_ui_page              (EphyW
 
 const char            *ephy_web_extension_get_guid                        (EphyWebExtension *self);
 
-GList                 *ephy_web_extension_get_commands                    (EphyWebExtension *self);
-
-void                   ephy_web_extension_get_command_data_from_index     (EphyWebExtension    *self,
-                                                                           guint                command,
-                                                                           char               **shortcut,
-                                                                           char               
**suggested_key,
-                                                                           char               **description);
-
-void                  *ephy_web_extension_get_command_data_from_name      (EphyWebExtension    *self,
-                                                                           const char          *name,
-                                                                           char               **shortcut,
-                                                                           char               
**suggested_key,
-                                                                           char               **description);
+GHashTable            *ephy_web_extension_get_commands                    (EphyWebExtension *self);
 
 gboolean               ephy_web_extension_has_tab_or_host_permission      (EphyWebExtension *self,
                                                                            EphyWebView      *web_view,
@@ -192,5 +189,7 @@ gboolean               ephy_web_extension_rule_matches_uri                (const
 gboolean               ephy_web_extension_has_web_accessible_resource     (EphyWebExtension *self,
                                                                            const char       *path);
 
+char                   *ephy_web_extension_parse_command_key              (const char       *suggested_key);
+
 G_END_DECLS