587865 - JM: MacOSX 10.5 mochitest-chrome failure on modules/plugin/test/test_convertpoint.xul

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[David Mandelin [:dmandelin]]

6494 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/modules/plugin/test/test_convertpoint.xul | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - Error calling method on NPObject! at chrome://mochikit/content/chrome/modules/plugin/test/test_convertpoint.xul:67
NPP_Destroy

Comment 1

[David Mandelin [:dmandelin]]

I can repro this locally individually.

Comment 2

[David Mandelin [:dmandelin]]

I have some more info. We fail on this call:

      xResult = pluginElement.convertPointX(sourceCoordSpaceValue, xValues[i], yValues[i], destCoordSpaceValue);

The failure is because xValues[1] (i.e., the failure occurs when i==1) is a double, value 13, while this test plugin method requires int32 arguments. This seems to be related to the fact that |domWindowUtils.screenPixelsPerCSSPixel|, one of the inputs used to compute xValues[1], is a double. The full computation is:

  var devPxPerCSSPx = domWindowUtils.screenPixelsPerCSSPixel;
  var pluginX = (pluginRect.left * devPxPerCSSPx) + ((window.mozInnerScreenX * devPxPerCSSPx) - window.screenX);
  var NPCoordinateSpaceWindowX = pluginX + xOffset;
  // The latter is xValues[1].

I am wondering if the engine normally does some demotion that JM skips, perhaps on the add/mul paths? If so, we might want to just fix the test case, but I also wonder if there might be other such bugs that are exposed by non-demotion.

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

I think fat values broke this invariant early on, and it'd probably be a perf loss to maintain it in JM.

Comment 4

[Brendan Eich [:brendan]]

The other option is to have the NPRuntime code do the double -> int conversions that the JS engine used to do.

Similar situation may arise with XPConnect and the DOM, if it hasn't already.

/be

Comment 5

[Brendan Eich [:brendan]]

I smell trouble if something doesn't handle this. SpiderMonkey's original jsval representation and promises are Old. Easy winning bet is that plugins count on this kind of representation canonicalization.

/be

Comment 6

[David Mandelin [:dmandelin]]

(In reply to comment #4)
> Similar situation may arise with XPConnect and the DOM, if it hasn't already.

I also started to wonder about that.

> The other option is to have the NPRuntime code do the double -> int conversions
> that the JS engine used to do.

That's what I was thinking--in particular to change JSValToNPVariant. Is that what you also had in mind?

Comment 7

[Brendan Eich [:brendan]]

Yeah -- change the conversion glue in the various JS <-> {Plugin,XPCOM,...} bridges.

/be

Comment 8

[David Mandelin [:dmandelin]]

http://hg.mozilla.org/projects/jaegermonkey/rev/6e5e2f31795b