782766 - [WebActivities] support blobs

Status: RESOLVED FIXED

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

Description

[David Flanagan [:djf]]

The B2G Gallery app shares images with other apps via "Pick" and "Share" activities. The right way to do this would be to pass a blob to the other app in response to a Pick activity or when initiating a Share activity.  But that does not currently work.

I could use URL.createObjectURL() and pass the resulting string, but that seems like it would cause problems knowing when to revoke the URL (and those blob urls might not even be valid cross domain.)

So currently the Gallery app just shares filenames. So any apps that interact with it have to use the DeviceStorage API to get the actual image, which doesn't seem ideal.  I'll probably rename my "share" activity to "share-filename" or something.

Note that blobs do not work either OOP or non-OOP.

I don't think this is a basecamp blocker, but it is an API stablity issue. Each activity is its own mini-protocol. If 3rd party devs start using Web Activities now, they'll end up with hacks like 'share-filename' that will need to be redone when the implementation is complete.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Why was this never nominated as a blocker. Or an owner hunted down? I thought we fixed this ages ago. We certainly did a bunch of backend work specifically to support this.

At this point we are past feature freeze so we definitely can't block on this. But if it's trivial to implement (which I think it might be) it might be worth optimistically taking.

If we were before feature freeze I definitely would have considered this a blocker.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

The functionality is actually basically there. It happened when we made the messagemanager automatically support structured clones. The theory is that the only reason that this is failing is that we fail to wrap the Blob properly when exposing it to content.

Please, in the future, speak up about bugs like this! WebActivities without Blob support is *really* crappy.

Comment 3

[Andrea Marchesini [:baku]]

Unfortunately it's not so easy. In the code I read:

  // Not clonable, try JSON
  //XXX This is ugly but currently structured cloning doesn't handle
  //    properly cases when interface is implemented in JS and used
  //    as a dictionary.
  [...]
  NS_ENSURE_TRUE(JS_Stringify(aCx, &v, nullptr, JSVAL_NULL, JSONCreator, &json), false);
  NS_ENSURE_TRUE(JS_ParseJSON(aCx, static_cast<const jschar*>(PromiseFlatString(json).get()),
                              json.Length(), &val), false);

  return WriteStructuredClone(aCx, val, aBuffer, aClosure);

This means that we cannot add the 'blob' functionality as workers do.

I still don't get why the structured clone algorithm is not enough for sharing data between processes. Do you?

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

That is just fallback code. We try to do a structured clone before that which should allow this to work. 

I actually think that the blob data gets over to the receiving process just fine. The message manager definitely supports passing blobs since we're using that in other places (at least the contacts API uses it).

The problem is likely that when we are trying to hand the data from chrome code to content code and fail to migrate it properly.

Comment 5

[[:fabrice] Fabrice Desré]

(In reply to Jonas Sicking (:sicking) from comment #4)

> The problem is likely that when we are trying to hand the data from chrome
> code to content code and fail to migrate it properly.

The chrome -> content wrapper in dom/base/ObjectWrapper.jsm doesn't wrap blobs properly. This would be the first think to try fixing.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

To fix that code, we need to detect that an object is a blob and then create a new blob in the scope of the page.

I don't know if we have a good way to check if something is a blob from chrome code.

Does object instanceof Ci.nsIDOMBlob work?

Blake: How would one go about creating a blob object in the scope of a particular webpage from chrome JS? Would |new contentwindow.Blob| work?

Comment 7

[[:fabrice] Fabrice Desré]

Investigating a bit more closely with a simple app written by djf that sends a simple blob in an activity I found out that:

- In the content process, at https://mxr.mozilla.org/mozilla-central/source/dom/activities/src/ActivityProxy.js#40 the object is a [object Blob @ 0x7f4d9778c200 (native @ 0x7f4d975aba80)]

- In the parent, when we start the activity at https://mxr.mozilla.org/mozilla-central/source/dom/activities/src/ActivitiesService.jsm#193 the blob is not a blob anymore. It's an [object Object] with no enumerable properties. I verified that Andrea is right in comment #3 in this case.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

The problem is likely that we are sending the options object. If we instead do

cpmm.sendAsyncMessage("Activity:Start", { id: this.id,
                                          options: { name: aOptions.name,
                                                     data: aOptions.data },
                                          manifestURL: manifestURL,
                                          pageURL: aWindow.document.location.href });

it will likely work.

We still have to fix the wrapping though.

Comment 9

[[:fabrice] Fabrice Desré]

Jonas's trick with |options| gets us a blob in the parent, and the activity service is then transmitting this to the system message implementation (also in the parent) which is responsible for the actual delivery to the web page.

In the system message manager we get messages when GetPendingMessages from the child side. At https://mxr.mozilla.org/mozilla-central/source/dom/messages/SystemMessageInternal.js#218 we have a blob that we send back, but this a synchronous call (https://mxr.mozilla.org/mozilla-central/source/dom/messages/SystemMessageManager.js#134) and unfortunately we only return JSON data for sendSyncMessage(). See nsFrameMessageManager::SendSyncMessage() calling DoSendSyncMessage() that returns |InfallibleTArray<nsString> retval|

So it looks like we have either to:
- fix the FrameMessageManager to send back structured clones.
- find a workaround in the message manager.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I assume we use a synchronous message in order to implement the synchronous mozHasPendingMessage function? Couldn't we simply use a synchronous message to detect what type of pending messages we have, and use an asynchronous message to deliver those messages?

Comment 11

[[:fabrice] Fabrice Desré]

Yes, I started to refactor it that way. I'm not very happy to make this kind of changes at this point tough.

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

If it isn't safe, or if it's too much work, we simply should not do it.

Comment 13

[[:fabrice] Fabrice Desré]

Attachment: wip

This patch refactors the system message implementation to only use a sync message for mozHasPendingMessages, and returns all data from the parent to the child using asynchronous messages. It looks ok for non-blobs activities, but we're crashing when sending a blob parent -> child ("error deserializing (better message TODO)"):

#0  0x00007fb4e66a783d in nanosleep () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x00007fb4e66a76dc in sleep () from /lib/x86_64-linux-gnu/libc.so.6
#2  0x00007fb4e78150da in ah_crap_handler (signum=11) at /home/fabrice/dev/inbound/toolkit/xre/nsSigHandlers.cpp:87
#3  0x00007fb4e7815125 in child_ah_crap_handler (signum=11) at /home/fabrice/dev/inbound/toolkit/xre/nsSigHandlers.cpp:99
#4  <signal handler called>
#5  0x00007fb4ebbcb5e1 in mozalloc_abort (
    msg=0x7ffff3193c70 "[Child 22694] ###!!! ABORT: [PContentChild] abort()ing as a result: file /home/fabrice/dev/builds/obj-b2g-desktop-inbound/ipc/ipdl/PContentChild.cpp, line 3012") at /home/fabrice/dev/inbound/memory/mozalloc/mozalloc_abort.cpp:23
#6  0x00007fb4e92f9e9e in Abort (
    aMsg=0x7ffff3193c70 "[Child 22694] ###!!! ABORT: [PContentChild] abort()ing as a result: file /home/fabrice/dev/builds/obj-b2g-desktop-inbound/ipc/ipdl/PContentChild.cpp, line 3012") at /home/fabrice/dev/inbound/xpcom/base/nsDebugImpl.cpp:423
#7  0x00007fb4e92f9da9 in NS_DebugBreak_P (aSeverity=3, aStr=0x7fb4ea7f8448 "[PContentChild] abort()ing as a result", aExpr=0x0, 
    aFile=0x7fb4ea7f7700 "/home/fabrice/dev/builds/obj-b2g-desktop-inbound/ipc/ipdl/PContentChild.cpp", aLine=3012)
    at /home/fabrice/dev/inbound/xpcom/base/nsDebugImpl.cpp:380
#8  0x00007fb4e9150535 in mozilla::dom::PContentChild::FatalError (this=0x7fb4de009c30, 
    msg=0x7fb4ea7f7788 "error deserializing (better message TODO)")
    at /home/fabrice/dev/builds/obj-b2g-desktop-inbound/ipc/ipdl/PContentChild.cpp:3012
#9  0x00007fb4e914fdf8 in mozilla::dom::PContentChild::OnMessageReceived (this=0x7fb4de009c30, __msg=...)
    at /home/fabrice/dev/builds/obj-b2g-desktop-inbound/ipc/ipdl/PContentChild.cpp:2736
#10 0x00007fb4e8ffea91 in mozilla::ipc::AsyncChannel::OnDispatchMessage (this=0x7fb4de009c40, msg=...)
    at /home/fabrice/dev/inbound/ipc/glue/AsyncChannel.cpp:473
#11 0x00007fb4e900b428 in mozilla::ipc::RPCChannel::OnMaybeDequeueOne (this=0x7fb4de009c40) at /home/fabrice/dev/inbound/ipc/glue/RPCChannel.cpp:402
#12 0x00007fb4e900f65a in DispatchToMethod<mozilla::ipc::RPCChannel, bool (mozilla::ipc::RPCChannel::*)()> (obj=0x7fb4de009c40, method=
    (bool (mozilla::ipc::RPCChannel::*)(mozilla::ipc::RPCChannel * const)) 0x7fb4e900b1f4 <mozilla::ipc::RPCChannel::OnMaybeDequeueOne()>, arg=...)
    at /home/fabrice/dev/inbound/ipc/chromium/src/base/tuple.h:383
#13 0x00007fb4e900f5a8 in RunnableMethod<mozilla::ipc::RPCChannel, bool (mozilla::ipc::RPCChannel::*)(), Tuple0>::Run (this=0x7fb4de013740)
    at /home/fabrice/dev/inbound/ipc/chromium/src/base/task.h:307
#14 0x00007fb4e9009c25 in mozilla::ipc::RPCChannel::RefCountedTask::Run (this=0x7fb4de01d220) at ../../dist/include/mozilla/ipc/RPCChannel.h:425
#15 0x00007fb4e9009d28 in mozilla::ipc::RPCChannel::DequeueTask::Run (this=0x7fb4d28e7580) at ../../dist/include/mozilla/ipc/RPCChannel.h:448
#16 0x00007fb4e933f6af in MessageLoop::RunTask (this=0x7ffff3195a60, task=0x7fb4d28e7580)
    at /home/fabrice/dev/inbound/ipc/chromium/src/base/message_loop.cc:333
#17 0x00007fb4e933f727 in MessageLoop::DeferOrRunPendingTask (this=0x7ffff3195a60, pending_task=...)
    at /home/fabrice/dev/inbound/ipc/chromium/src/base/message_loop.cc:341
#18 0x00007fb4e933fb0b in MessageLoop::DoWork (this=0x7ffff3195a60) at /home/fabrice/dev/inbound/ipc/chromium/src/base/message_loop.cc:441
#19 0x00007fb4e9008125 in mozilla::ipc::DoWorkRunnable::Run (this=0x7fb4de01c220) at /home/fabrice/dev/inbound/ipc/glue/MessagePump.cpp:42
#20 0x00007fb4e92ea8b7 in nsThread::ProcessNextEvent (this=0x7fb4de01abb0, mayWait=false, result=0x7ffff319452f)
    at /home/fabrice/dev/inbound/xpcom/threads/nsThread.cpp:612

Comment 14

[[:fabrice] Fabrice Desré]

Additional backtrace, breaking in ProtocolUtils.h :

#5  0x00007f1b08e6e74f in mozilla::ipc::ProtocolErrorBreakpoint (aMsg=0x7f1b0a6525fb "could not look up PBlob")
    at ../../dist/include/mozilla/ipc/ProtocolUtils.h:119
#6  0x00007f1b08facf74 in mozilla::dom::PContentChild::Read (this=0x7f1afde09c30, __v=0x7f1af30ff448, __msg=0x7fffc5caf530, __iter=0x7fffc5caf388, 
    __nullable=false) at /home/fabrice/dev/builds/obj-b2g-desktop-inbound/ipc/ipdl/PContentChild.cpp:3755
#7  0x00007f1b08fadde6 in mozilla::dom::PContentChild::Read (this=0x7f1afde09c30, __v=0x7fffc5caf3e8, __msg=0x7fffc5caf530, __iter=0x7fffc5caf388)
    at /home/fabrice/dev/builds/obj-b2g-desktop-inbound/ipc/ipdl/PContentChild.cpp:4674
#8  0x00007f1b08fadc0a in mozilla::dom::PContentChild::Read (this=0x7f1afde09c30, __v=0x7fffc5caf3d0, __msg=0x7fffc5caf530, __iter=0x7fffc5caf388)
    at /home/fabrice/dev/builds/obj-b2g-desktop-inbound/ipc/ipdl/PContentChild.cpp:4601
#9  0x00007f1b08faa415 in mozilla::dom::PContentChild::OnMessageReceived (this=0x7f1afde09c30, __msg=...)
    at /home/fabrice/dev/builds/obj-b2g-desktop-inbound/ipc/ipdl/PContentChild.cpp:2735
#10 0x00007f1b08e590cb in mozilla::ipc::AsyncChannel::OnDispatchMessage (this=0x7f1afde09c40, msg=...)
    at /home/fabrice/dev/inbound/ipc/glue/AsyncChannel.cpp:473

We try to Read:
ClonedMessageData -> InfallibleTArray<PBlobChild*> -> PBlobChild*

Comment 15

[[:fabrice] Fabrice Desré]

Attachment: wip v2

Updated wip that gets everything working up to wrapping the blob we send to content. We crash afterward when touching it (http://pastebin.mozilla.org/1875093)

Comment 16

[[:fabrice] Fabrice Desré]

Attachment: test apps

This archive contains two test applictions: a "blob sender" and a "blob receiver" one. Just move them in your gaia/test_apps folder, and |make profile| to include them in your apps.

Then launch the 'Blob Sender', and choose 'Share Blob'.

Comment 17

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

Attachment: Blob changes, v1

This applies on top of your other changes. See if this works?

Comment 18

[[:fabrice] Fabrice Desré]

Things look fine with Ben's patch!

Comment 19

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

Comment on attachment 674450 [details] [diff] [review]
Blob changes, v1

khuey can review this.

Comment 20

[Gene Lian (I already quit Mozilla)]

Comment on attachment 674066 [details] [diff] [review]
wip v2

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

Hi Fabrice,

Thanks for trusting me to review your codes (I'm still a newbie, though). Overall, this change is good and I love the part of handling the pending messages in the parent only. However, there is one critical issue that would fail the original logic. Please see my comments as below.

::: dom/base/ObjectWrapper.jsm
@@ +12,5 @@
>  
>  // Makes sure that we expose correctly chrome JS objects to content.
>  
>  let ObjectWrapper = {
> +  objectKind: function objWrapper_kind(aObject) {

How about s/objectKind/getObjectKind?

@@ +30,5 @@
> +    let kind = this.objectKind(aObject);
> +    if (kind == "array") {
> +      let res = Cu.createArrayIn(aCtxt);
> +      aObject.forEach(function(aObj) {
> +        res.push(objWrapper_wrap(aObj, aCtxt));

Within this function, sometimes you use |objWrapper_wrap()|, sometimes |ObjectWrapper.wrap()|. Can we just choose one if possible? Btw, can we just use |this.wrap()|?

@@ +44,4 @@
>      let res = Cu.createObjectIn(aCtxt);
>      let propList = { };
>      for (let prop in aObject) {
>        let value;

let propObj = aObject[prop];

Use |propObj| or other similar in the following codes instead?

@@ +44,5 @@
>      let res = Cu.createObjectIn(aCtxt);
>      let propList = { };
>      for (let prop in aObject) {
>        let value;
> +      let propKind = this.objectKind(aObject[prop]);

How about s/propKind/propObjKind?

::: dom/messages/SystemMessageInternal.js
@@ +227,5 @@
> +        aMessage.target.sendAsyncMessage("SystemMessageManager:GetPendingMessages:Return",
> +                                         { type: msg.type,
> +                                           manifest: msg.manifest,
> +                                           uri: msg.uri,
> +                                           msgQueue: pendingMessages });

I think we don't really need to send everything back except for the |msgQueue|. I know you might want to reuse most of the codes in SystemMessageManager when receiving it, but some codes can be cleaned up further. Well, it's just my picky thought, though. Please feel free to keep it if you like. ;)

::: dom/messages/SystemMessageManager.js
@@ -103,5 @@
>      handlers[aType] = aHandler;
>  
> -    // If we have pending messages, send them asynchronously.
> -    if (this._getPendingMessages(aType, true)) {
> -      let thread = Services.tm.mainThread;

I don't quite understand the original purpose of main thread thing here, but it seems you intentionally remove it. If you're OK, then I'm OK. Just confirm a bit. :)

@@ +108,5 @@
>  
> +    return pmm.sendSyncMessage("SystemMessageManager:HasPendingMessages",
> +                               { type: aType,
> +                                 uri: this._uri,
> +                                 manifest: this._manifest })[0];

s/pmm/cpmm?

@@ +130,3 @@
>      if (msg.manifest != this._manifest || msg.uri != this._uri) {
>        return;
>      }

As mentioned above, this condition is originally used for |SystemMessageManager:Message| type. We don't need this for |SystemMessageManager:GetPendingMessages:Return| type, though.

@@ +133,5 @@
>  
>      // Send an acknowledgement to parent to clean up the pending message,
>      // so a re-launched app won't handle it again, which is redundant.
>      cpmm.sendAsyncMessage(
>        "SystemMessageManager:Message:Return:OK",

I'm afraid this doesn't work here, because for the |SystemMessageManager:Message| type, we need to send an acknowledgement |SystemMessageManager:Message:Return:OK| back to clear the single message in parent. I think this logic cannot properly deal with the |SystemMessageManager:GetPendingMessages:Return| type, which receives an array of messages actually.

All in all, need to add a condition to do this part for |SystemMessageManager:Message| type only.

@@ +143,5 @@
>      // Bail out if we have no handlers registered for this type.
>      if (!(msg.type in this._handlers)) {
>        debug("No handler for this type");
>        return;
>      }

Similarly, we don't need this check for |SystemMessageManager:GetPendingMessages:Return| type, because when we fire |SystemMessageManager:GetPendingMessages| we're sure we must have a handler for this type already.

Comment 21

[Gene Lian (I already quit Mozilla)]

Btw, I'm glad to support the System Message part if you don't have enough bandwidth to redesign and test everything (but I'm not really familiar with the wrapper codes). Please feel free to let me know if I can help. :)

Comment 22

[Gene Lian (I already quit Mozilla)]

Comment on attachment 674066 [details] [diff] [review]
wip v2

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

::: dom/messages/SystemMessageManager.js
@@ +96,5 @@
> +    // Ask for the list of currently pending messages.
> +    cpmm.sendAsyncMessage("SystemMessageManager:GetPendingMessages",
> +                          { type: aType,
> +                            uri: this._uri,
> +                            manifest: this._manifest });

Hi Fabrice,

Also, according to Jonas' saying at [1] as described as below. Does the word "quickly" here means we have to make setMessageHandler() synchronous to handle the messages after calling hasPendingMessage()? Or it doesn't matter?

"However if you need to synchronously know if the application was 
started in response to a message, for example in order to decide what 
UI to show, we should also have a function like 
navigator.hasPendingMessage(type) 
which returns true if there's a pending message that will be fired. If 
this function returns true, you can be confident in that returning to 
the event loop will cause the message handler be called very quickly."

"Instead we just added the synchronous hasPendingMessages function. 
That way the page can check if there are incoming messages and wait 
with making any UI decisions until the message is delivered, which 
always happens quickly after setMessageHandler is called."

[1] https://groups.google.com/d/msg/mozilla.dev.webapi/o8bkwx0EtmM/6bmkp3vFGHcJ

Comment 23

[[:fabrice] Fabrice Desré]

Attachment: patch v2

Gene,

I addressed you comments in this patch except this one:

> @@ +130,3 @@
> >      if (msg.manifest != this._manifest || msg.uri != this._uri) {
> >        return;
> >      }
> 
> As mentioned above, this condition is originally used for
> |SystemMessageManager:Message| type. We don't need this for
> |SystemMessageManager:GetPendingMessages:Return| type, though.

We need that because we can potentially have several pages using the same message manager (if we shared the same process for different apps for instance).

Comment 24

[[:fabrice] Fabrice Desré]

(In reply to Gene Lian [:gene] from comment #22)

> Also, according to Jonas' saying at [1] as described as below. Does the word
> "quickly" here means we have to make setMessageHandler() synchronous to
> handle the messages after calling hasPendingMessage()? Or it doesn't matter?

No, this doesn't need to be synchronous. I think we're good here, but let me know if you find edge case where we fail (like with the RIL messages...)

Comment 25

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 674450 [details] [diff] [review]
Blob changes, v1

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

I would complain about adding a silly interface, but I know you won't listen, so lets just skip to the part where I r+ the patch even though I don't like it.

::: dom/ipc/Blob.cpp
@@ +35,5 @@
>  namespace {
>  
> +class NS_NO_VTABLE IPrivateRemoteInputStream : public nsISupports
> +{
> +public: 

Extra whitespace at EOL.

@@ +1212,5 @@
>    MOZ_ASSERT(NS_IsMainThread());
>    MOZ_ASSERT(mBlob);
>    MOZ_ASSERT(!mRemoteBlob);
>  
> +

Extra newline.

Comment 26

[Gene Lian (I already quit Mozilla)]

Comment on attachment 674850 [details] [diff] [review]
patch v2

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

One question that I wasn't aware of: where do you wrap the objects now in the System Message mechanism?

::: dom/messages/SystemMessageManager.js
@@ +144,5 @@
> +      // Bail out if we have no handlers registered for this type.
> +      if (!(msg.type in this._handlers)) {
> +        debug("No handler for this type");
> +        return;
> +      }

After some thoughts, let's move this check out of this if-block (sorry I shouldn't have suggested doing this in my first review). This is needed because the content process might have chance to cancel the handler for this type at any time by:

  if (!aHandler) {
    // Setting the handler to null means we don't want to receive messages
    // of this type anymore.
    delete handlers[aType];
    return;
  }

Since the |SystemMessageManager:GetPendingMessages:Return| type is now async code, the handler could already be cancelled when receiving it.

@@ +149,1 @@
>      }

Move that check to here. ;)

@@ +153,5 @@
> +                   : msg.msgQueue;
> +
> +    messages.forEach(function(aMsg) {
> +      this._dispatchMessage(msg.type, this._handlers[msg.type], aMsg) },
> +    this);

Personally, I'd prefer make

}, this);

a single line, which seems more consistent with other conventions.

Also, please add a |;| at the end of this._dispatchMessage(...)

::: ipc/glue/ProtocolUtils.h
@@ +115,5 @@
>  inline void
>  ProtocolErrorBreakpoint(const char* aMsg)
>  {
> +    char* crashMe = nullptr;
> +    *crashMe = '!';

Not very sure what is this used for. Is this your testing code? Remove it?

Comment 27

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

Attachment: Blob changes, v1.1

Addressed khuey's nits.

Comment 28

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b660774e0993
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ca467baad43

Comment 29

[[:fabrice] Fabrice Desré]

Comment on attachment 674850 [details] [diff] [review]
patch v2

[Approval Request Comment]
User impact if declined: Worse API for dev, and more memory usage.
Testing completed (on m-c, etc.): Gaia test apps + no regression on m-c
Risk to taking this patch (and alternatives if risky): Low
String or UUID changes made by this patch: None

This is a big improvement to WebActivities, and that this will help us save memory when images are sent between apps, which is a common use case in Gaia.

Comment 30

[[:fabrice] Fabrice Desré]

Comment on attachment 675183 [details] [diff] [review]
Blob changes, v1.1

[Approval Request Comment]
User impact if declined: Worse API for dev, and more memory usage.
Testing completed (on m-c, etc.): Gaia test apps + no regression on m-c
Risk to taking this patch (and alternatives if risky): Low
String or UUID changes made by this patch: None

This is a big improvement to WebActivities, and that this will help us save memory when images are sent between apps, which is a common use case in Gaia.

Comment 31

[Ryan VanderMeulen (PTO, back 6-April)]

https://hg.mozilla.org/mozilla-central/rev/b660774e0993
https://hg.mozilla.org/mozilla-central/rev/1ca467baad43

Comment 32

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 675183 [details] [diff] [review]
Blob changes, v1.1

saving memory ftw!

Comment 33

[Ryan VanderMeulen (PTO, back 6-April)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/12babf7726ad
https://hg.mozilla.org/releases/mozilla-aurora/rev/623e90f8a84b