Closed Bug 586768 Opened 12 years ago Closed 12 years ago

Incubator qt embedding: QMozView crashes if QMozApp is inited before

Categories

(Core Graveyard :: Embedding: APIs, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: tatiana, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.8) Gecko/20100723 Ubuntu/10.04 (lucid) Firefox/3.6.8
Build Identifier: 

QMozView crashes if QMozApp is inited before with no profile path specified
i.e. like here:

{code}
    mozApp = new QMozApp();
    /*...*/

    mozView = new QMozView(mForm);
{code}

bt:
#0  0x0066940b in MozView::CreateBrowser (this=0x9f58240,
aParentWindow=0x9f5e588, aX=0, aY=0, 
    aWidth=0, aHeight=0, aChromeFlags=0) at ../common/embed.cpp:307
#1  0x0066698f in QMozView (this=0x9f5e588, parent=0x9f53298, flags=0) at
QMozView.cpp:135
#2  0x0804dec5 in MyQGraphicsView (this=0x9f187f8, aUri=..., scene=0xbfe8fea0,
parent=0x0)
    at test.cpp:71
#3  0x0804e50d in main (argc=1, argv=0xbfe8ff64) at test.cpp:124


Reproducible: Always



Expected Results:  
test app should create default profile dir and run
Blocks: 583546
Attached patch patch (obsolete) — Splinter Review
Attachment #465361 - Flags: review?(romaxa)
Attachment #465361 - Flags: review?(romaxa)
Attachment #465361 - Flags: review?(mark.finkle)
Attachment #465361 - Flags: feedback?(romaxa)
No longer blocks: 583546
Depends on: 583546
Comment on attachment 465361 [details] [diff] [review]
patch

># HG changeset patch
># User Tatiana Meshkova <tanya.meshkova@gmail.com>
>diff -r 6bfdc9081f9c -r 7f661038e597 qt/QMozApp.cpp
>--- a/qt/QMozApp.cpp
>+++ b/qt/QMozApp.cpp
>@@ -44,17 +44,18 @@
> class QMozApp::Private
> {
> public:
>     Private(const char* aProfilePath = 0) : mozApp(aProfilePath) {}
>     MozApp mozApp;
> };
> 
> QMozApp::QMozApp(const QString& profilePath) :
>-    mPrivate(new Private(profilePath.toUtf8()))
>+    mPrivate(profilePath.isEmpty() ? new Private() 
>+                                   : new Private(profilePath.toUtf8()))

First of all I would like to see, value inside constructor params
>-    mPrivate(new Private(profilePath.toUtf8()))
>+    mPrivate(profilePath.isEmpty() ? nsnull : profilePath.toUtf8().data())

also profilePath.toUtf8() return QByteArray but we want char* then it should be .data()


Also here is possible another fix, inside EmbeddingSetup, InitEmbedding

instead of 
--- if (aProfilePath) {
+++ nsCString pr(aProfilePath);
+++ if (!pr.IsEmpty()) {

in this case it will work also for Gtk and other API's
Attachment #465361 - Flags: feedback?(romaxa) → feedback-
Attached patch patchSplinter Review
.data() and isEmpty() are in place now
Attachment #465361 - Attachment is obsolete: true
Attachment #466475 - Flags: review?(mark.finkle)
Attachment #466475 - Flags: feedback?(romaxa)
Attachment #465361 - Flags: review?(mark.finkle)
Comment on attachment 466475 [details] [diff] [review]
patch

yep, this is right fix.
Attachment #466475 - Flags: feedback?(romaxa) → feedback+
Attachment #466475 - Flags: review?(mark.finkle) → review+
http://hg.mozilla.org/incubator/embedding/rev/607aad0e0aea
Status: UNCONFIRMED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.