Bug 51879 – [Symbian] Adobe flash Lite plugin on Symbian needs null window quirk

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

[Leonid Ebril]

WebKit crash on Symbian if Adobe Flash Lite plugin (version 10) unloaded.
The PluginQuirkDontSetNullWindowHandleOnDestroy needs to be set for the Adobe Flash Lite plugin on Symbian so the window does not get unset when destroying the plugin. Setting the PluginQuirkDontSetNullWindowHandleOnDestroy  flag fixes the crash.

[Leonid Ebril]

Created attachment 78406 [details]
Patch for "Adobe flash Lite plugin on Symbian needs null window quirk"

Set the PluginQuirkDontSetNullWindowHandleOnDestroy for Adobe Lite plugin if Flash 10 or newer (for Symbian platform), setting a NULL window handler on destroy crashes WebKit.

[WebKit Review Bot]

Attachment 78406 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/plugins/symbian/PluginPackageSymbian.cpp']" exit_code: 1
Source/WebCore/plugins/symbian/PluginPackageSymbian.cpp:58:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/plugins/symbian/PluginPackageSymbian.cpp:58:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebCore/plugins/symbian/PluginPackageSymbian.cpp:71:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/plugins/symbian/PluginPackageSymbian.cpp:72:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/plugins/symbian/PluginPackageSymbian.cpp:72:  Use 0 instead of NULL.  [readability/null] [4]
Source/WebCore/plugins/symbian/PluginPackageSymbian.cpp:73:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 9 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Seidel (no email)]

Comment on attachment 78406 [details]
Patch for "Adobe flash Lite plugin on Symbian needs null window quirk"

Please fix the style errors.

[Laszlo Gombos]

> Source/WebCore/ChangeLog:5
> +        https://bugs.webkit.org/show_bug.cgi?id=51879

Title for the bug is missing. Use "[Qt][Symbian]" as a prefix in the title as this issue is specific to the Symbian port of QtWebKit.

> Source/WebCore/ChangeLog:7
> +	Set the PluginQuirkDontSetNullWindowHandleOnDestroy for Adobe Lite  

Fix English - Setting a NULL window handler on destroy crashes Symbian port of QtWebKit, use PluginQuirkDontSetNullWindowHandleOnDestroy quirk for Adobe Flash Lite 4 (Flash 10 or newer).

> Source/WebCore/plugins/symbian/PluginPackageSymbian.cpp:58
> +	    determineQuirks (mime[0]);

Perhaps make the call before "m_mimeToExtensions.add" line. I do not see a particular reason to call determineQuirks() between the two m_mimeToDescriptions.add.. lines.

[Kenneth Rohde Christiansen]

> Fix English - Setting a NULL window handler on destroy crashes Symbian port of QtWebKit, use PluginQuirkDontSetNullWindowHandleOnDestroy quirk for Adobe Flash Lite 4 (Flash 10 or newer).

We should say null, not NULL. It is null/somethingNull in Qt API's and in C++0x it is called nullptr.

[Leonid Ebril]

Created attachment 78420 [details]
An updated version of the patch file

[WebKit Review Bot]

Attachment 78420 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/plugins/symbian/PluginPackageSymbian.cpp']" exit_code: 1
Source/WebCore/plugins/symbian/PluginPackageSymbian.cpp:58:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebCore/plugins/symbian/PluginPackageSymbian.cpp:72:  Use 0 instead of NULL.  [readability/null] [4]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Leonid Ebril]

Created attachment 78427 [details]
Fixed check-webkit-style issues

[WebKit Review Bot]

Attachment 78427 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/plugins/symbian/PluginPackageSymbian.cpp']" exit_code: 1
Source/WebCore/plugins/symbian/PluginPackageSymbian.cpp:58:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kenneth Rohde Christiansen]

Comment on attachment 78427 [details]
Fixed check-webkit-style issues

View in context: https://bugs.webkit.org/attachment.cgi?id=78427&action=review

> Source/WebCore/ChangeLog:8
> +        plugin if Flash 10 or newer (for Symbian platform), setting a NULL 

s/NULL/nulled/

> Source/WebCore/plugins/symbian/PluginPackageSymbian.cpp:58
> +            determineQuirks (mime[0]);

no space needed before (

> Source/WebCore/plugins/symbian/PluginPackageSymbian.cpp:70
> +        static const PlatformModuleVersion flashTenVersion(0x000a0000);

flashVersionTen seems more proper.

> Source/WebCore/plugins/symbian/PluginPackageSymbian.cpp:72
> +            // Flash 10 doesn't like having a 0 window handle

comments need a dot at the end. ie. be proper sentences.

[Leonid Ebril]

Created attachment 78432 [details]
Modified files according to Kenneth Rohde Christiansen comments

[WebKit Commit Bot]

Comment on attachment 78432 [details]
Modified files according to Kenneth Rohde Christiansen comments

Clearing flags on attachment: 78432

Committed r75439: <http://trac.webkit.org/changeset/75439>

[Laszlo Gombos]

Comment on attachment 78420 [details]
An updated version of the patch file

Clearing review as a newer version of a patch has been committed.

[Laszlo Gombos]

Changing the state as fix has been committed.

[Ademar Reis]

Revision r75439 cherry-picked into qtwebkit-2.2 with commit a32c088 <http://gitorious.org/webkit/qtwebkit/commit/a32c088>