Issue 93366 - fpicker: more efficient to run the gtk (like macosx) fpicker in the normal thread
Summary: fpicker: more efficient to run the gtk (like macosx) fpicker in the normal th...
Status: CLOSED FIXED
Alias: None
Product: gsl
Classification: Code
Component: code (show other issues)
Version: DEV300m30
Hardware: All Linux, all
: P3 Trivial (vote)
Target Milestone: OOo 3.1
Assignee: kendy
QA Contact: issues@gsl
URL:
Keywords:
: 97423 (view as issue list)
Depends on: 92878
Blocks:
  Show dependency tree
 
Reported: 2008-09-02 09:35 UTC by caolanm
Modified: 2008-12-23 12:27 UTC (History)
4 users (show)

See Also:
Issue Type: PATCH
Latest Confirmation in: ---
Developer Difficulty: ---

Attachments
proposed patch (6.68 KB, patch)
2008-09-02 09:35 UTC, caolanm 		no flags Details | Diff
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this issue.
Description caolanm 2008-09-02 09:35:11 UTC 
The windows system fpicker was the first one, and in sfx2 it gets run in a
thread (for some reason or other).

The macosx one is ifdefed to get run in the current thread instead because of
drawing issues.

The gtk one ties itself in complete knots to launch the system dialog back in
the "main thread" because gnome-vfs2 backed dialogs need to have the gnome-vfs2
stuff run in the same thread as where gnome-vfs2 was initialized in ucb so as to
be reliable (though the new gio backed one seems to not need this)

Seeing as there are only three system fpicker implementations, windows, macosx
and gtk. It seems best to just special-case the windows one in sfx2 to get its
own thread, and let the other ones just get executed "naturally". 

It's way easier for any future fpickers to decide themselves in their own
implementations to run their system dialog in a custom thread than it is to get
themselves run in the primordial thread after-the-fact.

The plus side for the gtk fpicker is that the 100% cpu behaviour seen in 3.0.0
when it is open goes away.
Comment 1 caolanm 2008-09-02 09:35:35 UTC 
Created attachment 56152 [details]
proposed patch
Comment 2 caolanm 2008-09-02 09:36:44 UTC 
accepting...
Comment 3 philipp.lohmann 2008-09-23 14:27:42 UTC 
Do you have a CWS for this ?
Comment 4 caolanm 2008-09-23 14:32:53 UTC 
Yeah, but I waiting for m31 to sync it up, so as to not conflict with a
different fpicker thing.
Comment 5 caolanm 2008-09-23 14:33:43 UTC 
Yeah, but I waiting for m32 to sync it up, so as to not conflict with a
different fpicker thing.
Comment 6 caolanm 2008-10-09 10:35:05 UTC 
Done in cmcfixes50

cmc->kendy: You may need to check the out-of-tree kde fpicker to see if it will
be happy with this change, the in-tree ones are
Comment 7 kendy 2008-10-09 13:42:03 UTC 
If the patch hasn't changed when it went to the CWS, everything should be 
fine, I've already updated the KDE fpicker in ooo-build to deal with being in 
the main thread [it was not too happy with that before - it froze the entire 
KDE when chosen from the menu ;-)].
Comment 8 caolanm 2008-10-15 13:06:26 UTC 
cmc->kendy: Happy to verify this change ?
Comment 9 kendy 2008-10-16 20:12:51 UTC 
cmc: Please, is this CWS still in CVS, or in SVN?  I tried to extract the 
patch from the CVS, but it seems to be missing the fpicker part...
Comment 10 caolanm 2008-10-16 22:58:47 UTC 
svn, in old cvs land we were waiting on the conflicting issue 92878 before we
could commit it.
Comment 11 kendy 2008-10-21 16:18:44 UTC 
Now verified.
Comment 12 caolanm 2008-11-26 11:38:14 UTC 
closing, is in DEV300_m36
Comment 13 caolanm 2008-12-23 12:27:45 UTC 
*** Issue 97423 has been marked as a duplicate of this issue. ***