Bug 51879 - [Symbian] Adobe flash Lite plugin on Symbian needs null window quirk
Summary: [Symbian] Adobe flash Lite plugin on Symbian needs null window quirk
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Plug-ins (show other bugs)
Version: 528+ (Nightly build)
Hardware: S60 Hardware S60 3rd edition
: P2 Normal
Assignee: Leonid Ebril
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-04 10:29 PST by Leonid Ebril
Modified: 2011-01-12 05:15 PST (History)
7 users (show)

See Also:


Attachments
Patch for "Adobe flash Lite plugin on Symbian needs null window quirk" (1.94 KB, patch)
2011-01-10 09:57 PST, Leonid Ebril
eric: review-
Details | Formatted Diff | Diff
An updated version of the patch file (1.98 KB, patch)
2011-01-10 11:51 PST, Leonid Ebril
leonid.ebril: commit-queue?
Details | Formatted Diff | Diff
Fixed check-webkit-style issues (1.98 KB, patch)
2011-01-10 12:13 PST, Leonid Ebril
kenneth: review-
leonid.ebril: commit-queue?
Details | Formatted Diff | Diff
Modified files according to Kenneth Rohde Christiansen comments (1.97 KB, patch)
2011-01-10 12:49 PST, Leonid Ebril
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Leonid Ebril 2011-01-04 10:29:55 PST
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.
Comment 1 Leonid Ebril 2011-01-10 09:57:44 PST
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.
Comment 2 WebKit Review Bot 2011-01-10 10:00:55 PST
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.
Comment 3 Eric Seidel 2011-01-10 11:29:28 PST
Comment on attachment 78406 [details]
Patch for "Adobe flash Lite plugin on Symbian needs null window quirk"

Please fix the style errors.
Comment 4 Laszlo Gombos 2011-01-10 11:46:08 PST
> 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.
Comment 5 Kenneth Rohde Christiansen 2011-01-10 11:49:20 PST
> 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.
Comment 6 Leonid Ebril 2011-01-10 11:51:25 PST
Created attachment 78420 [details]
An updated version of the patch file
Comment 7 WebKit Review Bot 2011-01-10 11:52:50 PST
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.
Comment 8 Leonid Ebril 2011-01-10 12:13:55 PST
Created attachment 78427 [details]
Fixed check-webkit-style issues
Comment 9 WebKit Review Bot 2011-01-10 12:15:05 PST
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.
Comment 10 Kenneth Rohde Christiansen 2011-01-10 12:16:36 PST
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.
Comment 11 Leonid Ebril 2011-01-10 12:49:25 PST
Created attachment 78432 [details]
Modified files according to Kenneth Rohde Christiansen comments
Comment 12 WebKit Commit Bot 2011-01-10 14:47:36 PST
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>
Comment 13 Laszlo Gombos 2011-01-11 11:58:08 PST
Comment on attachment 78420 [details]
An updated version of the patch file

Clearing review as a newer version of a patch has been committed.
Comment 14 Laszlo Gombos 2011-01-11 11:58:26 PST
Changing the state as fix has been committed.
Comment 15 Ademar Reis 2011-01-12 05:15:35 PST
Revision r75439 cherry-picked into qtwebkit-2.2 with commit a32c088 <http://gitorious.org/webkit/qtwebkit/commit/a32c088>