773683 - Improve PCOMContentPermissionRequest usage safety

Status: RESOLVED FIXED

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

Description

[Doug Turner (:dougt)]

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.

Comment 1

[Josh Matthews [:jdm]]

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.

Comment 2

[William Chen [:wchen]]

Attachment: Added runtime assertion to ensure PCOMContentPermissionRequestChild is live while protocol is live.

Comment 3

[Josh Matthews [:jdm]]

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.

Comment 4

[William Chen [:wchen]]

Attachment: Added runtime assertion to ensure PCOMContentPermissionRequestChild is live while protocol is live.

Changed NS_ASSERTION to MOZ_ASSERT

Comment 5

[William Chen [:wchen]]

Attachment: Added runtime assertion to ensure PCOMContentPermissionRequestChild is live while protocol is live.

Updated patch description with r=jdm

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6528e7d3af05

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6528e7d3af05