Closed Bug 1149274 Opened 5 years ago Closed 5 years ago

Clear site-permissions should clear all registered push notifications

Categories

(Core :: DOM: Push Notifications, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: dougt, Assigned: dougt)

References

Details

Attachments

(1 file)

Currently we're using webapps-clear-data, but that doesn't work for sites.



case "webapps-clear-data":
        debug("webapps-clear-data");

        let data = aSubject.QueryInterface(Ci.mozIApplicationClearPrivateDataParams);
        if (!data) {
          debug("webapps-clear-data: Failed to get information about application");
          return;
        }

        // TODO
        // Need a way to go from manifest url to 'all scopes registered for push in this app'
        let appsService = Cc["@mozilla.org/AppsService;1"]
                            .getService(Ci.nsIAppsService);
        let scope = appsService.getScopeByLocalId(data.appId);
        if (!scope) {
          debug("webapps-clear-data: No scope found for " + data.appId);
          return;
        }

        this._db.getByScope(scope, function(record) {
          this._db.delete(records.channelID, null, function() {
              debug("webapps-clear-data: " + scope +
                    " Could not delete entry " + records.channelID);

            // courtesy, but don't establish a connection
            // just for it
            if (this._ws) {
              debug("Had a connection, so telling the server");
              this._send("unregister", {channelID: records.channelID});
            }
          }.bind(this), function() {
            debug("webapps-clear-data: Error in getByScope(" + scope + ")");
          });
        });

        break;
    }
Blocks: 1153499
Attachment #8594437 - Flags: review?(nsm.nikhil)
Assignee: nobody → dougt
QA Contact: dougt
Comment on attachment 8594437 [details] [diff] [review]
bug_xxxxxx_clearAll_from_ff_ui

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

::: dom/push/PushService.jsm
@@ +1630,5 @@
>  
> +  _clearAll: function _clearAll() {
> +    return new Promise((resolve, reject) => {
> +      this._db.clearAll(() => resolve(),
> +                        () => reject("Database error"));

Why not just this._db.clearAll(resolve, reject)?

@@ +1633,5 @@
> +      this._db.clearAll(() => resolve(),
> +                        () => reject("Database error"));
> +    });
> +  },
> +  

Nit: trailing whitespace
Attachment #8594437 - Flags: review?(nsm.nikhil) → review+
> Why not just this._db.clearAll(resolve, reject)?

I wanted the reject to have some error string.
https://hg.mozilla.org/mozilla-central/rev/82ea538860bc
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Blocks: 1188686
You need to log in before you can comment on or make changes to this bug.