492225 - Remove nsUTF8Utils dependency from nsWindowsWMain.cpp

Status: RESOLVED INCOMPLETE

(Toolkit Graveyard :: XULRunner, defect)

Description

[Hiroyuki Ikezoe (:hiro)]

Attachment: Proposed patch

It is sufficient to convert UTF16 to UTF8 with WideCharToMultiByte.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Checked in:
http://hg.mozilla.org/mozilla-central/rev/773dc0ba39c3

Caused a fatal assertion:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1242599695.1242602148.17227.gz

So I backed it out:
http://hg.mozilla.org/mozilla-central/rev/a7c0b3588242

Comment 2

[Hiroyuki Ikezoe (:hiro)]

I am so sorry. Attachment 376587 [details] [diff] does not consider about the last '\0' character.

I will revise the patch after my build machine is repaired. Robert, thank you for the notice.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

No problem, thanks for your contribution!

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix leaktest failure

Append '\0' at the tail of converted string.

Benjamin, could you please review this again?

Thank you for your patience.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Full leaktest log with the new patch:

$ python leaktest.py
INFO | automation.py | Application pid: 3644
WARNING: NS_ENSURE_TRUE(compMgr) failed: file c:/cygwin/home/user/hg/mozilla-cen
tral/objdir-firefox/xpcom/build/nsComponentManagerUtils.cpp, line 90
*** Registering components in: Apprunner
*** Registering components in: nsAccessibilityModule
*** Registering components in: application
*** Registering components in: appshell
*** Registering components in: nsAuthModule
*** Registering components in: nsAutoConfigModule
*** Registering components in: BrowserDirProvider
*** Registering components in: nsBrowserCompsModule
*** Registering components in: nsSecurityManagerModule
*** Registering components in: nsChardetModule
*** Registering components in: nsChromeModule
*** Registering components in: CommandLineModule
*** Registering components in: nsComposerModule
*** Registering components in: nsCookieModule
*** Registering components in: docshell_provider
*** Registering components in: embedcomponents
*** Registering components in: nsGfxModule
*** Registering components in: nsLayoutModule
*** Registering components in: nsParserModule
*** Registering components in: nsPluginModule
*** Registering components in: nsWidgetModule
*** Registering components in: nsI18nModule
*** Registering components in: nsIconDecoderModule
*** Registering components in: nsImageLib2Module
*** Registering components in: nsXPIntlModule
*** Registering components in: nsJarModule
*** Registering components in: JavaScript_Debugger
*** Registering components in: nsFindComponent
*** Registering components in: MyService
*** Registering components in: necko
*** Registering components in: nsPermissionsModule
*** Registering components in: BOOT
*** Registering components in: NSS
*** Registering components in: PKI
*** Registering components in: nsPlacesModule
*** Registering components in: nsRDFModule
*** Registering components in: satchel
*** Registering components in: mozSpellCheckerModule
*** Registering components in: mozStorageModule
*** Registering components in: nsTestDynamicModule
*** Registering components in: tkAutoCompleteModule
*** Registering components in: nsToolkitCompsModule
*** Registering components in: nsTransactionManagerModule
*** Registering components in: nsUConvModule
*** Registering components in: nsUCvMathModule
*** Registering components in: nsUniversalCharDetModule
*** Registering components in: Browser_Embedding_Module
*** Registering components in: nsWindowDataSourceModule
*** Registering components in: nsWindowsProxyModule
*** Registering components in: xpconnect
*** Registering components in: nsSampleModule
*** Registering components in: xpconnect_test
*** Registering components in: nsSoftwareUpdate
*** Registering components in: nsPrefModule
*** Registering components in: ZipWriterModule
*** Registering components in: Apprunner
pldhash: for the table at address 04A25B60, the given entrySize of 48 probably f
avors chaining over double hashing.
++DOCSHELL 04B20878 == 1
++DOMWINDOW == 1 (04B38E80) [serial = 1] [outer = 00000000]
*** registering aboutCertError.js: [ About Cert Error ]
*** registering aboutPrivateBrowsing.js: [ about:privatebrowsing ]
*** registering aboutRights.js: [ about:rights ]
*** registering aboutRobots.js: [ About Robots ]
*** registering aboutSessionRestore.js: [ about:sessionrestore ]
*** loading ISO8601DateUtils
*** registering FeedProcessor.js: [ Feed Processor, Feed Result, Feed, Feed Entr
y, Feed Text Construct, Feed Generator, Feed Person ]
*** registering FeedWriter.js: [ Feed Writer ]
*** registering fuelApplication.js: [ Application ]
*** registering jsconsole-clhandler.js: [ jsConsoleHandler ]
*** registering NetworkGeolocationProvider.js: [ A component that returns a geol
ocation based on WIFI ]
*** registering nsAddonRepository.js: [ Addon Repository ]
*** registering nsBadCertHandler.js: [ XMLHttpRequest Bad Cert Handler ]
*** registering nsBlocklistService.js: [ Blocklist Service ]
*** registering nsBrowserGlue.js: [ Firefox Browser Glue Service, Geolocation Pr
ompting Component ]
*** registering nsContentDispatchChooser.js: [ Used to handle different types of
 content ]
*** registering nsContentPrefService.js: [ Content Pref Service, Hostname Groupe
r ]
*** registering nsDownloadManagerUI.js: [ Used to show the Download Manager's UI
 to the user ]
*** registering nsExtensionManager.js: [ Extension Manager, Extension Manager Da
ta Source, Update Item ]
*** registering nsHandlerService.js: [ Handler Service ]
*** registering nsLoginInfo.js: [ LoginInfo ]
*** registering nsLoginManager.js: [ LoginManager ]
*** registering nsLoginManagerPrompter.js: [ LoginManagerPromptFactory, LoginMan
agerPrompter ]
*** registering nsMicrosummaryService.js: [ Microsummary Service ]
*** registering nsPlacesDBFlush.js: [ Used to synchronize the temporary and perm
anent tables of Places ]
*** registering nsPlacesTransactionsService.js: [ Places Transaction Manager ]
*** registering nsPrivateBrowsingService.js: [ PrivateBrowsing Service ]
*** Deferring registration of sample JS components
*** registering nsSearchSuggestions.js: [ Remote Search Suggestions ]
*** registering nsSessionStartup.js: [ Browser Session Startup Service ]
*** registering nsSessionStore.js: [ Browser Session Store Service ]
*** registering nsSetDefaultBrowser.js: [ Default Browser Cmdline Handler ]
*** registering nsTaggingService.js: [ Places Tagging Service, Places Tag AutoCo
mplete ]
*** registering nsTryToClose.js: [ tryToClose Service ]
*** registering nsUpdateService.js: [ Update Service, Update Checker, Update Pro
mpt, Timer Manager, Update Manager ]
*** registering nsURLFormatter.js: [ Application URL Formatter Service ]
*** registering nsWebHandlerApp.js: [ A web handler for protocols and content ]
*** registering storage-Legacy.js: [ LoginManagerStorage_legacy ]
*** registering storage-mozStorage.js: [ LoginManagerStorage_mozStorage ]
*** registering TestGeolocationProvider.js: [ A component that returns a geoloca
tion ]
*** registering WebContentConverter.js: [ Web Content Handler Registrar ]
*** Registering sample JS components
WARNING: EM tried to force us to restart twice! Forcefully preventing that.: fil
e c:/cygwin/home/user/hg/mozilla-central/toolkit/xre/nsAppRunner.cpp, line 3268
++DOCSHELL 0500BA30 == 2
++DOMWINDOW == 2 (0500C6F8) [serial = 2] [outer = 00000000]
++DOMWINDOW == 3 (05027508) [serial = 3] [outer = 0500C6C8]
++DOMWINDOW == 4 (00E5F080) [serial = 4] [outer = 04B38E50]
++DOCSHELL 05314D10 == 3
++DOMWINDOW == 5 (05318358) [serial = 5] [outer = 00000000]
++DOCSHELL 04FC8558 == 4
WARNING: NS_ENSURE_TRUE(browserChrome) failed: file c:/cygwin/home/user/hg/mozil
la-central/docshell/base/nsDocShell.cpp, line 9923
WARNING: Something wrong when creating the docshell for a frameloader!: file c:/
cygwin/home/user/hg/mozilla-central/content/base/src/nsFrameLoader.cpp, line 901

WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file c:/cygwin
/home/user/hg/mozilla-central/content/base/src/nsFrameLoader.cpp, line 925
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file c:/cygwin
/home/user/hg/mozilla-central/content/base/src/nsFrameLoader.cpp, line 182
++DOCSHELL 053941F8 == 5
++DOMWINDOW == 6 (053D3F00) [serial = 6] [outer = 00000000]
++DOMWINDOW == 7 (054947E0) [serial = 7] [outer = 05318328]
++DOMWINDOW == 8 (054A0618) [serial = 8] [outer = 053D3ED0]
localhost - - [18/May/2009 11:25:06] "GET /bloatcycle.html HTTP/1.1" 200 -
++DOMWINDOW == 9 (056D45B0) [serial = 9] [outer = 053D3ED0]
++DOCSHELL 0570BDD0 == 6
++DOMWINDOW == 10 (0570C6E0) [serial = 10] [outer = 00000000]
++DOMWINDOW == 11 (057126B0) [serial = 11] [outer = 0570C6B0]
WARNING: NS_ENSURE_TRUE(aURI) failed: file c:/cygwin/home/user/hg/mozilla-centra
l/docshell/base/nsDefaultURIFixup.cpp, line 76
WARNING: NS_ENSURE_TRUE(mMutable) failed: file c:/cygwin/home/user/hg/mozilla-ce
ntral/netwerk/base/src/nsSimpleURI.cpp, line 224
localhost - - [18/May/2009 11:25:06] code 404, message File not found
localhost - - [18/May/2009 11:25:06] "GET /favicon.ico HTTP/1.1" 404 -
WARNING: NS_ENSURE_TRUE(mMutable) failed: file c:/cygwin/home/user/hg/mozilla-ce
ntral/netwerk/base/src/nsSimpleURI.cpp, line 224
localhost - - [18/May/2009 11:25:09] code 404, message File not found
localhost - - [18/May/2009 11:25:09] "GET /favicon.ico HTTP/1.1" 404 -
++DOMWINDOW == 12 (0933CB00) [serial = 12] [outer = 0570C6B0]
JavaScript strict warning: chrome://browser/content/places/utils.js, line 1322:
reference to undefined property aPopup.childNodes[aPopup._startMarker + 1]
++DOMWINDOW == 13 (096C03F8) [serial = 13] [outer = 0570C6B0]
++DOMWINDOW == 14 (09405688) [serial = 14] [outer = 0570C6B0]
--DOMWINDOW == 13 (054A0618) [serial = 8] [outer = 053D3ED0] [url = about:blank]

--DOMWINDOW == 12 (057126B0) [serial = 11] [outer = 0570C6B0] [url = about:blank
]
--DOMWINDOW == 11 (0933CB00) [serial = 12] [outer = 0570C6B0] [url = http://www.
mozilla.org/]
++DOMWINDOW == 12 (096F1D60) [serial = 15] [outer = 0570C6B0]
++DOMWINDOW == 13 (09519548) [serial = 16] [outer = 0570C6B0]
++DOMWINDOW == 14 (09670F90) [serial = 17] [outer = 0570C6B0]
--DOMWINDOW == 13 (096C03F8) [serial = 13] [outer = 0570C6B0] [url = http://www.
mozilla.org/]
--DOCSHELL 05314D10 == 5
###!!! ASSERTION: Why are we creating new state when disabled?: '!gTimelineDisab
led', file c:/cygwin/home/user/hg/mozilla-central/xpcom/ds/nsTimelineService.cpp
, line 196
00000.969 (00db3c70): ...main1
--DOCSHELL 04B20878 == 4
--DOMWINDOW == 12 (09670F90) [serial = 17] [outer = 0570C6B0] [url = http://www.
mozilla.org/newlayout/samples/test8.html]
--DOMWINDOW == 11 (09519548) [serial = 16] [outer = 0570C6B0] [url = http://www.
mozilla.org/newlayout/samples/test6.html]
--DOMWINDOW == 10 (04B38E80) [serial = 1] [outer = 00000000] [url = resource://g
re/res/hiddenWindow.html]
--DOMWINDOW == 9 (096F1D60) [serial = 15] [outer = 0570C6B0] [url = http://www.m
ozilla.org/newlayout/samples/test8.html]
--DOMWINDOW == 8 (054947E0) [serial = 7] [outer = 05318328] [url = about:blank]
--DOMWINDOW == 7 (00E5F080) [serial = 4] [outer = 00000000] [url = resource://gr
e/res/hiddenWindow.html]
--DOMWINDOW == 6 (05318358) [serial = 5] [outer = 00000000] [url = about:blank]
--DOCSHELL 0570BDD0 == 3
--DOMWINDOW == 5 (09405688) [serial = 14] [outer = 0570C6B0] [url = http://www.m
ozilla.org/newlayout/samples/test2.html]
--DOMWINDOW == 4 (0570C6E0) [serial = 10] [outer = 00000000] [url = http://www.m
ozilla.org/newlayout/samples/test8.html]
--DOCSHELL 053941F8 == 2
--DOMWINDOW == 3 (053D3F00) [serial = 6] [outer = 00000000] [url = http://localh
ost:8888/bloatcycle.html]
--DOMWINDOW == 2 (056D45B0) [serial = 9] [outer = 00000000] [url = http://localh
ost:8888/bloatcycle.html]
--DOCSHELL 0500BA30 == 1
xpcom_core!NS_TimelineMarkV+0x000000000000001E (c:\cygwin\home\user\hg\mozilla-c
entral\xpcom\ds\nstimelineservice.cpp, line 321)
xpcom_core!NS_TimelineForceMark+0x0000000000000017 (c:\cygwin\home\user\hg\mozil
la-central\xpcom\ds\nstimelineservice.cpp, line 335)
xul!XRE_main+0x00000000000030C2 (c:\cygwin\home\user\hg\mozilla-central\toolkit\
xre\nsapprunner.cpp, line 3364)
firefox!NS_internal_main+0x00000000000002B2 (c:\cygwin\home\user\hg\mozilla-cent
ral\browser\app\nsbrowserapp.cpp, line 156)
firefox!wmain+0x0000000000000119 (c:\cygwin\home\user\hg\mozilla-central\toolkit
\xre\nswindowswmain.cpp, line 114)
firefox!__tmainCRTStartup+0x00000000000001A8 (f:\dd\vctools\crt_bld\self_x86\crt
\src\crtexe.c, line 583)
firefox!wmainCRTStartup+0x000000000000000F (f:\dd\vctools\crt_bld\self_x86\crt\s
rc\crtexe.c, line 403)
kernel32!RegisterWaitForInputIdle+0x0000000000000049
WARNING: nsExceptionService ignoring thread destruction after shutdown: file c:/
cygwin/home/user/hg/mozilla-central/xpcom/base/nsExceptionService.cpp, line 194
WARNING: NS_ENSURE_TRUE(factory) failed: file c:/cygwin/home/user/hg/mozilla-cen
tral/docshell/base/nsDocShell.cpp, line 9920
--DOCSHELL 04FC8558 == 0
--DOMWINDOW == 1 (05027508) [serial = 3] [outer = 0500C6C8] [url = about:blank]
--DOMWINDOW == 0 (0500C6F8) [serial = 2] [outer = 00000000] [url = chrome://brow
ser/content/browser.xul]
WARNING: not an nsIRDFRemoteDataSource: 'remote != nsnull', file c:/cygwin/home/
user/hg/mozilla-central/rdf/datasource/src/nsLocalStore.cpp, line 312
nsStringStats
 => mAllocCount:          41477
 => mReallocCount:         4701
 => mFreeCount:           41477
 => mShareCount:          41981
 => mAdoptCount:           2605
 => mAdoptFreeCount:       2605
INFO | automation.py | Application ran for: 0:00:31.484000

Comment 6

[[e-mail]]

Comment on attachment 377989 [details] [diff] [review]
Fix leaktest failure

>   int len = wcslen(arg);
>   char *s = new char[len * 3 + 1];
>   if (!s)
>     return NULL;
> 
>+  utf8_len = WideCharToMultiByte(CP_UTF8, 0, arg, len, s, len * 3 + 1, 0, 0);
This code would appear to fail when passed the empty string "" as its length is zero which is an illegal value for the cchWideChar parameter.

One option is to simply return the empty string on any failure, i.e.
s[WideCharToMultiByte(CP_UTF8, 0, arg, len, s, len * 3 + 1, 0, 0)] = 0;

The other suggestion mentioned in MSDN is simply to include the null terminator in the count of characters to be processed, i.e.
WideCharToMultiByte(CP_UTF8, 0, arg, len + 1, s, len * 3 + 1, 0, 0);

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Hiroyuki, are you still working on this?

Comment 8

[Hiroyuki Ikezoe (:hiro)]

No, I have no build environment for Windows now. Sorry.

Comment 9

[Hiroyuki Ikezoe (:hiro)]

CCing ABE Hiroki.

Hi Hiroki san, can you take time to fix this?

Comment 10

[ABE Hiroki (:hATrayflood)]

Attachment: patch v2

Ikezoe-san: Please tell me how to run leaktest.

Comment 11

[Hiroyuki Ikezoe (:hiro)]

(In reply to ABE Hiroki (hATrayflood) from comment #10)
> Created attachment 552030 [details] [diff] [review]
> patch v2

Woohoo, thank you very much again!

> Ikezoe-san: Please tell me how to run leaktest.

In this case, I suppose running debug build of firefox with some arguments (e.g. firefox -safe-mode) is sufficient for check the memory leak.

You can get the log like the following at exit:

 => mAllocCount:           7672
 => mReallocCount:          851
 => mFreeCount:            7672
 => mShareCount:          11497
 => mAdoptCount:            636
 => mAdoptFreeCount:        636

Comment 12

[Benjamin Smedberg]

Since we have absolutely no desire to resurrect WinCE support, why are we even bothering to work on this patch? Can't this just be WONTFIX?

Comment 13

[ABE Hiroki (:hATrayflood)]

Attachment: leaktest log

Comment 14

[ABE Hiroki (:hATrayflood)]

> In this case, I suppose running debug build of firefox with some arguments
> (e.g. firefox -safe-mode) is sufficient for check the memory leak.
> 
> You can get the log like the following at exit:
> 
>  => mAllocCount:           7672
>  => mReallocCount:          851
>  => mFreeCount:            7672
>  => mShareCount:          11497
>  => mAdoptCount:            636
>  => mAdoptFreeCount:        636

thank you.
and leaktest result.

 => mAllocCount:          32914
 => mReallocCount:         3572
 => mFreeCount:           32914
 => mShareCount:          42825
 => mAdoptCount:           2163
 => mAdoptFreeCount:       2163

Comment 15

[Benjamin Smedberg]

XULRunner has been removed from the Mozilla tree: see https://groups.google.com/forum/#!topic/mozilla.dev.platform/_rFMunG2Bgw for context.

I am closing all the bugs currently in the XULRunner bugzilla component, in preparation for moving this component to the graveyard. If this bug is still valid in a XULRunner-less world, it will need to be moved to a different bugzilla component to be reopened.