Bug 34729 - [Qt] Exposing an QVariantMap containing QObjectStar to Javascript causes Segmentation Fault
Summary: [Qt] Exposing an QVariantMap containing QObjectStar to Javascript causes Segm...
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: QtWebKit Unassigned
URL:
Keywords: Qt
Depends on:
Blocks: 35784
  Show dependency treegraph
 
Reported: 2010-02-08 15:50 PST by Bruno Schmidt
Modified: 2014-04-24 16:45 PDT (History)
5 users (show)

See Also:


Attachments
Patch fixing the bug (1.53 KB, patch)
2010-02-08 15:50 PST, Bruno Schmidt
no flags Details | Formatted Diff | Diff
Patch fixing the bug with unit tests and changelog (6.54 KB, patch)
2010-04-08 17:14 PDT, Bruno Schmidt
hausmann: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Rebased version of the previous patch to fix the Commit Bot error (6.02 KB, patch)
2010-04-22 09:41 PDT, Bruno Schmidt
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bruno Schmidt 2010-02-08 15:50:41 PST
Created attachment 48370 [details]
Patch fixing the bug

If an QVariantMap containing QObjectStar is added to the to QtWebkit Javascript, it's use causes Segmentation Fault.
It happens because, in the case QMetaType::QVariantMap, the "root" object that is inside of a PassRefPtr is passed recursively inside a loop to recover the content of the map, but the PassRefPtr semantics prohibit its use inside a loop, so the "root" object mus be passed using the method "PassRefPtr::get" in order to keep the current reference.
Comment 1 Tor Arne Vestbø 2010-03-10 06:31:34 PST
Please follow the QtWebKit bug reporting guidelines when reporting bugs.

See http://trac.webkit.org/wiki/QtWebKitBugs

Specifically:

  - The 'QtWebKit' component should only be used for bugs/features in the
    public QtWebKit API layer, not to signify that the bug is specific to
    the Qt port of WebKit

      http://trac.webkit.org/wiki/QtWebKitBugs#Component

  - Add the keyword 'Qt' to signal that it's a Qt-related bug

      http://trac.webkit.org/wiki/QtWebKitBugs#Keywords
Comment 2 Robert Hogan 2010-03-10 13:29:22 PST
Hi Bruno,

You need to add a Changelog etc. for this. And also a test case or layout test. You then need to put the r=? flag on the revised patch so a reviewer will know to look at your patch.

See also :

http://webkit.org/coding/contributing.html

and

http://webkit.org/quality/reporting.html

Good spot though!
Comment 3 Bruno Schmidt 2010-04-08 17:14:13 PDT
Created attachment 52919 [details]
Patch fixing the bug with unit tests and changelog
Comment 4 Bruno Schmidt 2010-04-13 13:17:46 PDT
(In reply to comment #2)
> Hi Bruno,
> 
> You need to add a Changelog etc. for this. And also a test case or layout test.
> You then need to put the r=? flag on the revised patch so a reviewer will know
> to look at your patch.
> 
> See also :
> 
> http://webkit.org/coding/contributing.html
> 
> and
> 
> http://webkit.org/quality/reporting.html
> 
> Good spot though!

I think everything is in shape now, any news about the merge?
Comment 5 Robert Hogan 2010-04-13 14:15:12 PDT
(In reply to comment #4)
> I think everything is in shape now, any news about the merge?

You're patch is waiting in the list for review!

https://bugs.webkit.org/request.cgi?action=queue&requester=&product=&type=all&requestee=&component=&group=type
Comment 6 Bruno Schmidt 2010-04-19 09:38:18 PDT
Changing the component related to the bug.
It is actually more related to a Qt JavaScriptGlue bug than to a generic WebKit bug.
Comment 7 Simon Hausmann 2010-04-19 17:44:25 PDT
A crash fix worth a release cherry-pick IMHO.
Comment 8 WebKit Commit Bot 2010-04-22 05:19:33 PDT
Comment on attachment 52919 [details]
Patch fixing the bug with unit tests and changelog

Rejecting patch 52919 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Simon Hausmann', u'--force']" exit_code: 1
Last 500 characters of output:
 lines).
1 out of 2 hunks FAILED -- saving rejects to file WebCore/bridge/qt/qt_runtime.cpp.rej
patching file WebKit/qt/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebKit/qt/tests/qwebframe/tst_qwebframe.cpp
Hunk #2 FAILED at 105.
Hunk #3 succeeded at 137 (offset 2 lines).
Hunk #4 succeeded at 496 (offset 11 lines).
Hunk #5 succeeded at 773 (offset 13 lines).
Hunk #6 FAILED at 2849.
2 out of 6 hunks FAILED -- saving rejects to file WebKit/qt/tests/qwebframe/tst_qwebframe.cpp.rej

Full output: http://webkit-commit-queue.appspot.com/results/1848040
Comment 9 Bruno Schmidt 2010-04-22 09:41:06 PDT
Created attachment 54067 [details]
Rebased version of the previous patch to fix the Commit Bot error
Comment 10 WebKit Commit Bot 2010-04-26 05:47:42 PDT
Comment on attachment 54067 [details]
Rebased version of the previous patch to fix the Commit Bot error

Clearing flags on attachment: 54067

Committed r58250: <http://trac.webkit.org/changeset/58250>
Comment 11 WebKit Commit Bot 2010-04-26 05:47:49 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Simon Hausmann 2010-04-26 08:24:55 PDT
Revision r58250 cherry-picked into qtwebkit-2.0 with commit f737ab24aab5cb171853390f3dad549823ac185c
Comment 13 Darin Adler 2014-04-24 16:45:08 PDT
Moving all JavaScriptGlue bugs to JavaScriptCore. The JavaScriptGlue framework itself is long gone. And most of the more recent bugs put in this component were put there by people who thought this was for some other aspect of “JavaScript glue” and have nothing to do with the actual original reason for the existence of this component, which was an OS-X-only framework named JavaScriptGlue.