1233907 - bookmark import needs to use the default user context origin attributes

Status: RESOLVED WONTFIX

(Firefox :: Bookmarks & History, defect)

Description

[Dave Huseby [:huseby]]

In the toolkit/components/places/BookmarkJSONUtils.jsm file there is a call to createCodebasePrincipal that needs to use the default global user context since we aren't doing any kind of user context isolation of bookmarks yet.

> toolkit/components/places/BookmarkJSONUtils.jsm
>   line 210
>       let channel = NetUtil.newChannel({
>         uri,
>         loadingPrincipal: Services.scriptSecurityManager.createCodebasePrincipal(uri, {}),
>         contentPolicyType: Ci.nsIContentPolicy.TYPE_INTERNAL_XMLHTTPREQUEST
>       });

Comment 1

[Dave Huseby [:huseby]]

Attachment: Bug_1233907.patch

Comment 2

[Dave Huseby [:huseby]]

Attachment: Bug_1233907.patch

updated for new API.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 8722055 [details] [diff] [review]
Bug_1233907.patch

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

::: toolkit/components/places/BookmarkJSONUtils.jsm
@@ +207,5 @@
>        };
>  
>        let uri = NetUtil.newURI(spec);
> +      let attrs ChromeUtils.createOriginAttributesFromOrigin(spec);
> +      attrs.userContextId = 0;

Since this is run by chrome code, i think you can simply assert that the userContextId is 0 here.

Also, you're missing a '=' after 'attrs' on the first line.

And, if 'spec' is a valid URI, then the ChromeUtils.createOriginAttributesFromOrigin call here is effectively the same as ChromeUtils.createOriginAttributes({}), right?

Comment 4

[Dave Huseby [:huseby]]

Attachment: Bug_1233907.patch

switched the patch to just an assert.

Comment 5

[Dave Huseby [:huseby]]

try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=945e2f9271fc

Comment 6

[Dave Huseby [:huseby]]

oooh, look at all of the red.  try one more time just to see: https://treeherder.mozilla.org/#/jobs?repo=try&revision=975cce4ce590

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 8726502 [details] [diff] [review]
Bug_1233907.patch

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

r=me assuming that NS_ASSERT is in fact defined here.

Comment 8

[Dave Huseby [:huseby]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=30f5d144a61c

Comment 9

[Dave Huseby [:huseby]]

I just rebased and notcied that the code has been refactored to support loading bookmarks with the system principal: http://mzl.la/1Mqr3zQ

This patch is no longer needed AFAICT.