949946 - "Assertion failure: !domainInfo->mSharedWorkerInfos.Get(sharedWorkerScriptSpec)"

Status: VERIFIED FIXED

(Core :: DOM: Workers, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Jesse Ruderman]

Assertion failure: !domainInfo->mSharedWorkerInfos.Get(sharedWorkerScriptSpec), at /Users/jruderman/trees/mozilla-central/dom/workers/RuntimeService.cpp:1314

Comment 3

[Andrea Marchesini [:baku]]

Attachment: crash.patch

I think that we can keep the name as nsCString instead nsString. The name getter is less common than any lookup in the hash table.

Comment 4

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

Comment on attachment 8370222 [details] [diff] [review]
crash.patch

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

Thanks!

::: dom/workers/RuntimeService.cpp
@@ +276,5 @@
>  }
>  
> +// This function creates a key for a SharedWorker composed by "name|scriptSpec".
> +// If the name contains a '|', this will be replaced by '||'.
> +static void

Nit: This is in an anonymous namespace so the 'static' isn't needed.

@@ +280,5 @@
> +static void
> +GenerateSharedWorkerKey(const nsACString& aScriptSpec, const nsACString& aName,
> +                        nsCString& aKey)
> +{
> +  aKey.Truncate();

Let's do a SetCapacity(aScriptSpec.Length() + aName.Length() + 1) to avoid mallocing more than once in the common case?

@@ +293,5 @@
> +      aKey.Append(*start);
> +    }
> +  }
> +
> +  aKey.AppendASCII("|");

Just |Append('|')|

@@ +1328,5 @@
>        domainInfo->mActiveWorkers.AppendElement(aWorkerPrivate);
>      }
>  
>      if (isSharedWorker) {
> +      nsCString key;

Let's make this nsAutoCString, and everywhere else where you call GenerateSharedWorkerKey

Comment 5

[Andrea Marchesini [:baku]]

https://tbpl.mozilla.org/?tree=Try&rev=59cdea3806b6

Comment 6

[Andrea Marchesini [:baku]]

 https://hg.mozilla.org/integration/mozilla-inbound/rev/e29e610f7567

Comment 7

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

We should really get this on aurora.

Comment 8

[Andrea Marchesini [:baku]]

Comment on attachment 8370222 [details] [diff] [review]
crash.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 643325
User impact if declined: a crash
Testing completed (on m-c, etc.): m-c 
Risk to taking this patch (and alternatives if risky): none.
String or IDL/UUID changes made by this patch: none.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e56650e6afe2

Comment 11

[Paul Silaghi, QA [:pauly]]

Reproduced in Nightly 2013-12-18-mozilla-central-debug.
Verified fixed in Nightly 2014-02-10-mozilla-central-debug, Win 7 x64.