Bugzilla

The patch from Bug 543435 failed the subtest `test_remote_window_open_aboutBlank` in test [browser_firstPartyIsolation_aboutPages.js](https://searchfox.org/mozilla-central/rev/43ed185f20ba4d3ef28c78b85102ea88fa63c947/browser/components/originattributes/test/browser/browser_firstPartyIsolation_aboutPages.js#18-43) and several related ones. Paraphrasing the log:
```
PASS should be a remote browser
INFO origin moz-nullprincipal:{9865c259-3a40-44b0-99d3-7994f00bee66}
PASS The principal of remote about:blank should be a NullPrincipal.
FAIL remote about:blank should have firstPartyDomain set to ... - got "", expected "9865c259-3a40-44b0-99d3-7994f00bee66.mozilla
```

From [pernosco](https://pernos.co/debug/FjWXxM42gUuM13xCCxbH3w/index.html), we can see that we call `BasePrincipal::GetOrigin` and `mOriginNoSuffix` is empty. On central ([pernosco](https://pernos.co/debug/vz1nIfmT6YZGuaed-JzpGg/index.html)), we would've created the right principal from `nsDocShell::LoadURI`  in `nsDocShellLoadState::SetupInheritingPrincipal` and propagated it to the Document during the channel load from `CreateDocument`, `StartDocumentLoad`, `Reset`, `ResetToURI`.

With the patch, we still do `LoadURI` and `SetupInheritingPrincipal`, but skip the channel load and rather commit to the already existing initial `about:blank` in `DoURILoad`. From [:smaug comment 188](https://bugzilla.mozilla.org/show_bug.cgi?id=543435#c188) we want to avoid changing the document principal and would prefer to have the right principal from the beginning.

So this leaves us with setting `firstPartyDomain` during `CreateBrowser`. :hsivonen added the `mPrincipalToInheritForAboutBlank` to `nsOpenWindowInfo`, which is set in `ContentParent::CreateBrowser` and then propagated to the document in the content process.
1. `mPrincipalToInheritForAboutBlank` is largely const, but we could create a new principal based on it somewhere in the child.
2. `ContentParent::CreateBrowser` could copy `aBrowsingContext->OriginAttributesRef()` and `SetFirstPartyDomain` when creating `mPrincipalToInheritForAboutBlank`. This will trigger [this](https://searchfox.org/mozilla-central/rev/e8da1e780e9b8ed2fd82a3b8d79c5f93e72697d3/docshell/base/nsDocShell.cpp#6444) assert, but I didn't see test failures when ignoring that assert.
3. When `OriginAttributesRef` is initialized during `nsFrameLoader::EnsureBrowsingContextAttached`, or somewhere else during browsing context creation, we could initialize the `firstPartyDomain`.

I favor the second option. `OriginAttributesRef` might impact much more, and I wouldn't know where to create the principal in the child. But the question remains what to do about the assertion. Remove? Exclude `about:blank`? Or should we update the bc origin attributes?

The patch from Bug 543435 failed the subtest `test_remote_window_open_aboutBlank` in test [browser_firstPartyIsolation_aboutPages.js](https://searchfox.org/mozilla-central/rev/43ed185f20ba4d3ef28c78b85102ea88fa63c947/browser/components/originattributes/test/browser/browser_firstPartyIsolation_aboutPages.js#18-43) and several related ones. Paraphrasing the log:
```
PASS should be a remote browser
INFO origin moz-nullprincipal:{9865c259-3a40-44b0-99d3-7994f00bee66}
PASS The principal of remote about:blank should be a NullPrincipal.
FAIL remote about:blank should have firstPartyDomain set to ... - got "", expected "9865c259-3a40-44b0-99d3-7994f00bee66.mozilla
```

From [pernosco](https://pernos.co/debug/FjWXxM42gUuM13xCCxbH3w/index.html), we can see that we call `BasePrincipal::GetOrigin` and `mOriginNoSuffix` is empty. On central ([pernosco](https://pernos.co/debug/vz1nIfmT6YZGuaed-JzpGg/index.html)), we would've created the right principal from `nsDocShell::LoadURI`  in `nsDocShellLoadState::SetupInheritingPrincipal` and propagated it to the Document during the channel load from `CreateDocument`, `StartDocumentLoad`, `Reset`, `ResetToURI`.

With the patch, we still do `LoadURI` and `SetupInheritingPrincipal`, but skip the channel load and rather commit to the already existing initial `about:blank` in `DoURILoad`. From [:smaug comment 188](https://bugzilla.mozilla.org/show_bug.cgi?id=543435#c188) we want to avoid changing the document principal and would prefer to have the right principal from the beginning.

So this leaves us with setting `firstPartyDomain` during `CreateBrowser`. Within the patch, :hsivonen added the `mPrincipalToInheritForAboutBlank` to `nsOpenWindowInfo`, which is set in `ContentParent::CreateBrowser` and then propagated to the document in the content process.
1. `mPrincipalToInheritForAboutBlank` is largely const, but we could create a new principal based on it somewhere in the child.
2. `ContentParent::CreateBrowser` could copy `aBrowsingContext->OriginAttributesRef()` and `SetFirstPartyDomain` when creating `mPrincipalToInheritForAboutBlank`. This will trigger [this](https://searchfox.org/mozilla-central/rev/e8da1e780e9b8ed2fd82a3b8d79c5f93e72697d3/docshell/base/nsDocShell.cpp#6444) assert, but I didn't see test failures when ignoring that assert.
3. When `OriginAttributesRef` is initialized during `nsFrameLoader::EnsureBrowsingContextAttached`, or somewhere else during browsing context creation, we could initialize the `firstPartyDomain`.

I favor the second option. `OriginAttributesRef` might impact much more, and I wouldn't know where to create the principal in the child. But the question remains what to do about the assertion. Remove? Exclude `about:blank`? Or should we update the bc origin attributes?

The patch from Bug 543435 failed the subtest `test_remote_window_open_aboutBlank` in test [browser_firstPartyIsolation_aboutPages.js](https://searchfox.org/mozilla-central/rev/43ed185f20ba4d3ef28c78b85102ea88fa63c947/browser/components/originattributes/test/browser/browser_firstPartyIsolation_aboutPages.js#18-43) and several related ones. Paraphrasing the log:
```
PASS should be a remote browser
INFO origin moz-nullprincipal:{9865c259-3a40-44b0-99d3-7994f00bee66}
PASS The principal of remote about:blank should be a NullPrincipal.
FAIL remote about:blank should have firstPartyDomain set to ... - got "", expected "9865c259-3a40-44b0-99d3-7994f00bee66.mozilla
```

From [pernosco](https://pernos.co/debug/FjWXxM42gUuM13xCCxbH3w/index.html), we can see that we call `BasePrincipal::GetOrigin` and `mOriginNoSuffix` is empty. On central ([pernosco](https://pernos.co/debug/vz1nIfmT6YZGuaed-JzpGg/index.html)), we would've created the right principal from `nsDocShell::LoadURI`  in `nsDocShellLoadState::SetupInheritingPrincipal` and propagated it to the Document during the channel load from `CreateDocument`, `StartDocumentLoad`, `Reset`, `ResetToURI`.

With the patch, we still do `LoadURI` and `SetupInheritingPrincipal`, but skip the channel load and rather commit to the already existing initial `about:blank` in `DoURILoad`. From [:smaug comment 188](https://bugzilla.mozilla.org/show_bug.cgi?id=543435#c188) we want to avoid changing the document principal and would prefer to have the right principal from the beginning.

So this leaves us with setting `firstPartyDomain` during `CreateBrowser`. Within the patch, :hsivonen added the `mPrincipalToInheritForAboutBlank` to `nsOpenWindowInfo`, which is set in `ContentParent::CreateBrowser` and then propagated to the document in the content process.
1. `mPrincipalToInheritForAboutBlank` is largely const, but we could create a new principal based on it somewhere in the child.
2. `ContentParent::CreateBrowser` could copy `aBrowsingContext->OriginAttributesRef()` and `SetFirstPartyDomain` when creating `mPrincipalToInheritForAboutBlank`. This will trigger [this](https://searchfox.org/mozilla-central/rev/e8da1e780e9b8ed2fd82a3b8d79c5f93e72697d3/docshell/base/nsDocShell.cpp#6444) assert, but I didn't see test failures when ignoring that assert.
3. When `OriginAttributesRef` is initialized during `nsFrameLoader::EnsureBrowsingContextAttached`, or somewhere else during browsing context creation, we could initialize the `firstPartyDomain`. Though we won't have `mPrincipalToInheritForAboutBlank` jet whose origin is used to generate the suffix string. We could use `ABOUT_URI_FIRST_PARTY_DOMAIN` see [c194](https://bugzilla.mozilla.org/show_bug.cgi?id=543435#c194), but this doesn't align with the test.

I favor the second option, but the question remains what to do about the assertion. Remove? Exclude `about:blank`? Or should we update the bc origin attributes?