Bug 33333 – [Qt] pass plugins/open-and-close-window-with-plugin.html on Qt

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Robert Hogan]

This test cannot pass on platforms other than Mac because the NP plugin
is a child of the page rather than the webview on those platforms.

The test is intended to cater for cases where a window is closed but
-[webview close] is not called by the plugin - Mac experienced a
regression where you could close the window and the plugin would continue
displaying because the plugin did not close the webview. When a window is closed
on non-Mac platforms the page is deleted and the plugin destroyed, so
the message logged by the plugin in the test reaches a deleted object.
In the case of Qt, because the object is (being) deleted the log message
from the plugin arrives in the QWebPage's implementation of
javascriptConsoleMessage() (which does nothing by default) rather than
in that of DRT's derived class WebPage (which writes to stdout). It's
pure chance that the memory has not been overwritten at
that point so it would not be correct to alter QWebpage to
cater for it. (Note that this behaviour only occurs in the DRT anyway
because the test plugin has logDestroy property).

[Robert Hogan]

Created attachment 46069 [details]
Patch

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 46069 [details] without any errors.

[Maciej Stachowiak]

Comment on attachment 46069 [details]
Patch

Are you sure it can't pass on platforms other than Mac? I don't think it's been a problem on anything but Qt. I think the right fix may be to put this test in the Qt skipped list. r- to consider that option (or just reflag if you are sure this is the best approach).

[Robert Hogan]

(In reply to comment #3)
> (From update of attachment 46069 [details])
> Are you sure it can't pass on platforms other than Mac? I don't think it's been a problem on anything but Qt. I think the right fix may be to put this test in the Qt skipped list. r- to consider that option (or just reflag if you are sure this is the best approach).

It's skipped by everyone except chromium:

LayoutTests/platform/mac-snowleopard/Skipped:plugins/open-and-close-window-with-plugin.html
LayoutTests/platform/qt/Skipped:plugins/open-and-close-window-with-plugin.html
LayoutTests/platform/gtk/Skipped:plugins/open-and-close-window-with-plugin.html
LayoutTests/platform/win/Skipped:plugins/open-and-close-window-with-plugin.html

[Robert Hogan]

Created attachment 58603 [details]
Patch

[Robert Hogan]

(In reply to comment #5)
> Created an attachment (id=58603) [details]
> Patch

Maybe it's ok/expected that QWebPage can receive the call to javasriptConsoleMessage() once DRT's WebPage is deleted. I'm not sure, so putting this up as a solution for discussion.

[Simon Hausmann]

Comment on attachment 58603 [details]
Patch

scary :) r=me

[Robert Hogan]

Committed r61346: <http://trac.webkit.org/changeset/61346>

[Robert Hogan]

Reverted r61346 for reason:

Broke inspector tests on Qt

Committed r61348: <http://trac.webkit.org/changeset/61348>

[Robert Hogan]

Created attachment 59034 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 59034 [details]
Patch

I dont get it. The WebPage inherits from QWebPage, and you say that the WebPage part has been destroyed but not the QWebPage one? Wouldn't it be better to fix the destruction?

[Kenneth Rohde Christiansen]

(In reply to comment #11)
> (From update of attachment 59034 [details])
> I dont get it. The WebPage inherits from QWebPage, and you say that the WebPage part has been destroyed but not the QWebPage one? Wouldn't it be better to fix the destruction?

Apparently now that I am reading your original comment

[Robert Hogan]

Committed r61433: <http://trac.webkit.org/changeset/61433>