1279493 - Use blob URLs exclusively rather than having mediastream and mediasource URLs

Status: RESOLVED FIXED

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

Description

[Anne (:annevk)]

See https://github.com/whatwg/url/issues/126 for details.

I think the basic idea is to have one store for all these object URLs and one scheme.

Comment 1

[Andrew Overholt [:overholt]]

I think baku knows about this situation.

Comment 2

[Andrea Marchesini [:baku]]

I wrote something for blobURL in multi-e10s (bug 1279186) and I want to see that landed before changing that code again: with this patch we have 2 indexes,  one on the parent process just for blob URLs, and another one 'locally' (per process) for any other mediastream/mediasource/font URLs. The solution here is to have 2 indexes again, but both for blob URLs: if the underlying object is a real blob/file object, we use the index running in the parent, otherwise the local (per process) one.

Comment 3

[Andrea Marchesini [:baku]]

Attachment: patch

It can be that I have to fix some tests. The patch has been pushed to try.

Comment 4

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

Comment on attachment 8772915 [details] [diff] [review]
patch

>+  nsresult rv =
>+    GenerateURIString(NS_LITERAL_CSTRING(BLOBURI_SCHEME), aPrincipal, aUri);
Might be worth to add a helper method GenerateBlobURIString which takes the latter 2 params and passes 
NS_LITERAL_CSTRING(BLOBURI_SCHEME) to GenerateURIString

>@@ -448,22 +496,22 @@ nsHostObjectProtocolHandler::GetAllBlobU
>   if (!gDataTable) {
>     return true;
>   }
> 
>   for (auto iter = gDataTable->ConstIter(); !iter.Done(); iter.Next()) {
>     DataInfo* info = iter.UserData();
>     MOZ_ASSERT(info);
> 
>-    nsCOMPtr<BlobImpl> blobImpl = do_QueryInterface(info->mObject);
>-    if (!blobImpl) {
>+    if (info->mObjectType == DataInfo::eBlobImpl) {
>       continue;
>     }
This looks like a bug. The old version continues if we don't have blobImpl, the new one only if we do have it.


> 
>-    PBlobParent* blobParent = aCP->GetOrCreateActorForBlobImpl(blobImpl);
>+    MOZ_ASSERT(info->mBlobImpl);
And this assert then fires.

>+  ErrorResult rv;
>+
> #ifdef DEBUG
>   nsCString spec;
>-  uri->GetSpec(spec);
>+  rv = uri->GetSpec(spec);
>+  if (NS_WARN_IF(rv.Failed())) {
>+    return rv.StealNSResult();
>+  }
Weird to use ErrorResult for nsresult handling. Please use nsresult.


> nsresult
> NS_GetStreamForMediaStreamURI(nsIURI* aURI, mozilla::DOMMediaStream** aStream)
> {
>   NS_ASSERTION(IsMediaStreamURI(aURI), "Only call this with mediastream URIs");
> 
>-  nsISupports* dataObject = GetDataObject(aURI);
>-  if (!dataObject) {
>+  nsCString spec;
>+  nsresult rv = aURI->GetSpec(spec);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  DataInfo* info = GetDataInfo(spec);
There should be a version of GetDataInfo which takes nsIURI*
That way there would be less copy-pasting code

Comment 5

[Andrea Marchesini [:baku]]

Attachment: blobUrl2.patch

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6e4999d9dd2
Use blob URLs exclusively rather than having mediastream and mediasource URLs, r=smaug

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d6e4999d9dd2