Linux plugins WIP.

Get windowed plugins (Flash) limping along on Linux.

We still crash when you navigate away from the page.

(PS: the plan is to unfork the _gtk.cc file once it gets closer to what we want, so you don't need to look at that too closely.  I just wanted to check in what I have since it's getting big.)

[Evan Martin, 2009-01-28 01:43:12]

This is my current work on plugins.  Jumping to a new branch to refactor the
wacky ReadPluginInfo API.

[jam, 2009-01-28 01:47:49]

we definitely don't want to fork webplugin_delegate_impl...

[Evan Martin, 2009-01-28 01:58:56]

On 2009/01/28 01:47:49, John Abd-El-Malek wrote:
> we definitely don't want to fork webplugin_delegate_impl...

Yeah, my plan is to get it working and then remerge.  I'm being careful to not
reorder the functions, etc.
Otherwise you end up with premature abstractions.

[Evan Martin, 2009-01-29 23:27:11]

+awalker fyi

This now successfully displays flash, though it's positioned slightly wrong and
it crashes when you leave the page.

[Evan Martin, 2009-01-30 02:06:23]

After some other changes I've submitted, this diff now getting smaller.

PS: To get the flash on the test_shell page to display, you have to wiggle the
scroll bar.  YouTube doesn't work because I don't do windowless yet.

[jam, 2009-01-30 03:39:18]

It might be better to svn copy webplugin_delegate_impl* so that the patch shows
the diff as compared to the original..  other than these files, the rest lgtm

[Amanda Walker, 2009-01-30 04:45:14]

On 2009/01/30 03:39:18, John Abd-El-Malek wrote:
> It might be better to svn copy webplugin_delegate_impl* so that the patch
shows
> the diff as compared to the original..  other than these files, the rest lgtm

Agreed-there are several places we may have to do this, so keeping the history
in svn is good.  otherwise, lgtm

[Evan Martin, 2009-02-02 23:27:25]

Ok, sorry for the repeated mails, but now I think it's ready for checkin pending
a trybot run.

(All that works right now is windowed flash and it's not reliable.)

I've now unforked the header file, but left the cc file forked as a temporary
thing; I intend to unfork it once I have windowless plugins working.  When I
unfork the cc file, you can look at the diff there to see what I've changed.

[Dean McNamee, 2009-02-03 10:26:07]

Some minor comments.  It's hard to review the forked file.  Besides that, it's
pretty slashy, but thats sort of what needs to happen now.  Looks Gnarly To me.

http://codereview.chromium.org/19413/diff/73/74
File third_party/npapi/bindings/npapi.h (right):

http://codereview.chromium.org/19413/diff/73/74#newcode107
Line 107: // This is the Chrome-specific define for Linux.  Map it into the
Mozilla one.
This comment is a bit bad.  We know it's Chrome-specific since it's in the
GOOGLE MODIFICATIONS.

Just say like

"On Linux, make sure to define the Mozilla UNIX platform flag"

Or something

http://codereview.chromium.org/19413/diff/73/76
File webkit/glue/plugins/plugin_host.cc (right):

http://codereview.chromium.org/19413/diff/73/76#newcode16
Line 16: #include "config.h"
should we have like a wrapper around npapi.h for this sort of thing?  I guess we
need to include it two ways, sounds like a bunch of pulling apart would need to
happen, blah.

http://codereview.chromium.org/19413/diff/73/77
File webkit/glue/plugins/webplugin_delegate_impl.h (right):

http://codereview.chromium.org/19413/diff/73/77#newcode39
Line 39: #endif // OS(WIN)
two spaces before //, and it's OS_WIN not OS(WIN)

http://codereview.chromium.org/19413/diff/73/77#newcode210
Line 210: gfx::NativeWindow parent_;
What's NativeView vs NativeWindow?

[Dean McNamee, 2009-02-03 15:37:13]

Before you commit, make sure to update the description.  I guess just try to
mention that what we have now is some really basic hacky/stubbed out code, which
lets flash load and sort of paint.  And mention all of the things we don't
handle yet (windowless, etc).

Thanks

On 2009/02/03 10:26:07, Dean McNamee wrote:
> Some minor comments.  It's hard to review the forked file.  Besides that, it's
> pretty slashy, but thats sort of what needs to happen now.  Looks Gnarly To
me.
> 
> http://codereview.chromium.org/19413/diff/73/74
> File third_party/npapi/bindings/npapi.h (right):
> 
> http://codereview.chromium.org/19413/diff/73/74#newcode107
> Line 107: // This is the Chrome-specific define for Linux.  Map it into the
> Mozilla one.
> This comment is a bit bad.  We know it's Chrome-specific since it's in the
> GOOGLE MODIFICATIONS.
> 
> Just say like
> 
> "On Linux, make sure to define the Mozilla UNIX platform flag"
> 
> Or something
> 
> http://codereview.chromium.org/19413/diff/73/76
> File webkit/glue/plugins/plugin_host.cc (right):
> 
> http://codereview.chromium.org/19413/diff/73/76#newcode16
> Line 16: #include "config.h"
> should we have like a wrapper around npapi.h for this sort of thing?  I guess
we
> need to include it two ways, sounds like a bunch of pulling apart would need
to
> happen, blah.
> 
> http://codereview.chromium.org/19413/diff/73/77
> File webkit/glue/plugins/webplugin_delegate_impl.h (right):
> 
> http://codereview.chromium.org/19413/diff/73/77#newcode39
> Line 39: #endif // OS(WIN)
> two spaces before //, and it's OS_WIN not OS(WIN)
> 
> http://codereview.chromium.org/19413/diff/73/77#newcode210
> Line 210: gfx::NativeWindow parent_;
> What's NativeView vs NativeWindow?

[Evan Martin, 2009-02-04 19:16:37]

ok, did what you asked, please look again

http://codereview.chromium.org/19413/diff/73/77
File webkit/glue/plugins/webplugin_delegate_impl.h (right):

http://codereview.chromium.org/19413/diff/73/77#newcode210
Line 210: gfx::NativeWindow parent_;
On 2009/02/03 10:26:08, Dean McNamee wrote:
> What's NativeView vs NativeWindow?

Conceptually, widget versus top-level window.
They're the same on Windows and effectively the same Linux, but different on
Mac.
This should've been NativeView, so good catch.