Last Comment Bug 327296 - [BeOS]GetMimeInfoFromExtension is very ineffective
: [BeOS]GetMimeInfoFromExtension is very ineffective
Status: RESOLVED FIXED
: fixed1.8.1.8
Product: Core
Classification: Components
Component: File Handling (show other bugs)
: Trunk
: x86 BeOS
: -- normal (vote)
: ---
Assigned To: Sergei Dolgov
: Hixie (not reading bugmail)
Mentors:
Depends on:
Blocks: 326898
  Show dependency treegraph
 
Reported: 2006-02-15 08:12 PST by Sergei Dolgov
Modified: 2007-09-26 15:14 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (3.87 KB, patch)
2006-02-19 09:06 PST, Sergei Dolgov
cbiesinger: review+
dveditz: approval1.8.1.8+
Details | Diff | Review

Description Sergei Dolgov 2006-02-15 08:12:46 PST
All code between
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/beos/nsOSHelperAppService.cpp#231
and
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/beos/nsOSHelperAppService.cpp#251

looks weird - it may be replaced with single call BMimeType::GuessMimeType() - native sniffer for file extensions.

It may be main reason for Bug 326898:
Comment 1 tqh 2006-02-15 11:10:07 PST
Yes, MIME-handling is if I remember correctly done quite inefficient in BeOS, which should be quite good at it.

Also bug 217723, bug 235350 might be interesting to keep in mind.
Comment 2 Sergei Dolgov 2006-02-19 09:06:37 PST
Created attachment 212389 [details] [diff] [review]
patch

1)replaces ineffective loops with system calls to handle mimeinfo.
2)changes a bit arguments for be_roster->Launch()

Honestly, all that NS file and mime handling chain is still bit messy for me, so i don't risk atm to change more things, inspite i have that temptation
Comment 3 Christian :Biesinger (don't email me, ping me on IRC) 2006-02-19 17:27:00 PST
Comment on attachment 212389 [details] [diff] [review]
patch

+	BMimeType mimeType, tmpType;
 
+	if( B_OK == mimeType.GuessMimeType(aFileExt, &tmpType))
+		return SetMIMEInfoForType(tmpType.Type(), _retval);

what's the point of tmpType? why not:

  BMimeType mimeType;
  if (B_OK == BMimeType::GuessMimeType(aFileExt, &mimeType))

(GuessMimeType is a static function)

(also, your style for this if doesn't seem to match the style of the file)
Comment 4 Sergei Dolgov 2006-02-19 18:14:17 PST
Per Comment 3:
Probably you are right, will rework/retest it tomorrow
Comment 5 Sergei Dolgov 2006-02-20 10:47:26 PST
Landed with biesi's corrections:
Checking in mozilla/uriloader/exthandler/beos/nsOSHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/beos/nsOSHelperAppService.cpp,v  <--  nsOSHelperAppService.cpp
new revision: 1.16; previous revision: 1.15
done  
Comment 6 Sergei Dolgov 2006-02-20 10:48:52 PST
Wondering if there is reason to land it also in 1.8 branch
Comment 7 Doug Shelton 2006-06-24 21:17:39 PDT
(In reply to comment #6)
> Wondering if there is reason to land it also in 1.8 branch
> 

wondering the same thing. 
Comment 8 tqh 2006-06-25 08:20:20 PDT
Isn't this one a big improvement?

Someone maybe could do a testbuild on 2.0?
Comment 9 Doug Shelton 2006-06-25 11:18:22 PDT
(In reply to comment #8)
> Isn't this one a big improvement?
> 
> Someone maybe could do a testbuild on 2.0?
> 
Right now, 2.0 will not build under BeOS.  We've committed at least 37 bug fixes and enhancements to the trunk but not the branch.
Comment 10 Sergei Dolgov 2006-06-25 16:53:49 PDT
I tried to build SeaMonkey release 1.02 which is also based on some old branch.
It builds, but backporting patches is really troublesome.
Tried to use (with some corrections which allows it building) current widget/src/beos - and got really very crashy version...

I think maybe  there were some special changes in branch which our port didn't take in account, but mostly i suspect DnD code around MouseMoved.

So personally I wouldn't spend time on SeaMonkey 1.02 release. Too late.

But FF may be worth some efforts, because there is real difference wuth SeaMonkey-trunk usability, inspite some bugs, and Minefield-BeOS state.
Comment 11 Sergei Dolgov 2007-09-25 12:01:05 PDT
Doug, I think this is one of bugs worth to back-port to branch. In recent SeaMonkey build I "enjoyed" again very slow download manager behavior related to slow icons painting related to this bug, I guess it may be problem in FF too.
Comment 12 Sergei Dolgov 2007-09-25 12:22:56 PDT
Patch applied cleanly to branch as is.
Will test results after remake.
Comment 13 Doug Shelton 2007-09-25 12:25:31 PDT
I haven't experienced the problem you describe with Firefox but I run on fast hardware.  I fully support back-porting as many BeOS-only improvements as possible, though.  I'll test the patch here as well.
Comment 14 Sergei Dolgov 2007-09-25 12:51:45 PDT
Tested.
Patch is very effective in branch too.
Comment 15 Doug Shelton 2007-09-25 17:34:37 PDT
Tested patch.  Confirm operation is correct.  Tested against Firefox 2.0.0.8pre branch.  Suggest you request inclusion in 1.8.1.8.   Thanks for finding this, Sergei!
Comment 16 Daniel Veditz [:dveditz] 2007-09-26 10:54:24 PDT
Comment on attachment 212389 [details] [diff] [review]
patch

approved for 1.8.1.8, a=dveditz for release-drivers
Comment 17 Sergei Dolgov 2007-09-26 15:14:16 PDT
Checking in mozilla/uriloader/exthandler/beos/nsOSHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/beos/nsOSHelperAppService.cpp,v  <--  nsOSHelperAppService.cpp
new revision: 1.13.4.2; previous revision: 1.13.4.1
done  

Note You need to log in before you can comment on or make changes to this bug.