327296 - [BeOS]GetMimeInfoFromExtension is very ineffective

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Sergei Dolgov]

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]

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]

Attachment: 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)]

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]

Per Comment 3:
Probably you are right, will rework/retest it tomorrow

Comment 5

[Sergei Dolgov]

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]

Wondering if there is reason to land it also in 1.8 branch

Comment 7

[Doug Shelton]

(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]

Isn't this one a big improvement?

Someone maybe could do a testbuild on 2.0?

Comment 9

[Doug Shelton]

(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]

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]

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]

Patch applied cleanly to branch as is.
Will test results after remake.

Comment 13

[Doug Shelton]

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]

Tested.
Patch is very effective in branch too.

Comment 15

[Doug Shelton]

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]]

Comment on attachment 212389 [details] [diff] [review]
patch

approved for 1.8.1.8, a=dveditz for release-drivers

Comment 17

[Sergei Dolgov]

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