Frank Tang Reporter
	Description • 23 years ago

we currently load maccharset.properties at startup time, we should find a way to 
remove the need of loading it at startup time to speed up startup performance
we could 
1. build in common used pair in the cpp code, and/or
2. cache the resolved result in registry.

	jbetak@netscape.com (away - not reading bugmail)
	Comment 1 • 23 years ago

per offline discussion with ftang - tentatively reassigning to myself

	Frank Tang Reporter
	Comment 2 • 23 years ago

mac sutff, move to ftang for now.

	Frank Tang Reporter
	Comment 3 • 23 years ago

nhotta- can you handle this?

what we should do is
1. move the loading of property file from constructor to a seperate 
InitInfo() function
2. build in some common used mapping into c++ (for US, JA, DE and FR ) so we 
don't need to look at those mapping in the property file if we running on those 
tier 1 platform
3. call InitInfo right before we really need to access those mapping.
In this way, we keep both the dynamic part and tuning the performance at the 
same time. 

	Frank Tang Reporter
	Comment 6 • 23 years ago

-  PR_AtomicIncrement(&gCnt);
why you want to do that ? we should always have balance of

110   PR_AtomicDecrement(&gCnt); 
in the destructory. if you remove this you proably also need to change the 
destrcutor

+    PR_AtomicIncrement(&gCnt);
gCnt is the count of nsMacCharset object. it should be in the contstructor.

+    if (NS_FAILED(rv)) { charset.AssignWithConversion("x-mac-roman");}
please break the if statement into multiple line
don't put the body of the block and the if in the same line.
we won't be able to set break point in the body of the block

	Frank Tang Reporter
	Comment 8 • 23 years ago

Comment on attachment 52649 [details] [diff] [review]
Patch, updated to ftang's comment.

look fine r=ftang

	Simon Fraser [no longer active]
	Comment 9 • 23 years ago

+    nsAutoString propertyURL(NS_LITERAL_STRING("resource:/res/
maccharset.properties"));
+    nsURLProperties *info = new nsURLProperties( propertyURL );

Not your fault, but nsURLProperties() needs to be updated to use some newer 
string stuff (e.g. is its argument read-only?).

+nsresult nsMacCharset::MapToCharset(short script, short region, nsString& 
charset)
Is 'charset' an out param? Rename it to make this clearer (e.g. 'outCharset').

+    rv = pMacLocale->GetPlatformLocale(&localeAsString, &script, &language, &
region);
Ugh, GetPlatformLocale takes a nsString*. Needs fixing later.

Rename 'charset' and sr=sfraser.

	nhottanscp Assignee
	Comment 11 • 23 years ago

Checked in to the trunk.

	Teruko Kobayashi
	Comment 12 • 23 years ago

Changed QA contact to nhotta@netscap.com.