1351835 - Update PrivacyLevel.canSave() calls in SessionCookies.jsm

Status: RESOLVED FIXED

(Firefox :: Session Restore, enhancement)

Description

[Tim Taubert [:ttaubert] (inactive)]

When bug 1235379 landed, we for some reason didn't touch SessionCookies.jsm. The PrivacyLevel.canSave() method has only a single `isHTTPS` parameter left, the `isPinned` is gone. We can simplify quite a bit of code that existed only to support the deferred privacy level stuff.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1351835-Update-PrivacyLevel.canSave-calls-in-Ses.patch

Here's a patch that cleans up some more stuff too.

Comment 2

[Mike de Boer [:mikedeboer]]

Comment on attachment 8852658 [details] [diff] [review]
0001-Bug-1351835-Update-PrivacyLevel.canSave-calls-in-Ses.patch

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

Nice! I think your commit message could use some TLC, but other than that this is ready to go ;-)

::: browser/components/sessionstore/SessionStore.jsm
@@ +4318,5 @@
>      let cookieHosts = SessionCookies.getHostsForWindow(aTargetWinState);
>  
>      // By creating a regex we reduce overhead and there is only one loop pass
>      // through either array (cookieHosts and aWinState.cookies).
> +    let hosts = [...cookieHosts].join("|").replace(/\./g, "\\.");

Unrelated note: what I know about escaping strings for use in constructing a regex is this:

```js
str => str.replace(/([.*+?\^${}()|\[\]\/\\])/g, "\\$1");
```

I'm not sure what the odds are that any of these characters can be used in a hostname. Punycode?

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Mike de Boer [:mikedeboer] from comment #2)
> >      // By creating a regex we reduce overhead and there is only one loop pass
> >      // through either array (cookieHosts and aWinState.cookies).
> > +    let hosts = [...cookieHosts].join("|").replace(/\./g, "\\.");
> 
> Unrelated note: what I know about escaping strings for use in constructing a
> regex is this:
> 
> ```js
> str => str.replace(/([.*+?\^${}()|\[\]\/\\])/g, "\\$1");
> ```
> 
> I'm not sure what the odds are that any of these characters can be used in a
> hostname. Punycode?

I honestly have no clue what the regex is used for, or if it's really necessary. I'd prefer to not touch it ;)

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

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

Comment 5

[Ryan VanderMeulen [:RyanVM]]

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