1172794 - [B2G][SMS][MMS] Make MobileMessage related objects structure cloneable.

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: RIL, defect)

Description

[Bevis Tseng[:bevis][:btseng](Exited)]

[Tracking Requested - why for this release]:

In bug 1171865, Gaia would like to pass these objects between service worker and main thread.

Hence, we have to figure out a way to support the structured clone algorithm for these objects [1].

The short-term solution is to have a pure JS object in Gaia as a wrapper of the properties that we are interested in and these properties are expected to be supported in [1] already.

[1] https://developer.mozilla.org/en-US/docs/Web/Guide/API/DOM/The_structured_clone_algorithm

Comment 1

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

I've done similar things for NFC, see Bug 1138886.

Comment 2

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Yoshi Huang[:allstars.chh] from comment #1)
> I've done similar things for NFC, see Bug 1138886.

Cool! Thanks for sharing this information.

Comment 3

[Julien Wajsberg [:julienw]]

If you can make them "jsonable" in the same time, it would be a huge win for debugging :)

Comment 4

[Bevis Tseng[:bevis][:btseng](Exited)]

After further study of the StructuredClone in current implementation, it seems that there is no way to prevent the change in nsJSEnvironment.cpp if we would like to add support of structured-clone to a specific DOM object. (e.g. nsIDOMMozMobileMessageThread, nsIDOMMozSmsMessage, nsIDOMMozMmsMessage for MobileMessageManager Web API.)

In the design of the New Gaia Architecture Approach [1], more and more DOM objects has to be cloneable between ServiceWorker and main thread because all the web APIs are used behind a ServiceWorker.

Instead of adding implementation one by one into JSEnvironment, we need a better design. For example, 
1. Define a cloneable interface that allows JSEnvrionment to identified if a JSObject is strcutured-cloneable.
2. Each DOM object has to implement this new interface to support structured-clone during message-passing among different threads.

In addition, there shall be some flexibility in the cloneable interface to allow a complicated DOM object to reuse the cloneable interface of other DOM objects to support structured-clone.
(For example, the attachments in the MmsMessage contains an array of Blobs. To make MmsMessage cloneable, we have to ensure that MmsMessage has the capability to serialize the Blobs.)

This requires more input from platform experts.

Hi Olli,

Are your the right person to answer this question?

[1] https://wiki.mozilla.org/Gaia/Architecture_Proposal

Comment 5

[Julien Wajsberg [:julienw]]

Is it more complicated to migrate to WebIDL instead ?

Comment 6

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Julien Wajsberg [:julienw] (PTO Sept 2nd -> Sept 10th) from comment #5)
> Is it more complicated to migrate to WebIDL instead ?

m...
It's not related to the migration of the same API to WebIDL (Bug 859764). It's a mandatory task if we want these DOM objects available during message-passing no matter these DOM object is implemented with WebIDL or not.
Take patch in bug 1138886 as example (The DOM object is defined in WebIDL):
https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=1138886&attachment=8572548
We have to add WriteStructuredClone/ReadStructuredClone for MozNDEFRecord and make it recognized by JSEnvironment.
It's not that difficult to add this but it's not a good way to add the support of the DOM objects which are cloneable into JSEnvironment as mentioned in comment 4 which will slow down the process when posting message. :(

Did I misunderstand your question?

BTW, I also have some concern about the NGA proposal in terms of performance:
1. Before the Web API be able to be accessed directly, in the proposal, the DOM objects has to be cloned twice from web api(main thread) -> serviceWorker -> main thread.
2. Even the web API is able to be accessed in ServiceWorker in the future, how much improvement when the structured-clone overhead is taken into consideration especially when big size objects are cloned like MMS messages.

If there is an alternative way to simulate the serviceWorker in main thread for the APIs not supporting access from Worker, will it be more clear to compare the improvement of the NGA approach?

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

baku has been making great simplifications to our structured clone handling recently, so he is better person to answer to the questions related to it.

Per the new setup new stuff should go to StructuredCloneHelper. The methods in nsJSEnvironment will be
moved/merged there too.

Comment 8

[Andrea Marchesini [:baku]]

Recently the use of the Structured Clone algorithm is changed a lot. In order to answer your question I need to know where do you want to send these object from and to. Let me tell you more why I need this information.

Now, we have a new object called StructuredCloneHelper that takes care about all the serialization, cloning, transferring of DOM objects. When this helper is used, the code has to specify where the write/read will happen:

. SameProcessSameThread - for instance window to window postMessages.
. SameProcessDifferentThread - for instance workers, ServiceWorkers, etc.
. DifferentProcess - IPC mainly.

You don't have to deal with this. All the code is already updated to work in such way.

Not all the DOM objects can 'survive' in these 3 contexts. For instance, DifferentProcess requires that the objects can be fully serializable into a buffer OR be DOM file/blobs. This because we cannot clone/transfer DOM objects between threads and we have to recreate them completely on the other side, keeping them alive somehow.

So, if you are able to serialize your objects like we do for:
FileList, FormData, ImageData, Cryptokey, MessagePort, etc etc 
https://mxr.mozilla.org/mozilla-central/source/dom/base/StructuredCloneHelper.cpp#553

Then, it's super easy to do what you want.

Remember: Blobs are a completely different story. They can be sent via IPC. Just add them to BlobImpl() in StructuredCloneHelper class.

If you need more info, let's talk here or on IRC.

Comment 9

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Andrea Marchesini (:baku) from comment #8)

Thanks for providing more detailed info about StructuredCloneHelper.

> Recently the use of the Structured Clone algorithm is changed a lot. In
> order to answer your question I need to know where do you want to send these
> object from and to. Let me tell you more why I need this information.

There are several DOM objects defined in 
nsIDOMMozMobileMessageThread.idl, 
nsIDOMMozMmsMessage.idl, and 
nsIDOMMozSmsMessage.idl 

These objects will be returned from the APIs of |getThreads| and |getMessages| defined in MozMobileMessageManager.webidl
https://dxr.mozilla.org/mozilla-central/source/dom/webidl/MozMobileMessageManager.webidl

With NGA approach in 
https://wiki.mozilla.org/Gaia/Architecture_Proposal#Front-End_.2F_Back-End_with_main-thread-only_WebAPIs

These objects will be posted from ContainWrapper(main thread) --BroadcastChannel--> SharedWorker(worker thread) --MessagePort::onmessage--> ContainWrapper(main thread).

> 
> Now, we have a new object called StructuredCloneHelper that takes care about
> all the serialization, cloning, transferring of DOM objects. When this
> helper is used, the code has to specify where the write/read will happen:

I saw the newly-defined StructuredClonedHelper sent by you recently in dev-platform :) but wasn't aware that we should add the support of these mobile-message related DOM objects into this helper due to the reason mentioned in comment 4:
Which means that, for example, we have to add some piece of code inside StructuredCloneHelper::WriteCallback() for every object that requires to be cloned:
  // See if this is a MmsMessage object.
  {
    MmsMessage* mmsMsg = nullptr;
    if (NS_SUCCEEDED(UNWRAP_OBJECT(MmsMessage, aObj, mmsMsg))) {
      return WriteMmsMessage(aWriter, mmsMsg, this);
    }
  }
instead of implementing something called "cloneable" interface inside MmsMessage to reduce the redundant check of "if (NS_SUCCEEDED(UNWRAP_OBJECT(...))" when the number of supported DOM objects increased.

> 
> . SameProcessSameThread - for instance window to window postMessages.
> . SameProcessDifferentThread - for instance workers, ServiceWorkers, etc.
> . DifferentProcess - IPC mainly.
> 
> You don't have to deal with this. All the code is already updated to work in
> such way.
> 
> Not all the DOM objects can 'survive' in these 3 contexts. For instance,
> DifferentProcess requires that the objects can be fully serializable into a
> buffer OR be DOM file/blobs. This because we cannot clone/transfer DOM
> objects between threads and we have to recreate them completely on the other
> side, keeping them alive somehow.
> 
> So, if you are able to serialize your objects like we do for:
> FileList, FormData, ImageData, Cryptokey, MessagePort, etc etc 
> https://mxr.mozilla.org/mozilla-central/source/dom/base/
> StructuredCloneHelper.cpp#553

I can follow the same rule in current stage if this is expected in current design instead of a predefined interface on each DOM object for cloning. :)

> 
> Then, it's super easy to do what you want.
> 
> Remember: Blobs are a completely different story. They can be sent via IPC.
> Just add them to BlobImpl() in StructuredCloneHelper class.
> 
> If you need more info, let's talk here or on IRC.

One more question is about the cloneable support of the 'dictionary' objects returned by the APIs.
https://dxr.mozilla.org/mozilla-central/source/dom/webidl/MozMobileMessageManager.webidl
Take |getSegmentInfoForText| and |getSmscAddress| as examples, these 2 APIs returns |dictionary SmsSegmentInfo| and |dictionary SmscAddress| respectively. Moreover, the implementation of these 2 dictionary are auto-generated in MozMobileMessageManagerBinding.h/cpp.
Then, how can we ensure that these dictionary objects could be cloned between main thread and worker thread? Shall we replace it with a new interface in WebIDL and add structured-clone support for it or it will be directly covered in our platform implementation?

Comment 10

[Andrea Marchesini [:baku]]

> I saw the newly-defined StructuredClonedHelper sent by you recently in
> dev-platform :) but wasn't aware that we should add the support of these
> mobile-message related DOM objects into this helper due to the reason
> mentioned in comment 4:

What you are proposing in comment 4 is very interesting but it cannot work for any DOM objects.
Something nice would be to add:

[FullySerializableObject] // maybe a better name.
interface RandomDOMObject { ... }

and if that key is defined, then the object has to implement a:

bool RandomDOMObject::CloneWrite(...) and CloneRead(...).

Then all these objects can be managed by StructuredCloneHelper::WriteFullySerializableObjects().
This method is already implemented and it will land soon (this week).

That would be nice to have and I'd like to speak with bz about it.
Sounds like a reasonable follow up.

> One more question is about the cloneable support of the 'dictionary' objects
> returned by the APIs.

Any dictionary can be serialized into a JSON object and it can be read from it.

Comment 11

[Bevis Tseng[:bevis][:btseng](Exited)]

After further review, there could be some dependency to bug 859764 because, in bug 859764, the internal implementation of the data fields might be changed when migrating these DOM objects from idl to WebIDL.
The change will affect the implementation of the structured-clone support of these DOM objects.

Comment 12

[Bevis Tseng[:bevis][:btseng](Exited)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=de37671c9a0a

Comment 13

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (WIP) Part 1: Unify The Naming of Internal Objects.

Comment 14

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (WIP) Part 2: Make MobileMessage related objects structure cloneable.

Comment 15

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8694017 [details] [diff] [review]
(WIP) Part 2: Make MobileMessage related objects structure cloneable.

Hi baku,

May I have your earlier feedback of this WIP patch?

In this bug, I'd like to make MmsMessage, MobileMessageThread, SmsMessage structured-cloneable between main thread and the service worker in application layer to allow Gaia developer to handle these objects in the NGA design in [1].

There are 2 additional questions about the change in this patch:
1. A handy StructuredCloneUtils is created to read/write the internal data types like nsString, nsStringArray, Blob(Simplified for MMS), etc in dom/mobilemessage folder. Except utilities of the Mms specific ones, shall I move these utilities into dom/base instead?
2. I didn't add the support of these interfaces in ExportHelpers.cpp because I am not pretty sure if it's required to allow these objects to be cloneable in the NGA design [1]. If it's necessary, then the |BlobImpls| has to be exposed from |StructuredCloneHolderBase| instead of |StructuredCloneHolder::BlobImpls| for the structuredClone of the attachments in a MmsMessage. However, the |BlobImpls| is varied in different subclass of |StructuredCloneHolderBase|. Hence, I'd like to know if there's any better suggestion if this change is required?

Thanks!

[1] https://wiki.mozilla.org/Gaia/Architecture_Proposal#Front-End_.2F_Back-End_with_main-thread-only_WebAPIs

Comment 16

[Andrea Marchesini [:baku]]

Comment on attachment 8694017 [details] [diff] [review]
(WIP) Part 2: Make MobileMessage related objects structure cloneable.

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

looks good. Better if we can use this StructuredCloneUtils also in StructuredCloneHolder.

::: dom/mobilemessage/StructuredCloneUtils.cpp
@@ +26,5 @@
> +{
> +  MOZ_ASSERT(aBlobImpl);
> +
> +  if (!aManager) {
> +    aManager = BackgroundChild::GetForCurrentThread();

1. This code is changed recently. Update your patch with the latest EnsureBlobForBackgroundManager.
2. This is not recursive: what about if BlobImpl is a MultiPartBlobImpl ?

::: dom/mobilemessage/StructuredCloneUtils.h
@@ +68,5 @@
> +
> +  static bool
> +  ReadStringArray(JSStructuredCloneReader* aReader,
> +                  nsTArray<nsString>& aData);
> +

Write a comment here saying the following methods are for MMS API.

Comment 17

[Bevis Tseng[:bevis][:btseng](Exited)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dead915e7dbc

Comment 18

[Bevis Tseng[:bevis][:btseng](Exited)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=501100ddf357

Comment 19

[Bevis Tseng[:bevis][:btseng](Exited)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4055066ccfec

Comment 20

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v1) Part 1: Unify The Naming of Internal Objects.

Hi Edgar,

May I have your review for this renaming?

Thanks!

Comment 21

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v1) Part 2: Create StructuredCloneUtils for Common Data Types.

Hi Baku,

I'd like to move the Structured-Clone related process of Blob/File/String from StructuredCloneHolder to StructuredCloneUtils for further use by other modules.

The MMS-specific properties will be done inside MmsMessage instead in patch part 3.

May I have your review for this change?

Thanks!

Comment 22

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v1) Part 3: Make MobileMessage Related Objects Structure Cloneable.

Add support of StructuredClone for MobileMessage/SmsMessage/MmsMessage.

Comment 23

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v1) Part 4: Add Test Coverage.

Comment 24

[Edgar Chen [:edgar]]

Comment on attachment 8701940 [details] [diff] [review]
(v1) Part 1: Unify The Naming of Internal Objects.

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

Looks good to me.

Comment 25

[Andrea Marchesini [:baku]]

Comment on attachment 8701943 [details] [diff] [review]
(v1) Part 2: Create StructuredCloneUtils for Common Data Types.

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

I'm so sorry for the delay for this review.
Is there any reason why you want to return DOM objects instead JSObject*  in all the Read methods?
And do you not write the 'SCTAG' in the Write methods?

::: dom/base/StructuredCloneHolder.cpp
@@ +555,5 @@
>    // while destructors are running.
>    JS::Rooted<JS::Value> val(aCx);
>    {
> +    RefPtr<Blob> blob;
> +    if (!StructuredCloneUtils::ReadBlob(aReader, aHolder, getter_AddRefs(blob)) ||

If StructuredCloneUtils::ReadBlob() returns a JSObject* you can get rid of this method.

@@ +570,5 @@
>            Blob* aBlob,
>            StructuredCloneHolder* aHolder)
>  {
> +  return JS_WriteUint32Pair(aWriter, SCTAG_DOM_BLOB, 0) &&
> +    StructuredCloneUtils::WriteBlob(aWriter, aHolder, aBlob);

Move JS_WriteUint32Pair() in StructuredCloneUtils::WriteBlob() and remove this method.

@@ +583,4 @@
>    JS::Rooted<JS::Value> val(aCx);
>    {
> +    RefPtr<FileList> fileList;
> +    if (!StructuredCloneUtils::ReadFileList(aReader, aHolder,

What about if StructuredCloneUtils::ReadFileList() returns a JSObject*?
In this way you can get rid of this method.

@@ +601,5 @@
>    MOZ_ASSERT(aWriter);
>    MOZ_ASSERT(aFileList);
>    MOZ_ASSERT(aHolder);
>  
> +  return JS_WriteUint32Pair(aWriter, SCTAG_DOM_FILELIST, 0) &&

If you move the JS_WriteUint32Pair in StructuredCloneUtils::WriteFileList() you can get rid of this method completely.

@@ +718,5 @@
>          return false;
>        }
>  
>        if (aValue.IsFile()) {
> +        return JS_WriteUint32Pair(closure->mWriter, SCTAG_DOM_BLOB, 0) &&

What about if you move this JS_WriteUint32Pair in StructuredCloneUtils::WriteFile? In such way you can keep the current serialization: (SCTAG_DOM_BLOB, index)

Comment 26

[Andrea Marchesini [:baku]]

Comment on attachment 8701944 [details] [diff] [review]
(v1) Part 3: Make MobileMessage Related Objects Structure Cloneable.

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

::: dom/base/StructuredCloneHolder.cpp
@@ +839,5 @@
> +  {
> +    MmsMessage* mmsMsg = nullptr;
> +    if (NS_SUCCEEDED(UNWRAP_OBJECT(MmsMessage, aObj, mmsMsg))) {
> +      return JS_WriteUint32Pair(aWriter, SCTAG_DOM_MMS_MSG, 0) &&
> +        MmsMessage::WriteStructuredClone(aCx, aWriter, this, mmsMsg);

same patter of the previous patch.

@@ +848,5 @@
> +  {
> +    MobileMessageThread* msgThread = nullptr;
> +    if (NS_SUCCEEDED(UNWRAP_OBJECT(MobileMessageThread, aObj, msgThread))) {
> +      return JS_WriteUint32Pair(aWriter, SCTAG_DOM_MSG_THREAD, 0) &&
> +        MobileMessageThread::WriteStructuredClone(aCx, aWriter, msgThread);

ditto.

@@ +857,5 @@
> +  {
> +    SmsMessage* smsMsg = nullptr;
> +    if (NS_SUCCEEDED(UNWRAP_OBJECT(SmsMessage, aObj, smsMsg))) {
> +      return JS_WriteUint32Pair(aWriter, SCTAG_DOM_SMS_MSG, 0) &&
> +        SmsMessage::WriteStructuredClone(aCx, aWriter, smsMsg);

ditto.

::: dom/mobilemessage/MmsMessage.cpp
@@ +336,5 @@
> +      StructuredCloneUtils::ReadString(aReader, smil) &&
> +      ReadAttachmentArray(aReader, aHolder, attachments) &&
> +      StructuredCloneUtils::ReadUint64(aReader, expiryDate) &&
> +      StructuredCloneUtils::ReadBoolean(aReader, readReportRequested)) {
> +    nsIGlobalObject *global = xpc::NativeGlobal(JS::CurrentGlobalOrNull(aCx));

What about if one of this Read() fails?

::: dom/mobilemessage/MobileMessageThread.cpp
@@ +130,5 @@
> +      StructuredCloneUtils::ReadUint64(aReader, timestamp) &&
> +      StructuredCloneUtils::ReadString(aReader, lastMessageSubject) &&
> +      StructuredCloneUtils::ReadString(aReader, body) &&
> +      StructuredCloneUtils::ReadUint64(aReader, unreadCount) &&
> +      StructuredCloneUtils::ReadInt32(aReader, lastMessageType)) {

ditto.

::: dom/mobilemessage/SmsMessage.cpp
@@ +194,5 @@
> +      StructuredCloneUtils::ReadInt32(aReader, messageClass) &&
> +      StructuredCloneUtils::ReadUint64(aReader, timestamp) &&
> +      StructuredCloneUtils::ReadUint64(aReader, sentTimestamp) &&
> +      StructuredCloneUtils::ReadUint64(aReader, deliveryTimestamp) &&
> +      StructuredCloneUtils::ReadBoolean(aReader, read)) {

ditto.

Comment 27

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Andrea Marchesini (:baku) from comment #25)
> Comment on attachment 8701943 [details] [diff] [review]
> (v1) Part 2: Create StructuredCloneUtils for Common Data Types.
> 
> Review of attachment 8701943 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm so sorry for the delay for this review.
Never mind. It's expected to get the feedback after this big holiday is ended. :)
> Is there any reason why you want to return DOM objects instead JSObject*  in
> all the Read methods?
I'd like to make these Blob/File like object to be reusable inside other structured-cloneable objects.
> And do you not write the 'SCTAG' in the Write methods?
> 
My understanding is that the SCTAG is only meaningful for the outer class to be structured-cloned and to be identified in StructuredCloneHolder::CustomReadHandler.
So for all these Blob/File like objects, which could be part of another object to be structured-cloned, are moved to StructuredCloneUtils.h and are defined in the same style as other Read/Write methods of Boolean/String/Int32/Uint64.
This also provides coding convention of other new structured-cloneable classes as you can see for all these mobile message related objects.

Did I clarify your concern?

Comment 28

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8701944 [details] [diff] [review]
(v1) Part 3: Make MobileMessage Related Objects Structure Cloneable.

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

::: dom/mobilemessage/MmsMessage.cpp
@@ +336,5 @@
> +      StructuredCloneUtils::ReadString(aReader, smil) &&
> +      ReadAttachmentArray(aReader, aHolder, attachments) &&
> +      StructuredCloneUtils::ReadUint64(aReader, expiryDate) &&
> +      StructuredCloneUtils::ReadBoolean(aReader, readReportRequested)) {
> +    nsIGlobalObject *global = xpc::NativeGlobal(JS::CurrentGlobalOrNull(aCx));

*nullptr* will be returned at the end of this function.

Comment 29

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #27)
> (In reply to Andrea Marchesini (:baku) from comment #25)
> > Is there any reason why you want to return DOM objects instead JSObject*  in
> > all the Read methods?
> I'd like to make these Blob/File like object to be reusable inside other
> structured-cloneable objects.
The example in this bug is the Blob in MMS attachments.

Comment 30

[Bevis Tseng[:bevis][:btseng](Exited)]

I'll add my answer in detail for your concern in the r? request instead to resume the review again.

Comment 31

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8701943 [details] [diff] [review]
(v1) Part 2: Create StructuredCloneUtils for Common Data Types.

Hi Baku,

I'd like to resume this review again with the following answer to your concern:

> (In reply to Andrea Marchesini (:baku) from comment #25)
> Is there any reason why you want to return DOM objects instead JSObject*  in
> all the Read methods?
As you suggested in comment 16 to have this StructCloneUtils.h be adopted in StructuredCloneHolder.
I'd like to make these Blob/File-like object in StructuredCloneUtils.h to be reusable as Boolean/String/Int32/Uint64 by other DOM Object implementation like MmsMessage. In this case, it's not necessary to be a JSObject* which is only meaningful inside StructuredCloneHolder::CustomReadHandler().
The 2nd advantage of this change is to have |EnsureBlobForBackgroundManager| that will be used by Blob/File/FileList in only one place.
> And do you not write the 'SCTAG' in the Write methods?
> 
By my understanding, the object to be structured-cloned will be serialized as:
- SCTAG_Of_ThatObject
- Serialization of Property#1
- Serialization of Property#2
- Serialization of Property#3
- ...

To make this WriteXXX in StructuredCloneUtils as utilities of other DOM objects to be structured-cloneable, it's not necessary to have SCTAG_Of_Xxx for the properties of these objects.
That's the reason why that |bool| is enough for the caller of the utilities to determinate if each property of the object is serialized properly.

In advance, the advantage of keeping SCTAG_DOM_XXX inside StructuredCloneHolder is that once our code-gen supports to identify the structured-clone support of the interface from webidl.
It's a clear-cut to have this tags and bindings defined in the XXXBinding automatically, then the owner of the DOM object can just focus on the implementation of XXX::WriteStructuredClone and XXX::ReadStructuredClone by utilizing the StructuredCloneUtils.h.

Comment 32

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8701944 [details] [diff] [review]
(v1) Part 3: Make MobileMessage Related Objects Structure Cloneable.

resume the review per comment 31.

Comment 33

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8701943 [details] [diff] [review]
(v1) Part 2: Create StructuredCloneUtils for Common Data Types.

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

We'll revisit this when needed. Cancel the r? flag.

Comment 34

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8701944 [details] [diff] [review]
(v1) Part 3: Make MobileMessage Related Objects Structure Cloneable.

We'll revisit this when needed. Cancel the r? flag.

Comment 35

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8701945 [details] [diff] [review]
(v1) Part 4: Add Test Coverage.

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

We'll revisit this when needed. Cancel the r? flag.

Comment 36

[Bevis Tseng[:bevis][:btseng](Exited)]

no longer following up this bug.

Comment 37

[BMO Automation]

Firefox OS is not being worked on