[Webkit-unassigned] [Bug 154444] [ThreadedCompositor] Opening the inspector in a window causes a crash.

[bugzilla-daemon at webkit.org]

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

--- Comment #17 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #16)
> (In reply to comment #15)
> > > > Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:69
> > > > +        ensureGLContext();
> > > > +        updateViewport();
> > > 
> > > It's not clear from the change log -- are these calls required in case of
> > > handle being null?
> > 
> > Ok, I'll update the ChangeLog. When the handler is null, ensureGLContext
> > actually destroys the current cotext, to ensure any pending or further
> > operation on the compositing run loop returns early in !glContext(). The
> > updateViewport is to ensure we re-schedule a display on next frame for the
> > new handle, since we have cancelled any previous one. It's probably not
> > needed in the case of null handle.
> > 
> 
> In case the handle is null you don't need to call the modified
> ensureGLContext(), it should be enough to just destroy the current GLContext
> on the spot. And the update should only be issued in case of a non-null
> handle -- if it's null, renderLayerTree() will bail out early anyway because
> of an inactive scene.

The update is not actually needed in any case, I've just removed it. When the handle is null, ensureGLContext() simply destroys the current context. I've renamed it to tryEnsureGLContext() to make it clear that it can fail, and I've added a comment there explaining what happens. Also added an assert to ensure that we always destroy the current native handle before setting a new one.

> > > > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp:202
> > > > +    scheduleLayerFlush();
> > > 
> > > Why cancel and then re-schedule the layer flush? Just re-scheduling should
> > > be enough.
> > 
> > Since setting the new handler on the compositor is sync now, I want to make
> > sure we cancel any scheduled layer flush before actually changing the
> > handler, and then scheduling a new one once we have the new handler. That's
> > exactly what we do in LayerTreeHostGtk too.
> 
> This method is called on the main thread, just like the layer flushes. You
> shouldn't have to worry about spurious layer flushes occurring while
> executing this method.

hmm, you are right, this is all in the main thread, so it's not possible that a layer flush happens until the new handle is updated, so if there's on scheduled, we don't need to do anything, it will happen after setting the new handler.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160614/4bdbfe5c/attachment-0001.html>