Bug 1718082 - move isAppTab from docshell to browsing context, r=nika,rpl

Differential Revision: https://phabricator.services.mozilla.com/D171412

browser/actors/BrowserTabChild.sys.mjs

@@ -37,12 +37,6 @@ export class BrowserTabChild extends JSWindowActorChild {
     let docShell = context.docShell;
 
     switch (message.name) {
-      case "Browser:AppTab":
-        if (docShell) {
-          docShell.isAppTab = message.data.isAppTab;
-        }
-        break;
-
       case "Browser:HasSiblings":
         try {
           let browserChild = docShell

browser/base/content/tabbrowser.js

@@ -760,11 +760,7 @@
     },
 
     _notifyPinnedStatus(aTab) {
-      aTab.linkedBrowser.sendMessageToActor(
-        "Browser:AppTab",
-        { isAppTab: aTab.pinned },
-        "BrowserTab"
-      );
+      aTab.linkedBrowser.browsingContext.isAppTab = aTab.pinned;
 
       let event = document.createEvent("Events");
       event.initEvent(aTab.pinned ? "TabPinned" : "TabUnpinned", true, false);
@@ -2049,12 +2045,6 @@
         // we call updatetabIndicatorAttr here, rather than _tabAttrModified, so as
         // to be consistent with how "crashed" attribute changes are handled elsewhere
         this.tabContainer.updateTabIndicatorAttr(tab);
-      } else {
-        aBrowser.sendMessageToActor(
-          "Browser:AppTab",
-          { isAppTab: tab.pinned },
-          "BrowserTab"
-        );
       }
 
       if (wasActive) {
@@ -2452,11 +2442,7 @@
         browser.setAttribute("usercontextid", aTab.userContextId);
       }
 
-      browser.sendMessageToActor(
-        "Browser:AppTab",
-        { isAppTab: aTab.pinned },
-        "BrowserTab"
-      );
+      browser.browsingContext.isAppTab = aTab.pinned;
 
       // We don't want to update the container icon and identifier if
       // this is not the selected browser.
@@ -6266,12 +6252,6 @@
             // crashed.
             tab.removeAttribute("crashed");
             gBrowser.tabContainer.updateTabIndicatorAttr(tab);
-          } else {
-            browser.sendMessageToActor(
-              "Browser:AppTab",
-              { isAppTab: tab.pinned },
-              "BrowserTab"
-            );
           }
 
           if (wasActive) {

browser/base/content/test/general/browser_restore_isAppTab.js

@@ -6,13 +6,10 @@ const { TabStateFlusher } = ChromeUtils.importESModule(
 );
 
 const DUMMY =
-  // eslint-disable-next-line @microsoft/sdl/no-insecure-url
-  "http://example.com/browser/browser/base/content/test/general/dummy_page.html";
+  "https://example.com/browser/browser/base/content/test/general/dummy_page.html";
 
 function isBrowserAppTab(browser) {
-  return SpecialPowers.spawn(browser, [], async () => {
-    return content.docShell.isAppTab;
-  });
+  return browser.browsingContext.isAppTab;
 }
 
 // Restarts the child process by crashing it then reloading the tab
@@ -38,29 +35,29 @@ add_task(async function navigate() {
   let browser = tab.linkedBrowser;
   gBrowser.selectedTab = tab;
   await BrowserTestUtils.browserStopped(gBrowser);
-  let isAppTab = await isBrowserAppTab(browser);
+  let isAppTab = isBrowserAppTab(browser);
   ok(!isAppTab, "Docshell shouldn't think it is an app tab");
 
   gBrowser.pinTab(tab);
-  isAppTab = await isBrowserAppTab(browser);
+  isAppTab = isBrowserAppTab(browser);
   ok(isAppTab, "Docshell should think it is an app tab");
 
   BrowserTestUtils.loadURIString(gBrowser, DUMMY);
   await BrowserTestUtils.browserStopped(gBrowser);
-  isAppTab = await isBrowserAppTab(browser);
+  isAppTab = isBrowserAppTab(browser);
   ok(isAppTab, "Docshell should think it is an app tab");
 
   gBrowser.unpinTab(tab);
-  isAppTab = await isBrowserAppTab(browser);
+  isAppTab = isBrowserAppTab(browser);
   ok(!isAppTab, "Docshell shouldn't think it is an app tab");
 
   gBrowser.pinTab(tab);
-  isAppTab = await isBrowserAppTab(browser);
+  isAppTab = isBrowserAppTab(browser);
   ok(isAppTab, "Docshell should think it is an app tab");
 
   BrowserTestUtils.loadURIString(gBrowser, "about:robots");
   await BrowserTestUtils.browserStopped(gBrowser);
-  isAppTab = await isBrowserAppTab(browser);
+  isAppTab = isBrowserAppTab(browser);
   ok(isAppTab, "Docshell should think it is an app tab");
 
   gBrowser.removeCurrentTab();
@@ -75,15 +72,15 @@ add_task(async function crash() {
   let browser = tab.linkedBrowser;
   gBrowser.selectedTab = tab;
   await BrowserTestUtils.browserStopped(gBrowser);
-  let isAppTab = await isBrowserAppTab(browser);
+  let isAppTab = isBrowserAppTab(browser);
   ok(!isAppTab, "Docshell shouldn't think it is an app tab");
 
   gBrowser.pinTab(tab);
-  isAppTab = await isBrowserAppTab(browser);
+  isAppTab = isBrowserAppTab(browser);
   ok(isAppTab, "Docshell should think it is an app tab");
 
   await restart(browser);
-  isAppTab = await isBrowserAppTab(browser);
+  isAppTab = isBrowserAppTab(browser);
   ok(isAppTab, "Docshell should think it is an app tab");
 
   gBrowser.removeCurrentTab();

browser/base/content/webext-panels.js

@@ -124,6 +124,8 @@ function getBrowser(panel) {
   };
 
   browser.addEventListener("DidChangeBrowserRemoteness", initBrowser);
+  // Potentially unnecessary: bug 1822037 will evaluate further.
+  browser.browsingContext.isAppTab = true;
   return readyPromise.then(initBrowser);
 }
 

browser/components/extensions/ExtensionPopups.jsm

@@ -403,6 +403,8 @@ class BasePopup {
     };
 
     browser.addEventListener("DidChangeBrowserRemoteness", initBrowser); // eslint-disable-line mozilla/balanced-listeners
+    // Potentially unnecessary: bug 1822037 will evaluate further.
+    browser.browsingContext.isAppTab = true;
 
     if (!popupURL) {
       // For remote browsers, we can't do any setup until the frame loader is

docshell/base/BrowsingContext.cpp

@@ -2899,6 +2899,11 @@ void BrowsingContext::DidSet(FieldIndex<IDX_Muted>) {
   });
 }
 
+bool BrowsingContext::CanSet(FieldIndex<IDX_IsAppTab>, const bool& aValue,
+                             ContentParent* aSource) {
+  return XRE_IsParentProcess() && !aSource && IsTop();
+}
+
 bool BrowsingContext::CanSet(FieldIndex<IDX_ShouldDelayMediaFromStart>,
                              const bool& aValue, ContentParent* aSource) {
   return IsTop();

docshell/base/BrowsingContext.h

@@ -136,6 +136,9 @@ struct EmbedderColorSchemes {
   /* Hold the audio muted state and should be used on top level browsing      \
    * contexts only */                                                         \
   FIELD(Muted, bool)                                                          \
+  /* Hold the pinned/app-tab state and should be used on top level browsing   \
+   * contexts only */                                                         \
+  FIELD(IsAppTab, bool)                                                       \
   /* Indicate that whether we should delay media playback, which would only   \
      be done on an unvisited tab. And this should only be used on the top     \
      level browsing contexts */                                               \
@@ -939,6 +942,8 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache {
 
   void AddDiscardListener(std::function<void(uint64_t)>&& aListener);
 
+  bool IsAppTab() { return GetIsAppTab(); }
+
  protected:
   virtual ~BrowsingContext();
   BrowsingContext(WindowContext* aParentWindow, BrowsingContextGroup* aGroup,
@@ -1111,6 +1116,9 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache {
   // volume of all media elements.
   void DidSet(FieldIndex<IDX_Muted>);
 
+  bool CanSet(FieldIndex<IDX_IsAppTab>, const bool& aValue,
+              ContentParent* aSource);
+
   bool CanSet(FieldIndex<IDX_ShouldDelayMediaFromStart>, const bool& aValue,
               ContentParent* aSource);
   void DidSet(FieldIndex<IDX_ShouldDelayMediaFromStart>, bool aOldValue);

docshell/base/CanonicalBrowsingContext.cpp

@@ -312,6 +312,7 @@ void CanonicalBrowsingContext::ReplacedBy(
   // SetWithoutSyncing can be used if context hasn't been attached.
   Transaction txn;
   txn.SetBrowserId(GetBrowserId());
+  txn.SetIsAppTab(GetIsAppTab());
   txn.SetHistoryID(GetHistoryID());
   txn.SetExplicitActive(GetExplicitActive());
   txn.SetEmbedderColorSchemes(GetEmbedderColorSchemes());

docshell/base/nsDocShell.cpp

@@ -352,7 +352,6 @@ nsDocShell::nsDocShell(BrowsingContext* aBrowsingContext,
       mAllowAuth(mItemType == typeContent),
       mAllowKeywordFixup(false),
       mDisableMetaRefreshWhenInactive(false),
-      mIsAppTab(false),
       mDeviceSizeIsPageSize(false),
       mWindowDraggingAllowed(false),
       mInFrameSwap(false),
@@ -4985,18 +4984,6 @@ void nsDocShell::ActivenessMaybeChanged() {
   }
 }
 
-NS_IMETHODIMP
-nsDocShell::SetIsAppTab(bool aIsAppTab) {
-  mIsAppTab = aIsAppTab;
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-nsDocShell::GetIsAppTab(bool* aIsAppTab) {
-  *aIsAppTab = mIsAppTab;
-  return NS_OK;
-}
-
 NS_IMETHODIMP
 nsDocShell::SetDefaultLoadFlags(uint32_t aDefaultLoadFlags) {
   if (!mWillChangeProcess) {
@@ -12971,7 +12958,8 @@ bool nsDocShell::ShouldOpenInBlankTarget(const nsAString& aOriginalTarget,
   // Only check targets that are in extension panels or app tabs.
   // (isAppTab will be false for app tab subframes).
   nsString mmGroup = mBrowsingContext->Top()->GetMessageManagerGroup();
-  if (!mmGroup.EqualsLiteral("webext-browsers") && !mIsAppTab) {
+  if (!mmGroup.EqualsLiteral("webext-browsers") &&
+      !mBrowsingContext->IsAppTab()) {
     return false;
   }
 

docshell/base/nsIDocShell.idl

@@ -447,13 +447,6 @@ interface nsIDocShell : nsIDocShellTreeItem
    */
   [noscript, notxpcom] nsIDRef HistoryID();
 
-  /**
-   * Sets whether a docshell is an app tab. An app tab docshell may behave
-   * differently than a non-app tab docshell in some cases, such as when
-   * handling link clicks. Docshells are not app tabs unless told otherwise.
-   */
-  attribute boolean isAppTab;
-
   /**
    * Create a new about:blank document and content viewer.
    * @param aPrincipal the principal to use for the new document.

dom/chrome-webidl/BrowsingContext.webidl

@@ -131,6 +131,12 @@ interface BrowsingContext {
 
   [SetterThrows] attribute boolean isActive;
 
+  /**
+   * Sets whether this is an app tab. Non-same-origin link navigations from app
+   * tabs may be forced to open in new contexts, rather than in the same context.
+   */
+  [SetterThrows] attribute boolean isAppTab;
+
   // The inRDMPane flag indicates whether or not Responsive Design Mode is
   // active for the browsing context.
   [SetterThrows] attribute boolean inRDMPane;

toolkit/components/extensions/ExtensionPageChild.jsm

@@ -409,10 +409,6 @@ ExtensionPageChild = {
   },
 
   expectViewLoad(global, viewType) {
-    if (["popup", "sidebar"].includes(viewType)) {
-      global.docShell.isAppTab = true;
-    }
-
     promiseEvent(
       global,
       "DOMContentLoaded",

toolkit/components/extensions/ExtensionParent.jsm

@@ -512,6 +512,10 @@ GlobalManager = {
     data.viewType = browser.getAttribute("webextension-view-type");
     if (data.viewType) {
       GlobalManager.frameData.set(browser, data);
+      // Potentially unnecessary: bug 1822037 will evaluate further.
+      browser.browsingContext.isAppTab = ["popup", "sidebar"].includes(
+        data.viewType
+      );
     }
   },
 

toolkit/components/extensions/ext-browser-content.js

@@ -48,9 +48,6 @@ const BrowserListener = {
       content.windowUtils.allowScriptsToClose();
     }
 
-    // Force external links to open in tabs.
-    docShell.isAppTab = true;
-
     if (this.blockParser) {
       this.blockingPromise = new Promise(resolve => {
         this.unblockParser = resolve;

toolkit/mozapps/extensions/content/aboutaddons.js

@@ -1921,6 +1921,8 @@ class InlineOptionsBrowser extends HTMLElement {
         browserOptions.stylesheets = extensionStylesheets;
       }
 
+      // Potentially unnecessary: bug 1822037 will evaluate further.
+      browser.browsingContext.isAppTab = true;
       mm.sendAsyncMessage("Extension:InitBrowser", browserOptions);
 
       if (browser.isConnectedAndReady) {