Closed Bug 773683 Opened 7 years ago Closed 7 years ago

Improve PCOMContentPermissionRequest usage safety

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: dougt, Assigned: wchen)

Details

Attachments

(1 file, 2 obsolete files)

From bug 761930 - jdm - 

Given that there are now so many implementations of PCOMContentPermissionRequest, I would like to propose some runtime asserts to help catch any misuses of the class. Specifically, add a member variable mIPCOpen that is initialized to true, and is set to false in a method that is called by TabChild::DeallocPContentPermissionRequest. Add a destructor to PCOMContentPermissionRequest that asserts that mIPCOpen is false, which should catch any cases when the request object dies before IPDL would expect it to.
We could even override PTabChild::SendPContentPermissionRequest to call a method that sets mIPCOpen to true, to ensure we don't have any problems with request objects that are created but not sent. Belt and suspenders, etc.
Assignee: nobody → wchen
Attachment #643045 - Flags: review?(doug.turner) → review?(josh)
Comment on attachment 643045 [details] [diff] [review]
Added runtime assertion to ensure PCOMContentPermissionRequestChild is live while protocol is live.

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

::: dom/ipc/PCOMContentPermissionRequestChild.h
@@ +25,5 @@
> +  PCOMContentPermissionRequestChild() : mIPCOpen(false) {}
> +  virtual ~PCOMContentPermissionRequestChild() {
> +    // mIPCOpen is set to true in TabChild::SendPContentPermissionRequestConstructor
> +    // and set to false in TabChild::DeallocPContentPermissionRequest
> +    NS_ASSERTION(!mIPCOpen, "Protocol must not be open when PCOMContentPermissionRequestChild is destroyed.");

Let's make this MOZ_ASSERT.
Attachment #643045 - Flags: review?(josh) → review+
Changed NS_ASSERTION to MOZ_ASSERT
Attachment #643045 - Attachment is obsolete: true
Attachment #643492 - Flags: review?(josh)
Updated patch description with r=jdm
Attachment #643492 - Attachment is obsolete: true
Attachment #643492 - Flags: review?(josh)
Attachment #643494 - Flags: review?(josh)
Attachment #643494 - Flags: review?(josh) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6528e7d3af05
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.