466495 - Mac OS X plugins can't draw if they explicitly set windowless

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Josh Aas]

All Mac OS X NPAPI plugins are windowless. Gecko does/should assume Mac OS X plugins are windowless, and because of that plugins generally don't say they are windowless explicitly.

It should be safe, though, for plugins to say that they are windowless:

NPP_New(...) {
  NPN_SetValue(instance, NPPVpluginWindowBool, NULL);
}

The problem is that this prevents plugins from being able to draw on Mac OS X. I did some logging and without that code, "mPluginWindow->type" in the nsObjectFrame is set to windowed, not windowless. This seems wrong. When I use the code above to set windowless explicitly, the window type is set to "drawable", which is windowless, which seems like the value we should always have whether or not the plugin sets that explicitly.

Comment 1

[Josh Aas]

WARNING: null plugin port during PaintPlugin: file /Users/josh/src/mozilla/ff_191_debug/layout/generic/nsObjectFrame.cpp, line 1399

That warning gets printed out when a plugin instance is loaded that sets windowless explicitly.

Comment 2

[Steven Michaud [:smichaud] (Retired)]

I tested this by (among other things) adding the following line
near the end of the NPP_New() method (in main.mm) of my
DebugEventsPlugin from bug 441880:

    browser.setvalue(instance, NPPVpluginWindowBool, NULL);

I confirmed that loading this (altered) plugin causes the error return
Josh reported in comment #1 in recent mozilla-central nightlies.  I
also found that it crashes FF 3.0.4!

bp-ef432917-867f-49fe-aa94-0f2e92081124

In both cases, the cause is the same:  Making a plugin "windowless"
means (among other things) that no widget gets created for it (in
nsObjectFrame::CreateWidget()).  On OS X this makes it impossible to
draw the plugin.  More specifically, this makes
nsObjectFrame::GetPluginPort() return NULL, which in FF 3.0.4 causes a
NULL pointer dereference.

Comment 3

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix (stop supporting NPPVpluginWindowBool on OS X)

The windowed/windowless setting is meaningless on OS X:

On OS X a plugin is always windowed (it always has a native "window"
-- i.e. a widget and a "plugin port" (an nsPluginPort object)).  But
in other respects it behaves like a windowless plugin does on Windows
and Linux -- all events are sent to it via NPP_HandleEvent().  (See
https://developer.mozilla.org/En/NPN_SetValue#section_5 for a
description of what windowed and windowless mean on Windows and
Linux/Unix.)

As we've seen above, without a widget (and a plugin port) it's not
possible to draw a plugin on OS X.  Substantial changes would be
required to make a "windowless" plugin have a "window" on OS X (as
opposed to Windows or Linux).  Rather than doing this, I think it's
best to acknowledge that OS X doesn't support the windowed/windowless
settings.

That's what my patch does.

A tryserver build will follow in an hour or two.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

Interestingly, Safari and Opera, though they (apparently) ignore calls
to set NPPVpluginWindowBool true or false, don't return errors on
these calls.  My patch from comment #3 does return an error.  But I
suppose we could just ignore calls (from the plugin) to set
NPPVpluginWindowBool, without returning an error.

Comment 5

[Steven Michaud [:smichaud] (Retired)]

Here's a tryserver build made with my patch from comment #3:

https://build.mozilla.org/tryserver-builds/2008-11-25_12:[e-mail]-bugzilla466495/[e-mail]

Comment 6

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #4)

Safari doesn't support a call to get the NPNVSupportsWindowless
setting -- in effect NPNVSupportsWindowless is false for Safari.  But
Opera (9.51) says NPNVSupportsWindowless is true ... though (as I said
above) setting NPPVpluginWindowBool to true or false appears to make
no more difference on Opera than it does on Safari.

Comment 7

[Josh Aas]

Comment on attachment 350071 [details] [diff] [review]
Fix (stop supporting NPPVpluginWindowBool on OS X)

>   case NPNVSupportsWindowless: {
>-#if defined(XP_WIN) || defined(XP_MACOSX) || (defined(MOZ_X11) && defined(MOZ_WIDGET_GTK2))
>+#if defined(XP_WIN) || (defined(MOZ_X11) && defined(MOZ_WIDGET_GTK2))
>     *(NPBool*)result = PR_TRUE;

I'm not sure we should do this. It really doesn't matter what we do here and if we change it there's a better chance we'll trip up existing plugins if we change it. Strictly speaking we do support the windowless API even if we don't support setting windowless via NPN_SetValue.

>+#ifdef XP_MACOSX
>+      // This setting is meaningless on OS X:  On OS X the plugin is
>+      // always in windowed mode (it always has a native "window" -- a
>+      // widget and a "plugin port" (an nsPluginPort object)).  But (as
>+      // with windowless mode on Windows and Linux) all events are sent
>+      // to it via NPP_HandleEvent.  See
>+      // https://developer.mozilla.org/En/NPN_SetValue#section_5.
>+      return NPERR_GENERIC_ERROR;

I think if we're going to talk about windowed vs. windowless, we need to be clear that in terms of NPAPI all Mac OS X plugins are windowless - not windowed. Windowless, in NPAPI, refers to the API in use not the implementation detail of whether or not the plugin actually has a view. The sense in which these plugins are windowed is internal to Gecko. Please make that clear in the comment.

I also feel like we should return NP_NO_ERROR here for the sake of compatibility and a single code-path for multi-platform plugins. We can just ignore the call on Mac OS X, as your patch does, and still return NP_NO_ERROR.

Comment 8

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #7)

> I think if we're going to talk about windowed vs. windowless, we
> need to be clear that in terms of NPAPI all Mac OS X plugins are
> windowless - not windowed. Windowless, in NPAPI, refers to the API
> in use not the implementation detail of whether or not the plugin
> actually has a view. The sense in which these plugins are windowed
> is internal to Gecko. Please make that clear in the comment.

If what https://developer.mozilla.org/En/NPN_SetValue#section_5 has to
say about NPPVpluginWindowBool is any guide, all Mac OS X plugins are
windowed, though they do have some characteristics of windowless
plugins (as I explained in comment #3).

Can you tell me where to find documentation on NPPVpluginWindowBool
that's more up to date than
https://developer.mozilla.org/En/NPN_SetValue#section_5?

Comment 9

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix rev1 (follow most of Josh's suggestions)

Josh and I don't agree about what "windowed" and "windowless" mean.  But we
do agree that the NPPVpluginWindowBool setting doesn't (currently) apply to
OS X -- it's only relevant on Windows and Unix/Linux (as per
https://developer.mozilla.org/En/NPN_SetValue#section_5).

I also now agree that it's best for us to treat NPPVpluginWindowBool the
same way Safari and Opera do on OS X -- ignore any attempts to set it, but
don't return an error.

A new tryserver build will follow in a while.

Comment 10

[Steven Michaud [:smichaud] (Retired)]

Here's a tryserver build made with my rev1 patch:

https://build.mozilla.org/tryserver-builds/2008-12-02_13:[e-mail]-bugzilla466495-rev1/[e-mail]

Comment 11

[Steven Michaud [:smichaud] (Retired)]

Rev1 patch landed on mozilla-central (changeset 2b1ea6fad542).