Open Bug 1580281 Opened 4 months ago Updated 28 days ago

Fix usage of nsIDocShellTreeItem in nsDocShell::SetupNewViewer

Categories

(Core :: DOM: Navigation, defect, P2)

defect

Tracking

()

Fission Milestone M6

People

(Reporter: djvj, Unassigned)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

(Whiteboard: [rm-docshell-tree-item:sync-state])

https://searchfox.org/mozilla-central/source/docshell/base/nsDocShell.cpp#8319

Fixing this seems a tad more involved. This code gets the parent docshell via the DocShellTreeItem interface, and then queries it for its ContentViewer (kept in oldCv). It then queries oldCv for the following parameters: GetForceCharset, GetHintCharset, GetHintCharacterSetSource, GetTextZoom, GetFullZoom GetOverrideDPPX, GetAuthorStyleDisabled.

Either these attributes need to be synced to BrowsingContext, or the set of attributes retrieved via IPC.

If this needs to be made into IPC and async IPC is a requirement, it seems like the main cutpoint is two calls above (via nsDocShell::Embed) to nsDocShell::CreateContentViewer (and CreateAboutBlankContentViewer).

@Nika - ^-- does the above sound reasonable?

I've copy-pasted the relevant bits of code below for easy reference:

   nsCOMPtr<nsIDocShellTreeItem> parentAsItem;
  NS_ENSURE_SUCCESS(GetInProcessSameTypeParent(getter_AddRefs(parentAsItem)),
                    NS_ERROR_FAILURE);
  nsCOMPtr<nsIDocShell> parent(do_QueryInterface(parentAsItem));
  ...
    parent->GetContentViewer(getter_AddRefs(oldCv));
  ...
    if (oldCv) {
      newCv = aNewViewer;
      if (newCv) {
        forceCharset = oldCv->GetForceCharset();
        hintCharset = oldCv->GetHintCharset();
        NS_ENSURE_SUCCESS(oldCv->GetHintCharacterSetSource(&hintCharsetSource),
                          NS_ERROR_FAILURE);
        NS_ENSURE_SUCCESS(oldCv->GetTextZoom(&textZoom), NS_ERROR_FAILURE);
        NS_ENSURE_SUCCESS(oldCv->GetFullZoom(&pageZoom), NS_ERROR_FAILURE);
        NS_ENSURE_SUCCESS(oldCv->GetOverrideDPPX(&overrideDPPX),
                          NS_ERROR_FAILURE);
        NS_ENSURE_SUCCESS(oldCv->GetAuthorStyleDisabled(&styleDisabled),
                          NS_ERROR_FAILURE);
      }
    }

Forgot to CC nika on question in top comment.

Flags: needinfo?(nika)

(In reply to Kannan Vijayan [:djvj] from comment #0)

https://searchfox.org/mozilla-central/source/docshell/base/nsDocShell.cpp#8319

Because this isn't a permalink, it starts pointing to the wrong line of code over time. I think the line you meant to reference is https://searchfox.org/mozilla-central/rev/588814f2edddf0e132d77d326ddae50911e8bad1/docshell/base/nsDocShell.cpp#8341

Fixing this seems a tad more involved. This code gets the parent docshell via the DocShellTreeItem interface, and then queries it for its ContentViewer (kept in oldCv). It then queries oldCv for the following parameters: GetForceCharset, GetHintCharset, GetHintCharacterSetSource, GetTextZoom, GetFullZoom GetOverrideDPPX, GetAuthorStyleDisabled.

Either these attributes need to be synced to BrowsingContext, or the set of attributes retrieved via IPC.

These can probably be sent alongside the other load context properties on the BrowsingContext object. Right now we pass this around with TabContext, but we're considering adding it as a property of the BrowsingContext. The zoom properties probably can to be synced that way. That being said, some properties like inherited charset shouldn't be used when cross-origin, so it's probably OK to not sync them in this case.

Apparently ForceCharset is thunderbird only (according to :smaug), so we can probably ignore that completely.

Flags: needinfo?(nika)
Fission Milestone: --- → M5
Priority: -- → P2
Depends on: 1587437
Whiteboard: [rm-docshell-tree-item:sync-state]
Fission Milestone: M5 → Future

Kannan says replacing nsIDocShellTreeItem calls should block enabling Fission in Nightly (M6).

Fission Milestone: Future → M6
You need to log in before you can comment on or make changes to this bug.