Bug 36702 – [Qt] NPP_SetWindow seems to not be called when TestNetscapePlugin is moved

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[Robert Hogan]

Created attachment 51828 [details]
Test Case

In the attached test, replacing

            window.addEventListener('load', doTest, false);

with

            window.addEventListener('load', function() {
                window.setTimeout(doTest, 0);

will cause the test to fail.

[Alexey Proskuryakov]

The test expectation is "should not crash". Are you saying that it crashes if doTest is deferred?

Note that WebKit Qt component is for bugs in the WebKit Qt API layer.

[Robert Hogan]

(In reply to comment #1)
> The test expectation is "should not crash". Are you saying that it crashes if
> doTest is deferred?
> 
> Note that WebKit Qt component is for bugs in the WebKit Qt API layer.

No, it times out in Qt. Note that you need to have the patch at https://bugs.webkit.org/show_bug.cgi?id=36675 applied to encounter the bug.

[Alexey Proskuryakov]

This seems to imply that NPP_SetWindow is not called on plug-in if it's moved after initial layout. I think that's correct behavior for windowed plug-ins (but I'm not sure whether TestNetscapePlugin is windowed or windowless, particularly on Qt).

[Balazs Kelemen]

Please clarify the situation:
  * is it crashing or just not working correctly?
  * is this the correct behavior as ap suggested or not?

[Robert Hogan]

(In reply to comment #4)
> Please clarify the situation:
>   * is it crashing or just not working correctly?
>   * is this the correct behavior as ap suggested or not?

The two tests: LayoutTests/plugins/update-widgets-crash.html and LayoutTests/plugins/reentrant-update-widget-positions.html are the same except for that the former has:
           window.addEventListener('load', doTest, false);

instead of

            window.addEventListener('load', function() {
                window.setTimeout(doTest, 0);

LayoutTests/plugins/update-widgets-crash.html passes while LayoutTests/plugins/reentrant-update-widget-positions.html fails.

It fails by timing out.

[Robert Hogan]

Created attachment 58702 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 58702 [details]
Patch

Clearing flags on attachment: 58702

Committed r61311: <http://trac.webkit.org/changeset/61311>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

It was rolled out by http://trac.webkit.org/changeset/61315,
because it make 2 tests fail:
http://build.webkit.org/results/Qt%20Linux%20Release/r61314%20%2813453%29/results.html

[Robert Hogan]

(In reply to comment #9)
> It was rolled out by http://trac.webkit.org/changeset/61315,
> because it make 2 tests fail:
> http://build.webkit.org/results/Qt%20Linux%20Release/r61314%20%2813453%29/results.html

That was sloppy, sorry. I'll look into this now.

[Robert Hogan]

(In reply to comment #9)
> It was rolled out by http://trac.webkit.org/changeset/61315,
> because it make 2 tests fail:
> http://build.webkit.org/results/Qt%20Linux%20Release/r61314%20%2813453%29/results.html

Qt expected results for resize-from-plugin.html were wrong anyway by the looks of it, the new results are slightly less wrong but still problematic. 

The mac expected result is:

x: 18, y: 52, width: 200, height: 200, clipRect: (18, 52, 200, 142)

Qt result was (without patch):

x: 18, y: 54, width: 100, height: 100, clipRect: (0, 0, 100, 100)

And with patch:

x: 18, y: 54, width: 100, height: 100, clipRect: (0, 0, 200, 142)

So closer, but still no cigar.

The other test looks like a minor layout difference due to the extra resizing now taking place in DRT. So results just need to be updated when I land the corrected patch.

[Robert Hogan]

Created attachment 59192 [details]
Patch

[Robert Hogan]

Committed r61505: <http://trac.webkit.org/changeset/61505>