[Spice-devel] [PATCH spice 8/9] spicec: add foreign menu

Hans de Goede hdegoede at redhat.com
Sun Oct 17 14:31:21 PDT 2010


Hi,

See 1 comment inline.

On 10/17/2010 04:13 PM, Arnon Gilboa wrote:
> Spice foreign menu enables external control of the client menu.
>
> The foreignmenu protocol enables an external application to:
> add a submenu, set its title, clear it, add/modify/remove an item etc.
>
> Foreign menu is based on the cross-platform named pipe.
> ---
>   client/Makefile.am         |    3 +
>   client/application.cpp     |   74 +++++++++-
>   client/application.h       |   26 +++-
>   client/foreign_menu.cpp    |  364 ++++++++++++++++++++++++++++++++++++++++++++
>   client/foreign_menu.h      |   98 ++++++++++++
>   client/foreign_menu_prot.h |  107 +++++++++++++
>   client/windows/redc.vcproj |   14 ++-
>   client/x11/Makefile.am     |    3 +
>   8 files changed, 683 insertions(+), 6 deletions(-)
>   create mode 100644 client/foreign_menu.cpp
>   create mode 100644 client/foreign_menu.h
>   create mode 100644 client/foreign_menu_prot.h
>
> diff --git a/client/Makefile.am b/client/Makefile.am
> index 185518a..0b3109b 100644
> --- a/client/Makefile.am
> +++ b/client/Makefile.am
> @@ -61,6 +61,9 @@ RED_COMMON_SRCS =			\
>   	debug.h				\
>   	display_channel.cpp		\
>   	display_channel.h		\
> +	foreign_menu.cpp		\
> +	foreign_menu.h			\
> +	foreign_menu_prot.h		\
>   	glz_decoded_image.h		\
>   	glz_decoder_config.h		\
>   	glz_decoder.cpp			\
> diff --git a/client/application.cpp b/client/application.cpp
> index c5d34ff..9a9a3f6 100644
> --- a/client/application.cpp
> +++ b/client/application.cpp
> @@ -335,6 +335,8 @@ enum AppCommands {
>   #ifdef USE_GUI
>       APP_CMD_SHOW_GUI,
>   #endif // USE_GUI
> +    APP_CMD_EXTERNAL_BEGIN = 0x400,
> +    APP_CMD_EXTERNAL_END = 0x800,
>   };
>
>   Application::Application()
> @@ -586,6 +588,13 @@ void Application::switch_host(const std::string&  host, int port, int sport,
>
>   int Application::run()
>   {
> +    _exit_code = ProcessLoop::run();
> +    return _exit_code;
> +}
> +
> +void Application::on_start_running()
> +{
> +    _foreign_menu.reset(new ForeignMenu(this));
>   #ifdef USE_GUI
>       if (_gui_mode != GUI_MODE_FULL) {
>           connect();


Erm, and who is going to call on_start_running, so that the
flow in the normal cmdline usage is not changed?

<snip>

> diff --git a/client/application.h b/client/application.h
> index 36ae86e..d6355ca 100644
> --- a/client/application.h
> +++ b/client/application.h
> @@ -26,6 +26,7 @@
>   #include "menu.h"
>   #include "hot_keys.h"
>   #include "process_loop.h"
> +#include "foreign_menu.h"
>
>   class RedScreen;
>   class Application;
> @@ -138,10 +139,23 @@ typedef std::list<KeyHandler*>  KeyHandlersStack;
>   typedef std::list<GUIBarrier*>  GUIBarriers;
>   #endif // USE_GUI
>
> +enum AppMenuItemType {
> +    APP_MENU_ITEM_TYPE_INVALID,
> +    APP_MENU_ITEM_TYPE_FOREIGN,
> +};
> +
> +typedef struct AppMenuItem {
> +    AppMenuItemType type;
> +    int32_t conn_ref;
> +    uint32_t ext_id;
> +} AppMenuItem;
> +
> +typedef std::map<int, AppMenuItem>  AppMenuItemMap;
> +
>   class Application : public ProcessLoop,
>                       public Platform::EventListener,
>                       public Platform::DisplayModeListener,
> -                    public CommandTarget {
> +                    public ForeignMenuInterface {
>   public:
>
>       enum State {

You're loosing CommandTarget as class Application inherits
from here, is that right?

<snip>

> diff --git a/client/foreign_menu_prot.h b/client/foreign_menu_prot.h
> new file mode 100644
> index 0000000..8c22461
> --- /dev/null
> +++ b/client/foreign_menu_prot.h
> @@ -0,0 +1,107 @@
> +/*
> +   Copyright (C) 2009 Red Hat, Inc.
> +
> +   This library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   This library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with this library; if not, see<http://www.gnu.org/licenses/>.
> +*/
> +
> +#ifndef _H_FOREIGN_MENU_PROT
> +#define _H_FOREIGN_MENU_PROT
> +
> +#define FOREIGN_MENU_MAGIC      (*(uint32_t*)"FRGM")
> +#define FOREIGN_MENU_VERSION    1
> +
> +#ifdef __GNUC__
> +#define ATTR_PACKED __attribute__ ((__packed__))
> +#else
> +#define ATTR_PACKED __declspec(align(1))
> +#endif
> +
> +typedef struct ATTR_PACKED FrgMenuInitHeader {
> +    uint32_t magic;
> +    uint32_t version;
> +    uint32_t size;
> +} FrgMenuInitHeader;
> +
> +typedef struct ATTR_PACKED FrgMenuInit {
> +    FrgMenuInitHeader base;
> +    uint64_t credentials;
> +    uint8_t title[0]; //UTF8
> +} FrgMenuInit;
> +
> +typedef struct ATTR_PACKED FrgMenuMsg {
> +    uint32_t id;
> +    uint32_t size;
> +} FrgMenuMsg;
> +
> +enum {
> +    //extrenal app ->  spice client
> +    FOREIGN_MENU_SET_TITLE = 1,
> +    FOREIGN_MENU_ADD_ITEM,
> +    FOREIGN_MENU_MODIFY_ITEM,
> +    FOREIGN_MENU_REMOVE_ITEM,
> +    FOREIGN_MENU_CLEAR,
> +
> +    //spice client ->  external app
> +    FOREIGN_MENU_ITEM_EVENT = 1001,
> +    FOREIGN_MENU_APP_ACTIVATED,
> +    FOREIGN_MENU_APP_DEACTIVATED,
> +};
> +
> +typedef struct ATTR_PACKED FrgMenuSetTitle {
> +    FrgMenuMsg base;
> +    uint8_t string[0]; //UTF8
> +} FrgMenuSetTitle;
> +
> +enum {
> +    FOREIGN_MENU_ITEM_TYPE_CHECKED      = 1<<  0,
> +    FOREIGN_MENU_ITEM_TYPE_DIM          = 1<<  1,
> +    FOREIGN_MENU_ITEM_TYPE_SEPARATOR    = 1<<  2
> +};
> +
> +#define FOREIGN_MENU_INVALID_ID 0
> +
> +typedef struct ATTR_PACKED FrgMenuAddItem {
> +    FrgMenuMsg base;
> +    uint32_t id;
> +    uint32_t type;
> +    uint32_t position;
> +    uint8_t string[0]; //UTF8
> +} FrgMenuAddItem, FrgMenuModItem;
> +
> +typedef struct ATTR_PACKED FrgMenuRmItem {
> +    FrgMenuMsg base;
> +    uint32_t id;
> +} FrgMenuRmItem;
> +
> +typedef struct FrgMenuMsg FrgMenuRmItems;
> +typedef struct FrgMenuMsg FrgMenuDelete;
> +
> +enum {
> +    FOREIGN_MENU_EVENT_CLICK,
> +    FOREIGN_MENU_EVENT_CHECKED,
> +    FOREIGN_MENU_EVENT_UNCHECKED
> +};
> +
> +typedef struct ATTR_PACKED FrgMenuEvent {
> +    FrgMenuMsg base;
> +    uint32_t id;
> +    uint32_t action; //FOREIGN_MENU_EVENT_?
> +} FrgMenuEvent;
> +
> +typedef struct FrgMenuMsg FrgMenuActivate;
> +typedef struct FrgMenuMsg FrgMenuDeactivate;
> +
> +#undef ATTR_PACKED
> +
> +#endif

Hmm, this feels like it should be in spice-protocol not in spice!

<snip>

Regards,

Hans



More information about the Spice-devel mailing list