The default bug view has changed. See this FAQ.

[BeOS]GetMimeInfoFromExtension is very ineffective

RESOLVED FIXED

Status

Core Graveyard
File Handling
RESOLVED FIXED
11 years ago
9 months ago

People

(Reporter: Sergei Dolgov, Assigned: Sergei Dolgov)

Tracking

({fixed1.8.1.8})

Trunk
x86
BeOS
fixed1.8.1.8

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

11 years ago
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

11 years ago
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.
(Assignee)

Updated

11 years ago
Blocks: 326898
(Assignee)

Comment 2

11 years ago
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
Assignee: file-handling → sergei_d
Status: NEW → ASSIGNED
Attachment #212389 - Flags: review?(cbiesinger)
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)
Attachment #212389 - Flags: review?(cbiesinger) → review+
(Assignee)

Comment 4

11 years ago
Per Comment 3:
Probably you are right, will rework/retest it tomorrow
(Assignee)

Comment 5

11 years ago
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  
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Comment 6

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

Comment 7

11 years ago
(In reply to comment #6)
> Wondering if there is reason to land it also in 1.8 branch
> 

wondering the same thing. 

Comment 8

11 years ago
Isn't this one a big improvement?

Someone maybe could do a testbuild on 2.0?

Comment 9

11 years ago
(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.
(Assignee)

Comment 10

11 years ago
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.
(Assignee)

Comment 11

10 years ago
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.
(Assignee)

Comment 12

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

Comment 13

10 years ago
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.
(Assignee)

Comment 14

10 years ago
Tested.
Patch is very effective in branch too.

Comment 15

10 years ago
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!

Updated

10 years ago
Attachment #212389 - Flags: approval1.8.1.8?
Comment on attachment 212389 [details] [diff] [review]
patch

approved for 1.8.1.8, a=dveditz for release-drivers
Attachment #212389 - Flags: approval1.8.1.8? → approval1.8.1.8+
(Assignee)

Comment 17

10 years ago
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  
Keywords: fixed1.8.1.8
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.