505135 - NPN_InvalidateRect does nothing on secondary threads beginning in Gecko 1.9.0.x

Status: RESOLVED WONTFIX

(Core Graveyard :: Plug-ins, defect)

Description

[Kevin Shepherd]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.5) Gecko/2008121622 Ubuntu/8.04 (hardy) Firefox/3.0.5
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5

I am a plugin developer.  Everything works on all Firefox platforms ( Linux 32/64 Win 32/64 Mac ppc/x86 ) except NPN_InvalidateRect on Firefox / Mac OS X.  Neither does NPN_ForceRedraw, but I do not need it.
The strange thing is that SeaMonkey works fine.
My plugin will be installable from http://www.scarletline.com/npcml/download.html from ~Jul 22 2009.  Any example plugin on the Mac will also reproduce the bug.

Reproducible: Always

Steps to Reproduce:
1.Install Plugin
2.View a molecule
3.You will need to use the mouse to resize the window to see the molecule.
Actual Results:  
Black rectangle appears where the molecule should be.  Only appears when you drag-resize the window.

Expected Results:  
On all other platforms, and other browsers on the Mac, the molecule appears automatically.

There is unfortunately no work-around.   I have tried many tricks.

Comment 1

[Kevin Shepherd]

Reproduced on v3.0.11 and v3.5 on Mac OS X PowerPC

Comment 2

[Matthias Versen [:Matti]]

Addon/extension!=Plugin

Which Seamonkey Do you mean ?
The SM1.1.X is still based on Gecko 1.8 (=Firefox2) while Firefox3 and the coming Seamonkey2.0 are based on Gecko 1.9.X

Comment 3

[Kevin Shepherd]

SeaMonkey v1.1.16

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Presumably this also worked fine in Firefox 2.0.0.x; does it work in Camino 1.6.x, or is it broken there, too?  (The answers to those determine whether it's a regression in Firefox from the switch to Cocoa, or a regression between Gecko 1.8.1 and 1.9.0.)

Also, have you tested on Intel?  There's at least one common drawing bug that is fixed on Intel and seems to still exist on PPC (bug 334321, though this is clearly a different description).

Comment 5

[Kevin Shepherd]

I do not think that this is related to bug 334321.  I do not have access to an Intel Mac OS X.  I will be testing this on Wednesday on ppc Mac OS X Firefox 2, SeaMonkey 2, Opera, Camino, OmniWeb, iCab and Shiira and I will post the exact version numbers and the results.

Comment 6

[Kevin Shepherd]

Here are the results of testing.  It looks like the comments about Gecko version were dead on.

Working:
Safari v4.0.2
SeaMonkey v1.1.16
Opera v9.64
Camino v1.6.7
OmniWeb v5.9.2
iCab v4.5.0
Firefox v2.0.0.20

Not Working:
Firefox v3.0.11
Firefox v3.5.0
SeaMonkey v2.0b1

NPP_SetWindow initially providing wrong window port position:
Camino v1.6.7
Firefox v3.0.11
Firefox v3.5.0
SeaMonkey v2.0b1

There are two bugs here, and I think that the above shows when each was introduced to Gecko.  The first is mild:
NPP_SetWindow initially provides the wrong port co-ordinates, but gives the correct ones later, after a window re-size.  This bug is shared by Camino v1.6.x, Firefox v3.x and SeaMonkey v2.x
The second is more serious:
NPN_InvalidateRect ( and NPN_ForceRedraw, and presumably NPN_InvalidateRegion ) does nothing at all.  Again the workaraound is to change the size of the window using the mouse, thus forcing a redraw.  This bug is shared by Firefox v3.x and SeaMonkey v2.x
Thank you to Matthias and Smokey for pointing to the tests that have shown this up.

Comment 7

[Kevin Shepherd]

Here are the Gecko versions of the various browser builds:
Gecko versions:
Camino v1.6.8	Gecko v1.8.1.22
Camino v1.6.7	Gecko v1.8.1.21
Camino v1.6.6	Gecko v1.8.1.19
Camino v1.6.5	Gecko v1.8.1.18
Camino v1.6.4	Gecko v1.8.1.17
Camino v1.6.3	Gecko v1.8.1.16
Camino v1.6.2	Gecko v1.8.1.15
SeaMonkey v1.0.x	Gecko v1.8.0.x
SeaMonkey v1.1.x	Gecko v1.8.1.x
SeaMonkey v1.1.16	Gecko v1.8.1.21
SeaMonkey v1.1.17	Gecko v1.8.1.22
SeaMonkey v2.0.x	Gecko v1.9.x
Firefox v2.x	Gecko v1.8.1
Firefox v2.0.0.20	Gecko v1.8.1.20
Firefox v3.x	Gecko v1.9.x
Firefox v3.6.x	Gecko v1.9.2.x

Based on this, I tested SeaMonkey v1.1.17 - which worked perfectly on both window position and InvalidateRect.

Therefore I conclude that the "NPN_InvalidateRect does nothing" bug was introduced in Gecko v1.9.0 and that the "NPP_SetWindow wrong port co-ordinates" is at least partially browser specific ( Camino with Gecko v1.8.1.21 has it, but SeaMonkey with the same Gecko version does not ).

I welcome input from any developers familiar with changes in the NPN_InvalidateRect code introduced in Gecko v1.9.0

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #7)
> and that the "NPP_SetWindow wrong port co-ordinates"
> is at least partially browser specific ( Camino with Gecko v1.8.1.21 has it,
> but SeaMonkey with the same Gecko version does not ).

This indicates that this particular bug has been latent in Cocoa widgets (which Camino has always used, but which Firefox and SeaMonkey only started using with Fx3 and Sm2). It's best to file a second bug on this part and keep the current bug 505135 focused on "NPN_InvalidateRect does nothing beginning in Gecko v1.9.0."  (There is/was a similar-sounding bug about the port out there somewhere, an old one filed against Camino long ago, but I can't find it now.)

Comment 9

[Steven Michaud [:smichaud] (Retired)]

I can't confirm this report.  As best I can tell, it's simply not true
that NPN_InvalidateRect does nothing in Gecko 1.9.0.X.

Here's how I tested.  To follow these steps yourself, you first need
to download my DebugEventsPlugin from bug 441880.

1) Copy DebugEventsPlugin.plugin from DebugEventsPlugin/build/Debug in
   the distro to /Library/Internet Plug-Ins.

2) Load test.html from the distro into Firefox (I used Firefox 3.5.1),
   click on the plugin to focus it, and start pressing and releasing
   keys (e.g. 'a').

   Notice that the plugin's display changes as you press and release
   each key.

3) Edit the plugin's main.mm to comment out the call to
   browser.invalidaterect() in invalidatePlugin():

   static void invalidatePlugin(PluginObject *obj)
   {
       NPRect rect;
       rect.left = 0;
       rect.top = 0;
       rect.right = obj->window.width;
       rect.bottom = obj->window.height;

       //browser.invalidaterect(obj->npp, &rect);
   }

   Note that this is the only call in the plugin to
   browser.invalidaterect().

4) Rebuild the plugin, and copy the rebuilt DebugEventsPlugin.plugin
   to /Library/Internet Plug-Ins/.

5) Restart FF and reload test.html from the plugin's distro.  Click on
   the plugin to focus it, and start pressing and releasing keys.

   Notice that the plugin's display no longer changes when you do this.

Kevin:  You may have found a problem with NPN_InvalidateRect that only
occurs in one or more special cases.  In order for us to be able to
fix this problem (if it exists), you'll need to write a testcase
plugin that demonstrates the problem and attach the sourcecode here.
It would be most convenient if you attached it as part of an XCode
project.

Comment 10

[Kevin Shepherd]

Thank you to both Smokey and Steven for your advice. 

I have added my NPP_SetWindow findings to Bug 474491 which seems to be reporting the same problem.

I will test with DebugEventsPlugin to see if I can reproduce the bug with it.  I will also be tracing a debug build of Firefox from my plugin to see what actually happens when I call NPN_InvalidateRect.

Comment 11

[Kevin Shepherd]

I have not yet persuaded DebugEventsPlugin to reproduce this problem.  The fact that the bug exists is beyond doubt - simply open a Firefox 2 and a Firefox 3 browser side by side on any Mac OS X.  The Firefox 2 will update automatically, and the Firefox 3 will not.
I am still working on this.  Any other plugin developers who have seen the same problem, please report it here.

Comment 12

[Kevin Shepherd]

I can now reliably reproduce the bug using a modified DebugEventsPlugin.  Thank you Steven for your excellent code example, it saved me a lot of time.

I have tested with Safari, Firefox v2.x, Firefox v3.x, Seamonkey v1.x and SeaMonkey v2.x and the modified plugin fails and works in exactly the same way as the bug I am experiencing.

I am uploading the modified DebugEventsPlugin to this bugzilla page.

Simply stated, on Firefox v3.x and SeaMonkey v2.x NPN_InvalidateRect does nothing if not called from the main thread, or while the plugin window does not have focus.  On all other browsers ( Firefox v2.x, SeaMonkey v1.x, Safari, Opera, OmniWeb, iCab, ..... ) it works.

Comment 13

[Kevin Shepherd]

Attachment: Complete compiled debug plugin with full source and project which always reproduces the bug.

Comment 14

[Kevin Shepherd]

Steps to reproduce:
1) Open the zip file somewhere, e.g. your home directory.  You will then have:
/Users/xxxx/DebugEventsPlugin/ and sub-dirs
2) Copy (cp) or link (ln -s) /Users/xxxx/DebugEventsPlugin/build/Debug/DebugEventsPlugin.plugin
to /Library/Internet\ Plug-Ins/DebugEventsPlugin.plugin
If you link, it saves time in the re-compile test cycle, but you must remember to Quit all browsers before building.
3) Start /Users/xxxx/DebugEventsPlugin/test.html by right click in the Finder - then you can use "Open With" to choose which of your installed browsers that you wish to test.
4) Do nothing for four seconds.  Do not click the browser to focus or anything else.  The plugin window should remain a grey square for four seconds, then it will display some text and a count every four seconds ( unless you are using FF 3 or SM 2 )
If you wish to see the debug output from the plugin, then run your test browser from a terminal, e.g. for Firefox, type:
/Applications/Firefox.app/Contents/MacOS/firefox-bin

Comment 15

[Kevin Shepherd]

I have now been able to trace the NPN_InvalidateRect call through a debug build of Firefox.  It ends up in function _invalidaterect in file:

mozilla-1.9.1/modules/plugin/base/src/nsNPAPIPlugin.cpp (ff v3.5.2)
mozilla/modules/plugin/base/src/ns4xPlugin.cpp (ff v3.0.9)

which returns immediately because !NS_IsMainThread() is true.

Therefore this NPN function ( and most others ) do not support threading.  This downgrade in capability from Firefox v2.x, and from all other NPAPI supporting browsers, needs to be documented at https://developer.mozilla.org/en/Plugins
( or fixed ).
The NPN_InvalidateRect function needs to be thread capable on the Mac OSX platform because otherwise there can be no background processing in a plugin, so any slow steps will simply stall the browser.  This is not true on any other platform because there the plugin has it's own window and is not hostage to the limitations imposed by NPN implementations.

Comment 16

[Steven Michaud [:smichaud] (Retired)]

Since most OS-level calls aren't thread-safe (at least those having to
do with display), it shouldn't be surprising that most NPAPI calls
need to be made on the main thread.

I suspect the fact that NPN_InvalidateRect() works in FF2 on a
secondary thread is simply an oversight.  As for Safari, I know from
personal experience that it does lots of things on secondary threads
that other browsers do only on the main thread -- so Safari may be
something of a special case.

I'll try to find time to look into this further -- maybe later this
week.  Since NPN_InvalidateRect() in effect just sets a flag (telling
the OS that a given part of the window needs updating), it might be
possible/reasonable to make an exception in this case to the general
rule that NPAPI calls should only be made on the main thread.

But I suspect this change won't be easy, and even if it's easy it may
be rejected for reasons of overall design.

So I strongly suggest finding ways to avoid calling
NPN_InvalidateRect() on secondary threads.

If we truly haven't documented the need to make NPAPI calls on the
main thread (and I haven't yet looked carefully), we definitely need
to do that.

Comment 17

[Steven Michaud [:smichaud] (Retired)]

Eric, do you know if our NPAPI docs say you need to make NPAPI

Comment 18

[Steven Michaud [:smichaud] (Retired)]

Sorry, lets try again:

Eric, do you know if our NPAPI docs say you need to make NPAPI calls on the main thread?

Comment 19

[Eric Shepherd [:sheppy]]

No, but there's now a warning message here:

https://developer.mozilla.org/en/Gecko_Plugin_API_Reference/Browser_Side_Plug-in_API

Comment 20

[Steven Michaud [:smichaud] (Retired)]

Thanks, Eric!

Comment 21

[Kevin Shepherd]

Thank you, Eric, for the documentation update.  In reading it I noticed the Gecko 1.9 addition "NPN_PluginThreadAsyncCall" which I plan to use to attempt a workaround to this problem.

Steven, I disagree with your attempt to marginalize this.  My modified version of your excellent DebugEventsPlugin works on every browser on Mac OSX ( this NPN function is Mac only ) but for the two Gecko v1.9 ones ( FF3 and SM2beta ).  That list includes:
Safari v4.0.2
SeaMonkey v1.1.16
Opera v9.64
Camino v1.6.7
OmniWeb v5.9.2
iCab v4.5.0
Firefox v2.0.0.20

Modern OS-calls are thread safe since Linux kernel v2.6, Mac OSX, Windows NT.
There are some display calls that you may wish to mutex around (such as XCB on Linux), but the equivalent of PostMessage(WM_PAINT) is not one of these. ( I can see how NPN_ForceRedraw could be a problem ).

Thank you to everyone who has been so helpful here.  If my workaround works, I will post the relevant code here - though I still consider that it would be sensible to allow NPN_InvalidateRect to work in a threaded environment as it does in all other browsers.

Comment 22

[Eric Shepherd [:sheppy]]

I just realized that I need to add that warning to each individual function's doc as well.

Comment 23

[Steven Michaud [:smichaud] (Retired)]

> Modern OS-calls are thread safe since Linux kernel v2.6, Mac OSX,
> Windows NT.  There are some display calls that you may wish to mutex
> around (such as XCB on Linux), but the equivalent of
> PostMessage(WM_PAINT) is not one of these. ( I can see how
> NPN_ForceRedraw could be a problem ).

On OS X, almost all Cocoa methods having to do with display must be
called on the main thread.  I don't know much about Windows or Linux
in this regard, but what you say here implies this is also true of at
least some display methods on Windows and Linux.

> My modified version of your excellent DebugEventsPlugin works on
> every browser on Mac OSX ( this NPN function is Mac only ) but for
> the two Gecko v1.9 ones ( FF3 and SM2beta ).

I assume you only tested NPN_InvalidateRect().

As I said above, there might be grounds for allowing this (and
possibly a few other methods) to be used on secondary threads.  But I
suspect you'd get into trouble in most browsers trying to use methods
that trigger synchronous drawing from anything else than the main
thread -- even if the browser allowed you do to do that.

Comment 24

[Kevin Shepherd]

I have tested a workaround for this bug which allows NPN_InvalidateRect to work on all Mac OSX browsers that I have been able to test (the nine previously listed browsers and versions).  I post the code here in the hopes that it will help other plugin developers who have hit the same problem:

In the plugin initialization code:
#ifdef XP_MACOSX
	if (strstr(NPN_UserAgent(mInstance), " rv:1.9."))
		_M_is_invalidaterect_threaded_broken = true;
#endif // Mac OS X

// The async function:
void Async_InvalidateRect(void * plugin)
	{
	NPN_InvalidateRect( ((nsPluginInstance *)plugin)->getInstance(), ((nsPluginInstance *)plugin)->get_async_invalid_rect() );
	}

To call NPN_InvalidateRect:
        ...
	NPRect invalidRect;
	invalidRect.top = 0; //y;
	invalidRect.left = 0; //x;
	invalidRect.bottom = height; // y + height;
	invalidRect.right = width; // x + width;
	if (_M_is_invalidaterect_threaded_broken)
		{ // i.e. Firefox v3.x, SeaMonkey v2.x on Mac OS X
		set_async_invalid_rect( & invalidRect);
		NPN_PluginThreadAsyncCall(mInstance, Async_InvalidateRect, this);
                }
        else
                NPN_InvalidateRect(mInstance, & invalidRect);

        ...


Note that set/get async_invalid_rect maintain a copy of the NPRect passed to them, not a pointer.