Closed Bug 1186909 Opened 6 years ago Closed 6 years ago

Use eTLD+1 rather than host for the history query lookup for nsIPermissionManager migrations

Categories

(Core :: Permission Manager, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: nika, Assigned: nika)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Assignee: nobody → michael
Comment on attachment 8637923 [details] [diff] [review]
Use eTLD+1 rather than host for the history query lookup for nsIPermissionManager migrations

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

::: extensions/cookie/nsPermissionManager.cpp
@@ +343,5 @@
> +      rv = tldService->GetBaseDomainFromHost(aHost, 0, eTLD1);
> +      NS_ENSURE_SUCCESS(rv, rv);
> +    } else {
> +      NS_WARNING("Should have a tld service!");
> +      eTLD1 = aHost;

This is technically incorrect, but there is no reason why the do_GetService() call above should fail, since this code cannot be called during shutdown, so r=me.
Attachment #8637923 - Flags: review?(ehsan) → review+
(In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!) from comment #2)
> This is technically incorrect, but there is no reason why the
> do_GetService() call above should fail, since this code cannot be called
> during shutdown, so r=me.

After bug 1185239 I'm just defensively guarding against services being unavailable I think :).

Would you like me to MOZ_ASSERT(tldService) instead?
Flags: needinfo?(ehsan)
Instead, no.  In addition to, yes please!
Flags: needinfo?(ehsan)
https://hg.mozilla.org/mozilla-central/rev/2c86f53d8b66
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.