Closed Bug 931307 Opened 9 years ago Closed 9 years ago

"Assertion failure: !aTitle.IsEmpty()" with DOM Notification

Categories

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

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: jruderman, Assigned: baku)

References

Details

(Keywords: assertion, testcase, Whiteboard: [systemsfe][qa-])

Attachments

(4 files, 1 obsolete file)

Attached file testcase
Assertion failure: !aTitle.IsEmpty(), at dom/src/notification/Notification.cpp:63
Attached file stack
Whiteboard: [systemsfe]
Attached patch notification.patch (obsolete) — Splinter Review
Following the spec, title can be an empty string.
Attachment #823365 - Flags: review?(wchen)
Assignee: nobody → amarchesini
OS: Mac OS X → All
Hardware: x86_64 → All
Comment on attachment 823365 [details] [diff] [review]
notification.patch

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

Hijacking this review since the bug was caused by my code.

::: dom/tests/mochitest/notification/mochitest.ini
@@ +4,5 @@
>    NotificationTest.js
>  
>  [test_notification_basics.html]
>  [test_notification_storage.html]
> +[test_test_bug931307.html]

This should be:

[test_bug931307.html]

::: dom/tests/mochitest/notification/test_bug931307.html
@@ +13,5 @@
> +<script type="application/javascript"><!--
> +
> +new Notification("");
> +Notification.get();
> +ok(true, "No crash!");

The crash was happening in NotificationStorageCallback::Handle, which happens asynchronously from this content script. To be on the safe side, let's only succeed in the promise callback. So perhaps something like:

SimpleTest.waitForExplicitFinish();
new Notification("");
var promise = Notification.get();
promise.then(
  function onSuccess() {
    ok(true, "No crash!");
    SimpleTest.finish();
  },
  function onFailure() {
    ok(false, "Should not get an error in promise callback");
  }
);
Attachment #823365 - Attachment is obsolete: true
Attachment #823365 - Flags: review?(wchen)
Attachment #824387 - Flags: review?(mhenretty)
Comment on attachment 824387 [details] [diff] [review]
notification.patch

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

LGTM!

We need some tests up in gaia for this too, so I'll add that to this bug.
Attachment #824387 - Flags: review?(mhenretty) → review+
Whiteboard: [systemsfe] → [systemsfe][keep open]
Once the Gecko patch lands, I'll re-kick off the travis test and flag Kyle for review.
Keywords: checkin-needed
Comment on attachment 824739 [details] [review]
Pull Request for Gaia Integration Test

Kyle, will you review this crash test up in gaia land?
Attachment #824739 - Flags: review?(kyle)
Attachment #824739 - Flags: review?(kyle) → review+
master: https://github.com/mozilla-b2g/gaia/commit/f79810d49bb83d515aacc4880468c0b29039c0cd
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [systemsfe][keep open] → [systemsfe]
Whiteboard: [systemsfe] → [systemsfe][qa-]
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.