[Webkit-unassigned] [Bug 48509] [GTK] Implement UpdateChunk, ChunkedUpdateDrawingArea/Proxy, WebView classes for WebKit2 and add WKAPI

[bugzilla-daemon at webkit.org]

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





--- Comment #11 from Carlos Garcia Campos <cgarcia at igalia.com>  2011-01-12 01:18:56 PST ---
(From update of attachment 78232)
View in context: https://bugs.webkit.org/attachment.cgi?id=78232&action=review

I'm not reviewer, this is an unofficial review.

> WebKit2/Shared/gtk/UpdateChunk.h:47
> +    GdkPixmap* memory() const { return m_bitmapSharedMemory; }

GdkPixmap is deprecated, cairo surfaces should be used instead. Also the name memory() is confusing to me, since it doesn't return the memory data but the GdkPixmap.

> WebKit2/Shared/gtk/UpdateChunk.cpp:48
> +    m_bitmapSharedMemory = gdk_pixmap_new(0, rect.width(), rect.height(), gdk_visual_get_depth(gdk_visual_get_system()));
> +    m_xID = static_cast<uint32_t>(GDK_PIXMAP_XID(m_bitmapSharedMemory));

Since GdkPixmap is deprecated we could use a mmapped file and a cairo image surface to draw into.

> WebKit2/WebProcess/WebPage/ChunkedUpdateDrawingArea.cpp:94
> +#if PLATFORM(GTK)
> +    xIDMap.set(updateChunk.xID(), updateChunk.memory());
> +#endif

I think we should avoid having platform #ifdefs in common code

> WebKit2/UIProcess/gtk/ChunkedUpdateDrawingAreaProxyGtk.cpp:64
> +    GdkGC* gc = gdk_gc_new(m_backingStoreBitmap);

GdkGC is deprecated too, you can get a cairo context with gdk_cairo_create() and use cairo api for drawing.

> WebKit2/UIProcess/gtk/ChunkedUpdateDrawingAreaProxyGtk.cpp:85
> +    gdk_window_invalidate_rect(m_webView->window()->window, &gdkRect, false);

GtkWidget::window is private now in GTK+. Instead of having a method that returns the view window, I would add a method to the view to update a rectangle that calls gdk_window_invalidate_rect().

> WebKit2/UIProcess/ChunkedUpdateDrawingAreaProxy.cpp:124
> +#if PLATFORM(GTK)
> +    page->process()->send(DrawingAreaLegacyMessage::DidSetSizeDone, page->pageID(), CoreIPC::In(info().identifier, updateChunk->xID()));
> +#endif

Platform #ifdefs in common code again, I think we can avoid this.

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