Turn MozApp into a singleton to stop it wreaking havok when created several times.

RESOLVED INCOMPLETE

Status

()

RESOLVED INCOMPLETE
10 years ago
3 years ago

People

(Reporter: tobias.hunger, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 6 obsolete attachments)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.4) Gecko/2008111616 Ubuntu/9.04 (jaunty) Firefox/3.0.4
Build Identifier: incubator/embedding hg repository rev. dabab67820e6

We ran into really strange behavior when calling the MozApp constructor several times. To stop this from wreaking havok we changed the MozApp into a singleton.

This patch may depend on patches found on reports #467071, #467072 and #467073.

This patch was tested with the Qt flavor of the new embedding APIs only. Please consider it for inclusion into the new embedding API.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
(Reporter)

Comment 1

10 years ago
Created attachment 350452 [details] [diff] [review]
The actual patch
(Reporter)

Comment 2

10 years ago
Created attachment 350458 [details] [diff] [review]
The actual patch (not gzip'ed)
Attachment #350458 - Flags: review?(mark.finkle)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 350458 [details] [diff] [review]
The actual patch (not gzip'ed)

r+, but 4 spaces for indents please.

Also, we shouldn't land this until the Windows and GTK wrappers are updated too. Separate patches for those are fine, we can just add them to this bug.
Attachment #350458 - Flags: review?(mark.finkle) → review+
I'm not saying Tobias needs to be the one to make the Windows and GTK patches either. I will update the Windows wrapper when I get a chance, for example.
(Reporter)

Comment 5

10 years ago
Created attachment 350766 [details] [diff] [review]
mozapp singleton patch v1.1

Incorporate feedback from reviewers.
Attachment #350452 - Attachment is obsolete: true
Attachment #350458 - Attachment is obsolete: true
(Reporter)

Comment 6

10 years ago
Is there anything I can do to make the patch better and to get it applied?
(In reply to comment #6)
> Is there anything I can do to make the patch better and to get it applied?

Tobias, there is something you could do if you had some time. Could you grep the rest of the code for uses of MozApp and switch to the MozApp::Instance()?

I am a bit swamped at the moment and we really shouldn't be breaking the other wrappers by landing the patch with only the Qt wrapper updated.
(Reporter)

Comment 8

10 years ago
I can do that, but I do not have a build environment handy for windows, so a patch for that will be exactly that: greping for MozApp and replacing it with MozApp::Instance.
(Reporter)

Comment 9

10 years ago
Created attachment 352506 [details] [diff] [review]
mozapp singleton patch v1.2: Incl. win/gtk

This patch should improve the GTK/win32 situation somewhat: Both should at least build now.

*ATTENTION*: GTK is untested (not even compiled!) as the Makefile shipped with the sources is broken.

*ATTENTION*: Windows is untested (not even compiled) as I have no windows environment to test with on hand. This should not be much of an issue since there was only one occurance of MozApp in the windows code anyway (and that was in the unit test and never used after construction).
Attachment #350766 - Attachment is obsolete: true
(Reporter)

Comment 10

10 years ago
Could we please get this merged? I can then update the other patches to apply
cleanly against this changed base.
(Reporter)

Updated

10 years ago
Blocks: 467076
(Reporter)

Updated

10 years ago
Depends on: 467072
(Reporter)

Comment 11

10 years ago
Created attachment 352714 [details] [diff] [review]
New version with updated dependencies.

This patch applies cleanly against current hg (with the patches it depends on applied of course).
Attachment #352506 - Attachment is obsolete: true
(Reporter)

Updated

10 years ago
Depends on: 467071
(Reporter)

Updated

10 years ago
Depends on: 469647
No longer depends on: 467071
(Reporter)

Comment 12

10 years ago
Created attachment 353010 [details] [diff] [review]
MozApp singleton patch v1.3

This patch applies cleanly on hg rev. 0c3baa33bba5 (with the patches it depends on applied of course).
Attachment #352714 - Attachment is obsolete: true
(Reporter)

Updated

10 years ago
Attachment #353010 - Flags: review+
(Reporter)

Updated

10 years ago
Attachment #353010 - Flags: review+
(Reporter)

Updated

10 years ago
Attachment #353010 - Flags: review?(mark.finkle)
(Reporter)

Comment 13

10 years ago
Created attachment 353409 [details] [diff] [review]
mozapp_singleton patch v1.4

Update patch to hg rev. 43a4f0d...
Attachment #353010 - Attachment is obsolete: true
Attachment #353010 - Flags: review?(mark.finkle)
(Reporter)

Updated

10 years ago
Attachment #353409 - Flags: review?(mark.finkle)
(Reporter)

Comment 14

10 years ago
Mark: Is there still interest in this patch? I don't want to spend time updating it again and again without any feedback whatsoever since about one month ago.
Tobias: Yes, I think we should take this patch. Can you update it to trunk one more time?
Attachment #353409 - Flags: review?(mark.finkle)
Marking a bunch of bugs in the "Embedding: APIs" component INCOMPLETE in preparation to archive that component. If I have done this incorrectly, please reopen the bugs and move them to a more correct component as we don't have "embedding" APIs any more.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.