Closed Bug 1287002 Opened 8 years ago Closed 8 years ago

Glue for COM IPC

Categories

(Core :: IPC, defect)

Product:
Core
Component:
IPC
Platform:
Unspecified
Windows
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla50
Tracking Flags:
Tracking Status
firefox50 --- fixed

People

(Reporter: bugzilla, Assigned: bugzilla)

References

Details

Attachments

(4 files)

Bug 1287002: Skeleton moz.build for ipc/mscom directory;
58 bytes, text/x-review-board-request
billm
: review+
Details
Bug 1287002: Add RAII class for entering a COM apartment;
58 bytes, text/x-review-board-request
jimm
: review+
Details
Bug 1287002: Utility functions for Microsoft COM glue;
58 bytes, text/x-review-board-request
jimm
: review+
Details
Bug 1287002: Add DynamicallyLinkedFunctionPtr helper class to mscom glue;
58 bytes, text/x-review-board-request
jimm
: review+
Details 
This bug is for landing a foundation for additional glue code later on. I talked to Bill about where he'd like this to go, and he suggested ipc/mscom.
Review commit: https://reviewboard.mozilla.org/r/64454/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64454/
Attachment #8771191 - Flags: review?(wmccloskey)
Attachment #8771192 - Flags: review?(jmathies)
Attachment #8771193 - Flags: review?(jmathies)
Attachment #8771194 - Flags: review?(jmathies)
Comment on attachment 8771192 [details]
Bug 1287002: Add RAII class for entering a COM apartment;

https://reviewboard.mozilla.org/r/64456/#review61598
Attachment #8771192 - Flags: review?(jmathies) → review+
Comment on attachment 8771193 [details]
Bug 1287002: Utility functions for Microsoft COM glue;

https://reviewboard.mozilla.org/r/64458/#review61600

::: ipc/mscom/moz.build:13
(Diff revision 1)
>      'COMApartmentRegion.h',
> +    'utils.h',
>  ]
>  
>  UNIFIED_SOURCES += [
> +    'utils.cpp',

nit - I think I'd prefer Utils.* vs. utils.*
Attachment #8771193 - Flags: review?(jmathies) → review+
https://reviewboard.mozilla.org/r/64460/#review61602

This seems like it would be useful generally, maybe we should put this in the nsWindowsHelpers file?
Comment on attachment 8771194 [details]
Bug 1287002: Add DynamicallyLinkedFunctionPtr helper class to mscom glue;

https://reviewboard.mozilla.org/r/64460/#review61614

code looks fine regardless of where you put it.
Attachment #8771194 - Flags: review?(jmathies) → review+
(In reply to Jim Mathies [:jimm] from comment #6)
> Comment on attachment 8771193 [details]
> Bug 1287002: Utility functions for Microsoft COM glue;
> 
> https://reviewboard.mozilla.org/r/64458/#review61600
> 
> ::: ipc/mscom/moz.build:13
> (Diff revision 1)
> >      'COMApartmentRegion.h',
> > +    'utils.h',
> >  ]
> >  
> >  UNIFIED_SOURCES += [
> > +    'utils.cpp',
> 
> nit - I think I'd prefer Utils.* vs. utils.*

I do too, but unfortunately hg histedit doesn't like me doing that in the middle of a patch queue. I'll rename it in a future commit that doesn't have a bunch of other stuff layered on top of it, if that's okay.

(In reply to Jim Mathies [:jimm] from comment #7)
> https://reviewboard.mozilla.org/r/64460/#review61602
> 
> This seems like it would be useful generally, maybe we should put this in
> the nsWindowsHelpers file?

I was thinking that, however I was unsure whether we'd want to keep it Windows only or make it more generic. At any rate, moving it right now is also a big pain in the butt due to my patch queue, so I'd also prefer to file a follow-up on this.
I filed bug 1287167 for ipc/mscom nits and cleanup.
(In reply to Aaron Klotz [:aklotz] from comment #9)
> I do too, but unfortunately hg histedit doesn't like me doing that in the
> middle of a patch queue. I'll rename it in a future commit that doesn't have
> a bunch of other stuff layered on top of it, if that's okay.

IIRC, hg renames that only affect capitalization do bad things on Windows, so you probably want to avoid that.
(In reply to Andrew McCreight [:mccr8] from comment #11)
> (In reply to Aaron Klotz [:aklotz] from comment #9)
> > I do too, but unfortunately hg histedit doesn't like me doing that in the
> > middle of a patch queue. I'll rename it in a future commit that doesn't have
> > a bunch of other stuff layered on top of it, if that's okay.
> 
> IIRC, hg renames that only affect capitalization do bad things on Windows,
> so you probably want to avoid that.

Looks like I was able to do it by starting at the top of my patch queue and propagating the change down to the bottom.
Comment on attachment 8771191 [details]
Bug 1287002: Skeleton moz.build for ipc/mscom directory;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64454/diff/1-2/
Comment on attachment 8771192 [details]
Bug 1287002: Add RAII class for entering a COM apartment;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64456/diff/1-2/
Comment on attachment 8771193 [details]
Bug 1287002: Utility functions for Microsoft COM glue;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64458/diff/1-2/
Comment on attachment 8771194 [details]
Bug 1287002: Add DynamicallyLinkedFunctionPtr helper class to mscom glue;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64460/diff/1-2/
Pushed by aklotz@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9c96b9d1b060
Skeleton moz.build for ipc/mscom directory; r=billm
https://hg.mozilla.org/integration/autoland/rev/1021393b4763
Add RAII class for entering a COM apartment; r=jimm
https://hg.mozilla.org/integration/autoland/rev/88553f61add4
Utility functions for Microsoft COM glue; r=jimm
https://hg.mozilla.org/integration/autoland/rev/521b6b0a9b39
Add DynamicallyLinkedFunctionPtr helper class to mscom glue; r=jimm
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: