518924 - NPAPI: Implement out-of-process NPN_Timer and NPN_AsyncCallback

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Filed together because the implementations should be essentially the same.

The first versions can simply enqueue events in the plugin process's event loop.

Eventually we might want to make these "rendezvous" with the content process in order to reduce NPAPI race conditions.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Quick note: the lack of these interfaces isn't what's preventing windowless plugins on X11 from animating.  I have no clue what the real cause is.

Comment 2

[Benjamin Smedberg]

Attachment: Implement NPN_AsyncCallback with no parent synchronization, rev. 1

This implements NPN_AsyncCallback which Java needs to function properly. I'm slightly worried about callbacks firing after we destroy the NPP associated with them, but I'm going to leave that for later.

Comment 3

[Benjamin Smedberg]

NPN_AsyncCallback is http://hg.mozilla.org/mozilla-central/rev/a1022a154520
Still need to do the timer.

Comment 4

[Johnny Stenback (:jst)]

The m-c code does deal with the plugin going away between us scheduling an async callback and us trying to run it, I'd recommend dealing with that in the same way in this case as IIRC we did hit that with Java w/o trying too hard.

Comment 5

[Benjamin Smedberg]

jgriffin, this could use tests too!

Comment 6

[Benjamin Smedberg]

http://hg.mozilla.org/mozilla-central/rev/ee38b6b13c25

Comment 7

[Benjamin Smedberg]

FWIW, there appear to be some tests with test_npapi_timers.xul (part of mochitest-chrome) which this did fix, but AFAICT that doesn't actually test the timer functionality, nor asynccall.

Comment 8

[Jonathan Griffin (:jgriffin)]

Attachment: mochitests

Mochitests for NPN_Schedule/UnscedulerTimer, and NPN_PluginThreadAsyncCall.  There were already some existing timer tests, but I moved them to mochitest-plain (they didn't need to be in chrome) and beefed them up a little.  

The tests all pass with and without OOPP enabled.  But, when OOPP is enabled, and the NPN_PluginThreadAsyncCall test is run, I will sometimes see this output in the console:

WARNING: RPC in-calls have raced!: file c:/mozilla/mozilla-central/src/ipc/glue/RPCChannel.cpp, line 285
WARNING: RPC in-calls have raced!: file c:/mozilla/mozilla-central/src/ipc/glue/RPCChannel.cpp, line 285
  (child won, so we're not deferring)

When this occurs, there follows many instances of this assertion:

###!!! ASSERTION: constructing frames in the middle of a paint: 'mPresContext->mLayoutPhaseCount[eLayoutPhase_Paint] == 0', file c:\mozilla\mozilla-central\src\layout\base\nsPresContext.h, line 1218

which have the following stack trace:

xul!nsAutoLayoutPhase::nsAutoLayoutPhase+0x000000000000002A (c:\mozilla\mozilla-central\src\layout\base\nsprescontext.h, line 1189)
xul!nsCSSFrameConstructor::ContentInserted+0x000000000000002C (c:\mozilla\mozilla-central\src\layout\base\nscssframeconstructor.cpp, line 6459)
xul!PresShell::ContentInserted+0x00000000000000B4 (c:\mozilla\mozilla-central\src\layout\base\nspresshell.cpp, line 4991)
xul!nsNodeUtils::ContentInserted+0x000000000000016C (c:\mozilla\mozilla-central\src\content\base\src\nsnodeutils.cpp, line 160)
xul!nsGenericElement::doInsertChildAt+0x0000000000000508 (c:\mozilla\mozilla-central\src\content\base\src\nsgenericelement.cpp, line 3241)
xul!nsGenericElement::InsertChildAt+0x0000000000000054 (c:\mozilla\mozilla-central\src\content\base\src\nsgenericelement.cpp, line 3170)
xul!nsGenericElement::doReplaceOrInsertBefore+0x0000000000001041 (c:\mozilla\mozilla-central\src\content\base\src\nsgenericelement.cpp, line 3929)
xul!nsGenericElement::InsertBefore+0x0000000000000023 (c:\mozilla\mozilla-central\src\content\base\src\nsgenericelement.cpp, line 3464)
xul!nsHTMLBodyElement::InsertBefore+0x0000000000000018 (c:\mozilla\mozilla-central\src\content\html\content\src\nshtmlbodyelement.cpp, line 93)
xul!nsIDOMNode_InsertBefore+0x0000000000000295 (c:\mozilla\mozilla-central\obj-ff-dbg\js\src\xpconnect\src\dom_quickstubs.cpp, line
4246)
mozjs!js_Interpret+0x000000000000F6E0 (c:\mozilla\mozilla-central\src\js\src\jsops.cpp, line 2263)
mozjs!js_Invoke+0x000000000000091D (c:\mozilla\mozilla-central\src\js\src\jsinterp.cpp, line 1384)
mozjs!js_InternalInvoke+0x0000000000000082 (c:\mozilla\mozilla-central\src\js\src\jsinterp.cpp, line 1439)
mozjs!JS_CallFunctionValue+0x000000000000005D (c:\mozilla\mozilla-central\src\js\src\jsapi.cpp, line 5120)
xul!doInvoke+0x0000000000000343 (c:\mozilla\mozilla-central\src\modules\plugin\base\src\nsjsnpruntime.cpp, line 698)
xul!nsJSObjWrapper::NP_Invoke+0x0000000000000028 (c:\mozilla\mozilla-central\src\modules\plugin\base\src\nsjsnpruntime.cpp, line 724)
xul!mozilla::plugins::parent::_invoke+0x000000000000010A (c:\mozilla\mozilla-central\src\modules\plugin\base\src\nsnpapiplugin.cpp,
line 1631)
xul!mozilla::plugins::PluginScriptableObjectParent::AnswerInvoke+0x0000000000000376 (c:\mozilla\mozilla-central\src\dom\plugins\pluginscriptableobjectparent.cpp, line 837)
xul!mozilla::plugins::PPluginScriptableObjectParent::OnCallReceived+0x000000000000091B (c:\mozilla\mozilla-central\obj-ff-dbg\ipc\ipdl\ppluginscriptableobjectparent.cpp, line 1035)
xul!mozilla::plugins::PPluginModuleParent::OnCallReceived+0x0000000000000066 (c:\mozilla\mozilla-central\obj-ff-dbg\ipc\ipdl\ppluginmoduleparent.cpp, line 421)
xul!mozilla::ipc::RPCChannel::DispatchIncall+0x00000000000000B7 (c:\mozilla\mozilla-central\src\ipc\glue\rpcchannel.cpp, line 347)
xul!mozilla::ipc::RPCChannel::Incall+0x0000000000000235 (c:\mozilla\mozilla-central\src\ipc\glue\rpcchannel.cpp, line 333)
xul!mozilla::ipc::RPCChannel::Call+0x00000000000004FE (c:\mozilla\mozilla-central\src\ipc\glue\rpcchannel.cpp, line 182)
xul!mozilla::plugins::PPluginInstanceParent::CallNPP_HandleEvent+0x00000000000000C4 (c:\mozilla\mozilla-central\obj-ff-dbg\ipc\ipdl\pplugininstanceparent.cpp, line 394)
xul!mozilla::plugins::PluginInstanceParent::NPP_HandleEvent+0x00000000000000B1 (c:\mozilla\mozilla-central\src\dom\plugins\plugininstanceparent.cpp, line 532)
xul!mozilla::plugins::PluginModuleParent::NPP_HandleEvent+0x000000000000002A (c:\mozilla\mozilla-central\src\dom\plugins\pluginmoduleparent.cpp, line 300)
xul!nsNPAPIPluginInstance::HandleEvent+0x00000000000000CE (c:\mozilla\mozilla-central\src\modules\plugin\base\src\nsnpapiplugininstance.cpp, line 1378)
xul!nsObjectFrame::PaintPlugin+0x00000000000003DB (c:\mozilla\mozilla-central\src\layout\generic\nsobjectframe.cpp, line 1753)
xul!nsDisplayPlugin::Paint+0x000000000000003B (c:\mozilla\mozilla-central\src\layout\generic\nsobjectframe.cpp, line 1157)

Comment 9

[Jonathan Griffin (:jgriffin)]

Attachment: mochitest v0.2

This version works on Mac & Linux and not just Windows (oops)

Comment 10

[Jonathan Griffin (:jgriffin)]

Tests pushed as http://hg.mozilla.org/mozilla-central/rev/56e1067046fe

Comment 11

[Justin Wood (:Callek)]

(In reply to comment #10)
> Tests pushed as http://hg.mozilla.org/mozilla-central/rev/56e1067046fe

nit (for future)

The whitespace in the Makefile does not match surrounding lines.

The surrounding lines use tabs, you use spaces.

Comment 12

[Phil Ringnalda (:philor)]

Tests disabled in http://hg.mozilla.org/mozilla-central/rev/c43378b568df - while your first two Windows debug runs didn't show anything because they both hit bug 540967, and your first three Linux debug runs didn't show anything because Vlad was burning them, after those every single Windows and Linux Md6 had

TEST-UNEXPECTED-FAIL | plugin process 10661 | automationutils.processLeakLog() | leaked 8 bytes during test execution
TEST-UNEXPECTED-FAIL | plugin process 10661 | automationutils.processLeakLog() | leaked 8 bytes during test execution
TEST-UNEXPECTED-FAIL | plugin process 10661 | automationutils.processLeakLog() | leaked 1 instance of ChildNPObject with size 8 bytes
TEST-UNEXPECTED-FAIL | plugin process 10661 | automationutils.processLeakLog() | leaked 1 instance of ChildNPObject with size 8 bytes

Comment 13

[(not currently active) Ted Mielczarek]

(In reply to comment #11)
> (In reply to comment #10)
> > Tests pushed as http://hg.mozilla.org/mozilla-central/rev/56e1067046fe
> 
> nit (for future)
> 
> The whitespace in the Makefile does not match surrounding lines.
> 
> The surrounding lines use tabs, you use spaces.

FWIW, spaces are the preferred indentation for non-rule commands in Makefiles.

Comment 14

[Justin Wood (:Callek)]

(In reply to comment #13)
> (In reply to comment #11)
> > The surrounding lines use tabs, you use spaces.
> 
> FWIW, spaces are the preferred indentation for non-rule commands in Makefiles.

O I agree and knew that too; but unless I am wrong we prefer consistency (even if not ideal consistency).  I would have *preferred* to have the tabs surrounding changed to spaces too; but not too much worry at this moment though.