Bug 91793 – [chromium] Use CCGraphicsContext in LayerRenderChromium

• Status: RESOLVED
• Resolution: INVALID
• Component: New Bugs

[Brian Anderson]

[chromium] Use CCGraphicsContext in LayerRenderChromium

[Brian Anderson]

Created attachment 153365 [details]
Patch

[Nat Duca]

Comment on attachment 153365 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=153365&action=review

Any idea why the patch isn't applying cleanly? LGTM with nits, @enne for review.

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:136
> +PassOwnPtr<LayerRendererChromium> LayerRendererChromium::create(CCRendererClient* client, WebCore::CCGraphicsContext* context, TextureUploaderOption textureUploaderSetting)

Why does this got the webcore namespace on it?

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:1442
>  bool LayerRendererChromium::makeContextCurrent()

/me wonders out loud why we have this function in the first place? Does anyone call it aside from the first time?

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:1444
> +    // We call CCGraphcisContext's makeContextCurrent so it can lazily initialize.

Not sure this comment adds value.

> Source/WebCore/platform/graphics/chromium/cc/CCGraphicsContext.h:54
> +            lazyInitialize();

I'd leave this part out. E.g. pass directly through to
 m_context3D->makeContextCurrent();

The patch to lazyInit can be in your other patch.

Also, I'd say initializeIfNeeded() and use an m_initialized to ensure that makeContextCurrent will only happen once. The makeCurrent code returns true even if you call it twice in a row, i think.

[Adrienne Walker]

Comment on attachment 153365 [details]
Patch

I don't have anything to add over Nat's comments.  Fix those, get this applying and green on the EWS bots, and I'll take a final look.

[Brian Anderson]

Looks like https://bugs.webkit.org/show_bug.cgi?id=91044 just went in, which resulted in a lot of conflicts.  Will rebase and address comments.

[Brian Anderson]

Turns out the patch for bug 91044 completely invalidates the need for this patch.  Closing.  A simpler shallowFlush patch will be coming through shortly.

[Adam Barth]

Comment on attachment 153365 [details]
Patch

Cleared review? from attachment 153365 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).