[Webkit-unassigned] [Bug 35762] New port: EFL; adding files to WebCore/plugins/efl (patch 2/2)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=35762





--- Comment #2 from Holger Freyther <zecke at selfish.org>  2010-03-09 20:11:46 PST ---
(From update of attachment 50045)
Patch mostly looks fine. I have minir comments inside...



> +X11EmbedContainer::~X11EmbedContainer()
> +{
> +    setVisible(false);
> +
> +    if (client()) {
> +        XUnmapWindow(x11Info()->display(), client());
> +        XReparentWindow(x11Info()->display(), client(), rootWindow(), 0, 0);
> +    }
> +
> +    delete m_focusProxy;
> +    delete m_clientWindow;

It would make sense to make m_focusProxy and m_clientWindow OwnPtr here.



> +class X11Info {
> +public:
> +    Display* display() { return (Display*) ecore_x_display_get(); }
> +    Visual* visual() { return (Visual*) m_att.visual; }
> +    unsigned int depth() { return m_att.depth; }
> +    Colormap colormap() { return m_att.colormap; }
> +    unsigned int x11Time() { return ecore_x_current_time_get(); }
> +
> +private:
> +    X11Info(WinId owner)
> +    {
> +        m_owner = owner;
> +        ecore_x_window_attributes_get(owner, &m_att);
> +        m_att.colormap = DefaultColormap(display(), DefaultScreen(display()));
> +    }
> +    WinId m_owner;
> +    Ecore_X_Window_Attributes m_att;
> +    friend class X11Window;
> +};


Is there any reason to not have X11Info more static? Currently you will end up
having
one instance per plugin, I think having one instance per Screen should be fine?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.