Created attachment 70462 [details] Patch

Attachment 70462 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4324027

Created attachment 70464 [details] Patch fixing Chromium build

Attachment 70464 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4355019

Created attachment 70466 [details] Patch with more Chromium fixes and added m_context null checks to DrawingBuffer functions

Attachment 70466 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4343025

Created attachment 70468 [details] Patch

Created attachment 70469 [details] Patch adding removeDrawingBuffer calls and fixing more Chromium build problems

Created attachment 70473 [details] Patch adding protection against null m_context in DrawingBuffer ctors

Attachment 70469 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4369015

Attachment 70473 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4336025

Created attachment 70480 [details] Patch fixing another Chromium build issue

Created attachment 70483 [details] Patch fixing merge conflicts

Attachment 70483 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4304034

Created attachment 70537 [details] Patch

Attachment 70537 [details] did not build on mac: Build output: http://queues.webkit.org/results/4286037

Attachment 70537 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4289034

Created attachment 70542 [details] Patch gets rid of const on platformLayer() so Chromium builds

Created attachment 70571 [details] Patch using ref counting rather than weak pointers (please hold review)

Comment on attachment 70571 [details] Patch using ref counting rather than weak pointers (please hold review) View in context: https://bugs.webkit.org/attachment.cgi?id=70571&action=review > WebCore/platform/graphics/GraphicsContext3D.h:442 > + static PassRefPtr<GraphicsContext3D> create(Attributes attrs, HostWindow* hostWindow, RenderStyle renderStyle = RenderOffscreen); The argument names here are not helpful and should be omitted. > WebCore/platform/graphics/gpu/DrawingBuffer.cpp:42 > + return (drawingBuffer->m_context) ? drawingBuffer : 0; Should be drawingBuffer.release() rather than drawingBuffer. > WebCore/platform/graphics/gpu/DrawingBuffer.cpp:55 > + m_context = 0; Should use clear() to clear RefPtr rather than assignment to 0. > WebCore/platform/graphics/gpu/DrawingBuffer.h:65 > + // Clear all resources from this object, as well as context. Called when context is destroyed > + // to prevent invalid accesses to the resources. > + void clear(); The first line of the comment is indented incorrectly. > WebCore/platform/graphics/gpu/DrawingBuffer.h:88 > +protected: > + static PassRefPtr<DrawingBuffer> create(PassRefPtr<GraphicsContext3D>, const IntSize&); Why protected rather than private? > WebCore/platform/graphics/gpu/SharedGraphicsContext3D.cpp:64 > - return adoptRef(new SharedGraphicsContext3D(context.release(), solidFillShader.release(), texShader.release())); > + return adoptRef(new SharedGraphicsContext3D(context, solidFillShader.release(), texShader.release())); This need not be changed: A call to release() here is still a good idea. It will avoid a bit of reference count churn. > WebCore/platform/graphics/gpu/mac/DrawingBufferMac.mm:63 > + if (!m_context) > + return; > + > + clear(); Since the clear() function is already safe to call if m_context is 0, I suggest omitting the if statement and early return here. > WebCore/platform/graphics/mac/GraphicsContext3DMac.mm:86 > - OwnPtr<GraphicsContext3D> context(new GraphicsContext3D(attrs, hostWindow, false)); > - return context->m_contextObj ? context.release() : 0; > + RefPtr<GraphicsContext3D> context = adoptRef(new GraphicsContext3D(attrs, hostWindow, false)); > + return context->m_contextObj ? context : 0; No reason to remove the release() call here. Including it reduces the reference count churn a bit.

(In reply to comment #20) > ... > > WebCore/platform/graphics/gpu/DrawingBuffer.h:88 > > +protected: > > + static PassRefPtr<DrawingBuffer> create(PassRefPtr<GraphicsContext3D>, const IntSize&); > > Why protected rather than private? This create method is used by GraphicsContext3D, which is a friend, so it has to be protected.

(In reply to comment #21) > (In reply to comment #20) > > ... > > > WebCore/platform/graphics/gpu/DrawingBuffer.h:88 > > > +protected: > > > + static PassRefPtr<DrawingBuffer> create(PassRefPtr<GraphicsContext3D>, const IntSize&); > > > > Why protected rather than private? > > This create method is used by GraphicsContext3D, which is a friend, so it has to be protected. I realize now that friends can access private members. So I will make this change. I'm not sure where I got the outdated notion that friends could only access protected members.

Landed in http://trac.webkit.org/changeset/69619

http://trac.webkit.org/changeset/69619 might have broken Chromium Win Release

Attachment 70571 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4392002