842451 - B2G MMS: provide nsIDOMMozMmsMessage structure

Status: RESOLVED DUPLICATE of bug 844431

(Core :: DOM: Device Interfaces, defect)

Description

[Gene Lian (I already quit Mozilla)]

+++ This bug was initially created as a clone of Bug #839436 +++

We're hoping to provide a nsIDOMMozMmsMessage structure, which is going to be exposed to the content to access the properties of an MMS message. The structure is define as below:

interface nsIDOMMozMmsMessage : nsISupports
{
  readonly attribute long      id;
  readonly attribute DOMString delivery;
  readonly attribute DOMString deliveryStatus;
  readonly attribute DOMString sender;

  [implicit_jscontext]
  readonly attribute jsval     receivers;   // DOMString[]

  [implicit_jscontext]
  readonly attribute jsval     timestamp;   // Date

  readonly attribute boolean   read;
  readonly attribute DOMString smil;

  [implicit_jscontext]
  readonly attribute jsval     attachments; // nsIDOMBlob[]
};

Comment 1

[Gene Lian (I already quit Mozilla)]

Attachment: Patch

Hi Bent,

May we invite you to take a look on the usage of |PBlob[]| in .ipdlh? We had less experience on the |PBlob| IPC and we're hoping to make sure it's correctly being used. Thanks a lot!

The purpose is to provide a |nsIDOMMozMmsMessage| for carrying the properties of an MMS message. To let the structure have chance to be passed across processes in the future, the |nsIDOMMozMmsMessage| will encapsulate its value into a |MmsMessageData| generated by .ipdlh:

  struct MmsMessageData
  {
    int32_t        id;
    DeliveryState  delivery;
    DeliveryStatus deliveryStatus;
    nsString       sender;
    nsString[]     receivers;
    uint64_t       timestamp;
    bool           read;
    nsString       smil;
    PBlob[]        attachments;
  };

Could you please return us some feedback for the following two functions to see if it's a correct way to interact with the |PBlob[]|?

1. MmsMessage::Create():

  nsCOMPtr<nsIDOMBlob> blob;
  ...
  if (isChild) {
    BlobChild* bc = ContentChild::GetSingleton()->GetOrCreateActorForBlob(blob);
    data.attachmentsChild().AppendElement(static_cast<PBlobChild*>(bc));
  } else {
    BlobParent* bp = ContentParent::GetNewOrUsed()->GetOrCreateActorForBlob(blob);
    data.attachmentsParent().AppendElement(static_cast<PBlobParent*>(bp));
  }

2. MmsMessage::GetAttachments()

  nsCOMPtr<nsIDOMBlob> blob;
  ...
  if (isChild) {
    BlobChild* bc = static_cast<BlobChild*>(mData.attachmentsChild()[i]);
    blob = bc->GetBlob();
  } else {
    BlobParent* bp = static_cast<BlobParent*>(mData.attachmentsParent()[i]);
    blob = bp->GetBlob();
  }

My understanding is when we're in the child process, we need to interact with the |.attachmentsChild()|; otherwise, with the |.attachmentsParent()|. When the |PBlob[]| crosses processes, the blob array will only be accessed by either |.attachmentsParent()| or |.attachmentsChild()|. Is that correct?

Any feedback is highly appreciated! Thank you so much! :)

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 715342 [details] [diff] [review]
Patch

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

::: dom/mms/src/MmsMessage.cpp
@@ +199,5 @@
> +        return NS_ERROR_FAILURE;
> +      }
> +      data.attachmentsChild().AppendElement(static_cast<PBlobChild*>(bc));
> +    } else {
> +      BlobParent* bp = ContentParent::GetNewOrUsed()->GetOrCreateActorForBlob(blob);

This is not right... PBlob actors are specific to a single process and cannot be shared. GetNewOrUsed() will find a random ContentParent (really, it uses rand()!) and create blob actors that are only suitable for that process. It's easier sending from the child to the parent because there is always only a single ContentChild per child process. However, when sending from the parent to a child you will need to know which ContentParent you're sending the blobs to before you can create actors for the blobs.

Comment 3

[Gene Lian (I already quit Mozilla)]

Attachment: Patch, V2

Comment 4

[Gene Lian (I already quit Mozilla)]

After discussing with Vicamo, we eventually thought it's too early to take this into consideration for now. Instead, we hope to construct the MMS IPC architecture first by following the current SMS IPC architecture.

Let's finish bug 844429 and bug 844431 first and come back to add this one when we really need it.

Comment 5

[Joe Cheng [:jcheng] (please needinfo)]

leo+, this bug is needed to fulfill MMS user stories for v1.1

Comment 6

[Gene Lian (I already quit Mozilla)]

We've already covered this structure at bug 844431 (part-2 patch). Let's dupe this and remove leo+.