Closed
Bug 1219482
Opened 9 years ago
Closed 9 years ago
Replace PRLogModuleInfo usage with LazyLogModule in various remaining subdirectories
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla47
People
(Reporter: erahm, Assigned: sajitk)
References
Details
Attachments
(4 files, 8 obsolete files)
35.45 KB,
patch
|
Details | Diff | Splinter Review | |
23.10 KB,
patch
|
Details | Diff | Splinter Review | |
13.61 KB,
patch
|
Details | Diff | Splinter Review | |
12.74 KB,
patch
|
Details | Diff | Splinter Review |
This covers replacing the few remaining instances of PRLogModuleInfo w/ LazyLogModule in everything in various subdirectories.
Current usage:
> ./docshell/base/nsDocShell.cpp:818: gDocShellLog = PR_NewLogModule("nsDocShell");
> ./docshell/base/nsDocShell.cpp:822: gDocShellLeakLog = PR_NewLogModule("nsDocShellLeak");
> ./docshell/shistory/nsSHistory.cpp:67: sLog = PR_NewLogModule("nsSHistory");
> ./extensions/auth/nsAuthFactory.cpp:229: gNegotiateLog = PR_NewLogModule("negotiateauth");
> ./extensions/gio/nsGIOProtocolHandler.cpp:914: sGIOLog = PR_NewLogModule("gio");
> ./extensions/pref/autoconfig/src/nsReadConfig.cpp:79: MCD = PR_NewLogModule("MCD");
> ./hal/Hal.cpp:69: sHalLog = PR_NewLogModule("hal");
> ./ipc/chromium/src/base/logging.cc:66: gChromiumPRLog = PR_NewLogModule("chromium");
> ./js/xpconnect/loader/mozJSComponentLoader.cpp:201: gJSCLLog = PR_NewLogModule("JSComponentLoader");
> ./js/xpconnect/src/nsXPConnect.cpp:237: gJSDiagnostics = PR_NewLogModule("JSDiagnostics");
> ./js/xpconnect/src/XPCLog.cpp:32: g_LogMod = PR_NewLogModule("xpclog");
> ./modules/libjar/nsJARChannel.cpp:209: gJarProtocolLog = PR_NewLogModule("nsJarProtocol");
> ./parser/htmlparser/nsExpatDriver.cpp:45: sLog = PR_NewLogModule("expatdriver");
> ./rdf/base/nsCompositeDataSource.cpp:494: nsRDFLog = PR_NewLogModule("RDF");
> ./rdf/base/nsInMemoryDataSource.cpp:748: gLog = PR_NewLogModule("InMemoryDataSource");
> ./rdf/base/nsRDFContentSink.cpp:295: gLog = PR_NewLogModule("nsRDFContentSink");
> ./rdf/base/nsRDFService.cpp:752: gLog = PR_NewLogModule("nsRDFService");
> ./rdf/base/nsRDFXMLDataSource.cpp:401: gLog = PR_NewLogModule("nsRDFXMLDataSource");
> ./security/certverifier/CertVerifier.cpp:55: gCertVerifierLog = PR_NewLogModule("certverifier");
> ./security/manager/ssl/CertBlocklist.cpp:130: gCertBlockPRLog = PR_NewLogModule("CertBlock");
> ./security/manager/ssl/nsNSSComponent.cpp:235: gPIPNSSLog = PR_NewLogModule("pipnss");
> ./security/manager/ssl/nsNTLMAuthModule.cpp:37: sNTLMLog = PR_NewLogModule("NTLM");
> ./security/manager/ssl/nsSecureBrowserUIImpl.cpp:121: gSecureDocLog = PR_NewLogModule("nsSecureBrowserUI");
> ./security/manager/ssl/nsSecurityHeaderParser.cpp:54: sSHParserLog = PR_NewLogModule("nsSecurityHeaderParser");
> ./security/manager/ssl/nsSiteSecurityService.cpp:48: gSSSLog = PR_NewLogModule("nsSSService");
> ./security/manager/ssl/PublicKeyPinningService.cpp:27: PR_NewLogModule("PublicKeyPinningService");
> ./security/manager/ssl/RootCertificateTelemetryUtils.cpp:17: PR_NewLogModule("PublicKeyPinningTelemetryService");
> ./storage/mozStorageConnection.cpp:737: gStorageLog = ::PR_NewLogModule("mozStorage");
> ./toolkit/components/downloads/ApplicationReputation.cpp:1133: prlog = PR_NewLogModule("ApplicationReputation");
> ./toolkit/components/filewatcher/NativeFileWatcherWin.cpp:139: gNativeWatcherPRLog = PR_NewLogModule("NativeFileWatcherService");
> ./toolkit/components/url-classifier/nsUrlClassifierDBService.cpp:1120: gUrlClassifierDbServiceLog = PR_NewLogModule("UrlClassifierDbService");
> ./toolkit/components/url-classifier/nsUrlClassifierPrefixSet.cpp:41: gUrlClassifierPrefixSetLog = PR_NewLogModule("UrlClassifierPrefixSet");
> ./toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp:44: gUrlClassifierStreamUpdaterLog = PR_NewLogModule("UrlClassifierStreamUpdater");
> ./toolkit/components/url-classifier/ProtocolParser.cpp:81: PR_NewLogModule("UrlClassifierProtocolParser");
> ./toolkit/xre/nsUpdateDriver.cpp:79: sUpdateLog = PR_NewLogModule("updatedriver");
> ./uriloader/base/nsDocLoader.cpp:119: gDocLoaderLog = PR_NewLogModule("DocLoader");
> ./uriloader/base/nsURILoader.cpp:758: mLog = PR_NewLogModule("URILoader");
> ./uriloader/exthandler/nsExternalHelperAppService.cpp:659: mLog = PR_NewLogModule("HelperAppService");
> ./uriloader/prefetch/nsOfflineCacheUpdateService.cpp:299: gOfflineCacheUpdateLog = PR_NewLogModule("nsOfflineCacheUpdate");
> ./uriloader/prefetch/nsPrefetchService.cpp:337: gPrefetchLog = PR_NewLogModule("nsPrefetch");
Attachment #8693401 -
Flags: review?(benjamin)
Attachment #8693401 -
Attachment is obsolete: true
Attachment #8693401 -
Flags: review?(benjamin)
Attachment #8693402 -
Flags: review?(benjamin)
Attachment #8693403 -
Flags: review?(nfroyd)
Attachment #8693404 -
Flags: review?(dougt)
Attachment #8693406 -
Flags: review?(erahm)
![]() |
||
Updated•9 years ago
|
Attachment #8693403 -
Flags: review?(nfroyd) → review+
Reporter | ||
Comment 6•9 years ago
|
||
Comment on attachment 8693406 [details] [diff] [review]
1219482.Part4.patch
Review of attachment 8693406 [details] [diff] [review]:
-----------------------------------------------------------------
Thank you for the patch. r=me
Attachment #8693406 -
Flags: review?(erahm) → review+
Attachment #8693402 -
Flags: review?(benjamin) → review?(nfroyd)
Attachment #8693404 -
Flags: review?(dougt) → review?(erahm)
Reporter | ||
Comment 7•9 years ago
|
||
Comment on attachment 8693404 [details] [diff] [review]
1219482.Part3.patch
Review of attachment 8693404 [details] [diff] [review]:
-----------------------------------------------------------------
r=me
Attachment #8693404 -
Flags: review?(erahm) → review+
Updated•9 years ago
|
Attachment #8693402 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 10•9 years ago
|
||
Assignee | ||
Comment 11•9 years ago
|
||
Hi, I am getting test failures related to these changes, and they seem to be related to this one in particular:
> ./toolkit/xre/nsUpdateDriver.cpp:79: sUpdateLog = PR_NewLogModule("updatedriver");
The test failures are triggered by an assertion failure in LogModule::Get() function
> MOZ_ASSERT(sLogModuleManager != nullptr);
This implies that the LogModule::Init() is not getting called in the tests.
For example, the test toolkit/mozapps/update/tests/unit_base_updater/marAppApplyUpdateStageSuccess fails and the stack trace is
>#0 mozilla::LogModule::Get (aName=0x7fffebb6959c "updatedriver")
> at /home/firefox-dev/mozilla-central/xpcom/base/Logging.cpp:80
>#1 0x00007fffe4fbb341 in mozilla::LazyLogModule::operator mozilla::LogModule*
> (this=0x7fffee85e960 <GetUpdateLog()::sUpdateLog>)
> at ../../dist/include/mozilla/Logging.h:117
>#2 0x00007fffe9343d0d in GetUpdateLog ()
> at /home/firefox-dev/mozilla-central/toolkit/xre/nsUpdateDriver.cpp:78
>#3 0x00007fffe9344ee4 in SwitchToUpdatedApp (greDir=0x7ffff6b52900,
> updateDir=0x7ffff6b547c0, appDir=0x7ffff6b54700, appArgc=1,
> appArgv=0x7ffff6b457d0)
> at /home/firefox-dev/mozilla-central/toolkit/xre/nsUpdateDriver.cpp:633
>#4 0x00007fffe9345cf7 in ProcessUpdates (greDir=0x7ffff6b52900,
> appDir=0x7ffff6b54700, updRootDir=0x7ffff6b54640, argc=1,
> argv=0x7ffff6b457d0, appVersion=0x7ffff6b7f340 "45.0a1", restart=true,
> isOSUpdate=false, osApplyToDir=0x0, pid=0x0)
> at /home/firefox-dev/mozilla-central/toolkit/xre/nsUpdateDriver.cpp:1043
>#5 0x00007fffe9339e0e in XREMain::XRE_mainStartup (this=0x7fffffffc890,
> aExitFlag=0x7fffffffc802)
> at /home/firefox-dev/mozilla-central/toolkit/xre/nsAppRunner.cpp:3875
>#6 0x00007fffe933bfd2 in XREMain::XRE_main (this=0x7fffffffc890, argc=1,
> argv=0x7fffffffdd98, aAppData=0x7fffffffcab0)
> at /home/firefox-dev/mozilla-central/toolkit/xre/nsAppRunner.cpp:4368
>#7 0x00007fffe933c3c6 in XRE_main (argc=1, argv=0x7fffffffdd98,
> aAppData=0x7fffffffcab0, aFlags=0)
> at /home/firefox-dev/mozilla-central/toolkit/xre/nsAppRunner.cpp:4485
>#8 0x0000000000405894 in do_main (argc=1, argv=0x7fffffffdd98,
> xreDirectory=0x7ffff6b52900)
> at /home/firefox-dev/mozilla-central/browser/app/nsBrowserApp.cpp:212
>#9 0x0000000000405c93 in main (argc=1, argv=0x7fffffffdd98)
> at /home/firefox-dev/mozilla-central/browser/app/nsBrowserApp.cpp:352
>
There are no calls to LogModule::Init in nsBrowserApp.cpp or nsApprunner.cpp
Is a further fix then needed, that the LogModule::Init be put in all application entry paths? Or at least the places where NS_LogInit() is called?
Appreciate the help.
Flags: needinfo?(nfroyd)
![]() |
||
Comment 12•9 years ago
|
||
I'm guessing that this bit of code is getting called even prior to LogModule::Init(), because XPCOM has not been initialized at this point in the code. Maybe calling LogModule::Init() hereabouts:
https://dxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#4332
You'll notice that the call to XRE_mainStartup in the above stack is here:
https://dxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#4368
and XPCOM doesn't start until here:
https://dxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#4375
You may find yourself needing to add LogModule::Init() everywhere NS_LogInit is called.
Flags: needinfo?(nfroyd)
Assignee | ||
Comment 13•9 years ago
|
||
Reporter | ||
Comment 14•9 years ago
|
||
satjik do you plan on continuing with this bug? Let us know if you have any further questions.
Flags: needinfo?(sajitk)
Assignee | ||
Comment 15•9 years ago
|
||
Hi,
Unfortunately, I have not been able to get the build working for this particular instance of the PR_NewLogModule use:
> ./toolkit/xre/nsUpdateDriver.cpp:79: sUpdateLog = PR_NewLogModule("updatedriver");
With your permission, I was thinking of creating a new bug for tracking this usage, and checking all the rest in. Then someone with more knowledge about the build and link process can resolve it.
Please let me know if this is OK.
Flags: needinfo?(sajitk) → needinfo?(erahm)
Reporter | ||
Comment 16•9 years ago
|
||
(In reply to sajitk from comment #15)
> Hi,
> Unfortunately, I have not been able to get the build working for this
> particular instance of the PR_NewLogModule use:
> > ./toolkit/xre/nsUpdateDriver.cpp:79: sUpdateLog = PR_NewLogModule("updatedriver");
>
> With your permission, I was thinking of creating a new bug for tracking this
> usage, and checking all the rest in. Then someone with more knowledge about
> the build and link process can resolve it.
>
> Please let me know if this is OK.
That sounds reasonable, thank you for working through all this!
Flags: needinfo?(erahm)
Assignee | ||
Comment 17•9 years ago
|
||
Attachment #8693402 -
Attachment is obsolete: true
Assignee | ||
Comment 18•9 years ago
|
||
Attachment #8693403 -
Attachment is obsolete: true
Assignee | ||
Comment 19•9 years ago
|
||
Attachment #8713264 -
Attachment is obsolete: true
Assignee | ||
Comment 20•9 years ago
|
||
After fixing pull errors
Attachment #8713265 -
Attachment is obsolete: true
Assignee | ||
Comment 21•9 years ago
|
||
Removed entry in xre/toolkit causing errors.
Attachment #8693404 -
Attachment is obsolete: true
Assignee | ||
Comment 22•9 years ago
|
||
After resolving pull errors
Attachment #8693406 -
Attachment is obsolete: true
Assignee | ||
Comment 23•9 years ago
|
||
Keywords: checkin-needed
Comment 24•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c56aed63687
https://hg.mozilla.org/integration/mozilla-inbound/rev/7ec471c99263
https://hg.mozilla.org/integration/mozilla-inbound/rev/24af6caa9bba
https://hg.mozilla.org/integration/mozilla-inbound/rev/a1439ce8da77
Keywords: checkin-needed
Comment 25•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2c56aed63687
https://hg.mozilla.org/mozilla-central/rev/7ec471c99263
https://hg.mozilla.org/mozilla-central/rev/24af6caa9bba
https://hg.mozilla.org/mozilla-central/rev/a1439ce8da77
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Comment 26•9 years ago
|
||
This checkin seems to break b2g. See bug 124419.
![]() |
||
Comment 27•9 years ago
|
||
(In reply to Hubert Figuiere [:hub] from comment #26)
> This checkin seems to break b2g. See bug 124419.
That bug seems to be about windows scrolling or form control issues?
Flags: needinfo?(hub)
Comment 28•9 years ago
|
||
I meant bug 1244149 - not enough digits, but it was mentioned above without comments. "Depends on"
Flags: needinfo?(hub)
Comment 29•9 years ago
|
||
backout bugherder landing |
Backed out to see if it fixes the intermittent hazard build failures like https://treeherder.mozilla.org/logviewer.html#?job_id=20736122&repo=mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/155ce4441e22
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c16728755d3
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e5b76d86634
https://hg.mozilla.org/integration/mozilla-inbound/rev/ebc3fa14b72a
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 30•9 years ago
|
||
When you reland, please push bug 1244149 at the same time, or fold it into part 4. Thanks!
Assignee | ||
Comment 31•9 years ago
|
||
Attachment #8713272 -
Attachment is obsolete: true
Assignee | ||
Comment 32•9 years ago
|
||
Reporter | ||
Comment 33•9 years ago
|
||
Reporter | ||
Comment 34•9 years ago
|
||
Latest try build doesn't have hazard issues, I'm going to try relanding.
Reporter | ||
Comment 35•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/df2083106d8d69bcb8112ab95efe814103947ad9
Bug 1219482: Replace PRLogModuleInfo with LazyLogModule in various files.r=benjamin
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c19306be55e240d32c7b36f39a06b640c69fce5
Bug 1219482: Replace PRLogModuleInfo with LazyLogModule in security subdirectory.r=nfroyd
https://hg.mozilla.org/integration/mozilla-inbound/rev/8fbf8a036b71960beecf26f942cb960c69af95d9
Bug 1219482: Replace PRLogModuleInfo with LazyLogModule in toolkit subdirectory.r=erahm
https://hg.mozilla.org/integration/mozilla-inbound/rev/602a58ea423e0691f6cdc702dee98b318b91b485
Bug 1219482: Replace PRLogModuleInfo with LazyLogModule in uriloader subdirectory.r=erahm
Comment 36•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/df2083106d8d
https://hg.mozilla.org/mozilla-central/rev/5c19306be55e
https://hg.mozilla.org/mozilla-central/rev/8fbf8a036b71
https://hg.mozilla.org/mozilla-central/rev/602a58ea423e
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•