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

VERIFIED FIXED in Firefox 29

Status

()

Core
DOM: Workers
--
critical
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: Jesse Ruderman, Assigned: baku)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
mozilla30
x86_64
Mac OS X
assertion, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox29+ fixed, firefox30 verified)

Details

Attachments

(3 attachments)

198 bytes, text/html
Details
11.28 KB, text/plain
Details
19.63 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
(Reporter)

Description

4 years ago
Created attachment 8347102 [details]
testcase
Flags: needinfo?(bent.mozilla)
(Reporter)

Comment 1

4 years ago
Created attachment 8347103 [details]
stack
(Reporter)

Comment 2

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

Updated

4 years ago
Assignee: nobody → amarchesini
(Assignee)

Comment 3

4 years ago
Created attachment 8370222 [details] [diff] [review]
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.
Attachment #8370222 - Flags: review?(bent.mozilla)
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
Attachment #8370222 - Flags: review?(bent.mozilla) → review+
Flags: needinfo?(bent.mozilla)
(Assignee)

Comment 5

4 years ago
https://tbpl.mozilla.org/?tree=Try&rev=59cdea3806b6
(Assignee)

Comment 6

4 years ago
 https://hg.mozilla.org/integration/mozilla-inbound/rev/e29e610f7567
We should really get this on aurora.
(Assignee)

Comment 8

4 years ago
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.
Attachment #8370222 - Flags: approval-mozilla-aurora?
Attachment #8370222 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-firefox29: --- → affected
tracking-firefox29: --- → +
https://hg.mozilla.org/mozilla-central/rev/e29e610f7567
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
https://hg.mozilla.org/releases/mozilla-aurora/rev/e56650e6afe2
status-firefox29: affected → fixed
status-firefox30: --- → fixed
Reproduced in Nightly 2013-12-18-mozilla-central-debug.
Verified fixed in Nightly 2014-02-10-mozilla-central-debug, Win 7 x64.
Status: RESOLVED → VERIFIED
status-firefox30: fixed → verified
You need to log in before you can comment on or make changes to this bug.