Notification.get() should reject promise on error cases

RESOLVED WONTFIX

Status

()

Core
General
RESOLVED WONTFIX
5 years ago
2 years ago

People

(Reporter: gerard, Unassigned)

Tracking

Trunk
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

5 years ago
In bug 899574, the Notification.get() has been implemented. While using and testing its usage inside the Dialer app, I noticed that sometimes my Notification.get().then() callbacks does not get called.

I've investigated with gdb, and I get the following:
(gdb) n
694	  aRv = GetOrigin(window, origin);
(gdb) print origin
$2 = {<nsAString_internal> = {mData = 0x41ec6de0 u"", mLength = 0, mFlags = 1}, <No data fields>}
(gdb) n
695	  if (aRv.Failed()) {
(gdb) 
701	    do_GetService(NS_NOTIFICATION_STORAGE_CONTRACTID, &rv);
(gdb) print origin
$3 = {<nsAString_internal> = {mData = 0x449f2518 u"app://communications.gaiamobile.org/manifest.webapp", mLength = 51, mFlags = 5}, <No data fields>}
(gdb) n
702	  if (NS_FAILED(rv)) {
(gdb) 
180	  return static_cast<uint32_t>(_nsresult) & 0x80000000;
(gdb) n
703	    aRv.Throw(rv);
(gdb) n
180	  return static_cast<uint32_t>(_nsresult) & 0x80000000;
(gdb) l
175	 * @return 0 or 1 (false/true with bool type for C++)
176	 */
177	
178	#ifdef __cplusplus
179	inline uint32_t NS_FAILED_impl(nsresult _nsresult) {
180	  return static_cast<uint32_t>(_nsresult) & 0x80000000;
181	}
182	#define NS_FAILED(_nsresult)    ((bool)MOZ_UNLIKELY(NS_FAILED_impl(_nsresult)))
183	#define NS_SUCCEEDED(_nsresult) ((bool)MOZ_LIKELY(!NS_FAILED_impl(_nsresult)))
184	
(gdb) n
707	  nsRefPtr<Promise> promise = new Promise(window);
(gdb) n
709	    new NotificationStorageCallback(aGlobal, window, promise);
(gdb) n
[New Thread 961.1013]
43	    bool empty() const { return !constructed; }
(gdb) 
712	                 EmptyString();
(gdb) 
43	    bool empty() const { return !constructed; }
(gdb) 
712	                 EmptyString();

So, as far as I understand, it means we are passing inside this branch: https://hg.mozilla.org/mozilla-central/file/60373454fc52/dom/src/notification/Notification.cpp#l703

I think this is wrong, since it's a promise we should never return a null pointer but rather reject the promise with the reject() method.

Updated

5 years ago
Blocks: 911002
But do you know what was the initial failure, Alexandre?
(Reporter)

Comment 2

4 years ago
No, but I still think we should fix this.
(Reporter)

Updated

2 years ago
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.