Closed Bug 770535 Opened 10 years ago Closed 8 years ago

Remove XPConnect locks

Categories

(Core :: XPConnect, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: bholley, Assigned: mccr8)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

I split this off into its own bug.
Depends on: 770840
Blocks: 839535, 843923
Blocks: 911303
Not strictly related, but a simple cleanup related to making it single threaded.
These two maps are identical with the removal of locks, so fold them together.
Attachment #8337420 - Flags: review?(bobbyholley+bmo)
Attachment #8337421 - Flags: review?(bobbyholley+bmo)
Comment on attachment 8337422 [details] [diff] [review]
part 3: Fold mMainThreadWrappedNativeProtoMap into mWrappedNativeProtoMap

This third patch enjoys a cute sort of bisimulation property.  I initially wrote it as two patches, one that deletes the non-mainthread-only map, then another that renames the mainthread-only to the same name as the non-mainthread-only, but folding them together results in this exact patch, modulo one bit of whitespace, thereby demonstrating the equivalence of the two maps.
Attachment #8337422 - Flags: review?(bobbyholley+bmo)
Comment on attachment 8337420 [details] [diff] [review]
part 1 - remove the locks

Review of attachment 8337420 [details] [diff] [review]:
-----------------------------------------------------------------

Huzzah! Thanks for doing this.
Attachment #8337420 - Flags: review?(bobbyholley+bmo) → review+
Attachment #8337421 - Flags: review?(bobbyholley+bmo) → review+
Attachment #8337422 - Flags: review?(bobbyholley+bmo) → review+
The errors are this:

c:\builds\moz2_slave\m-in-w32-d-0000000000000000000\build\js\xpconnect\src\XPCJSRuntime.cpp(2934) : error C2220: warning treated as error - no 'object' file generated
c:\builds\moz2_slave\m-in-w32-d-0000000000000000000\build\js\xpconnect\src\XPCJSRuntime.cpp(2934) : warning C4355: 'this' : used in base member initializer list

c:\builds\moz2_slave\m-in-w32-d-0000000000000000000\build\js\xpconnect\src\XPCWrappedJS.cpp(347) : error C2220: warning treated as error - no 'object' file generated
c:\builds\moz2_slave\m-in-w32-d-0000000000000000000\build\js\xpconnect\src\XPCWrappedJS.cpp(347) : warning C4355: 'this' : used in base member initializer list

The two lines in question are
  mWatchdogManager(new WatchdogManager(this)),
and
  mRoot(root ? root : this),

which, okay, do use |this|, so it looks like a legit error message, but I didn't change anything in those lines.  My current theory is that something changed in the build configuration to enable warnings as errors for xpconnect, and nobody has touched these files since then.
Warnings as errors has been on for this directory since August, so that can't be it.  Maybe something about the way it is run or isn't run on Windows changed?  We'll see if the backout builds on Windows.
No, it's because you removed the MSVC pragma in part 1. That probably needs to go back, unless passing |this| is actually unsafe for some reason.
Oh, thanks for identifying that!  I just blindly deleted that entire section. ;)
(In reply to Andrew McCreight [:mccr8] from comment #13)
> Oh, thanks for identifying that!  I just blindly deleted that entire
> section. ;)

Fwiw, you can also drop the pragma and use MOZ_THIS_IN_INITIALIZER_LIST().
You need to log in before you can comment on or make changes to this bug.