Bug 80343 – [Chromium] NOT FOR REVIEW: Support for Browser Plugin

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

[Fady Samuel]

[Chromium] Support for Browser Plugin

[Fady Samuel]

Created attachment 130221 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

Please wait for approval from [e-mail], [e-mail] or [e-mail] before submitting because this patch contains changes to the Chromium platform API.

[James Robinson]

Comment on attachment 130221 [details]
Patch

What is this?

[Adam Barth]

Comment on attachment 130221 [details]
Patch

The bug title says this isn't meant for review.  Note: You can submit patches to the EWS without marking them for review by clicking the "submit to EWS" button that appears next to patches when they haven't been submitted to the EWS.

[Darin Fisher (:fishd, Google)]

I was hoping this work wouldn't require WebKit changes.  Does this patch represent all of the changes?  If not, maybe we could sync up over email, or you could point me at a document describing the design.

[Fady Samuel]

(In reply to comment #5)
> I was hoping this work wouldn't require WebKit changes.  Does this patch represent all of the changes?  If not, maybe we could sync up over email, or you could point me at a document describing the design.

This is all of it, yes.

[Fady Samuel]

Created attachment 130383 [details]
Patch

[Fady Samuel]

(In reply to comment #5)
> I was hoping this work wouldn't require WebKit changes.  Does this patch represent all of the changes?  If not, maybe we could sync up over email, or you could point me at a document describing the design.

I've just reduced the number of lines of change here, and I'm continuing to tweak the chromium side changes.

[Adam Barth]

Comment on attachment 130383 [details]
Patch

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

> Source/Platform/chromium/public/WebGraphicsContext3D.h:150
> +    virtual void playDead() { }

This function doesn't appear to be implemented or called anywhere.

[Fady Samuel]

(In reply to comment #9)
> (From update of attachment 130383 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=130383&action=review
> 
> > Source/Platform/chromium/public/WebGraphicsContext3D.h:150
> > +    virtual void playDead() { }
> 
> This function doesn't appear to be implemented or called anywhere.

It's implemented outside of WebKit. This is another patch-in-progress. The name is a bit silly and may change or this may go away completely. I'm still iterating.

[James Robinson]

You should not be modifying WebKit at all for this.  To control what context the compositor is using for a given WebView, override the implementation of WebViewClient::createGraphicsContext3D()

[Fady Samuel]

(In reply to comment #11)
> You should not be modifying WebKit at all for this.  To control what context the compositor is using for a given WebView, override the implementation of WebViewClient::createGraphicsContext3D()

Trying that out now. I've been prototyping locally for a while, and things have changed quite a bit since. Thanks :)

[Fady Samuel]

(In reply to comment #11)
> You should not be modifying WebKit at all for this.  To control what context the compositor is using for a given WebView, override the implementation of WebViewClient::createGraphicsContext3D()

You are absolutely right! Sweet thanks! I love reducing lines of code on cls. :-)

[Fady Samuel]

Created attachment 130405 [details]
Patch

[James Robinson]

Comment on attachment 130405 [details]
Patch

...and if you are doing that then you don't need the playDead() either, since you can refer to the more-concrete type from chromium land.

[Fady Samuel]

(In reply to comment #15)
> (From update of attachment 130405 [details])
> ...and if you are doing that then you don't need the playDead() either, since you can refer to the more-concrete type from chromium land.

The purpose of "playDead" is to tell the WebGraphicsContext3D to pretend that the context is lost, and so Chromium will attempt to create a new graphics context in place of the existing one. There might be a better way to accomplish what I'm trying to accomplish here.

[Fady Samuel]

There is no longer any need for WebKit changes to support the browser plugin except for this which is now landing: https://bugs.webkit.org/show_bug.cgi?id=81954