Bug 1600356 - Part 3 - Streamline Touch Bar image loading. r=spohl

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

--HG--
extra : moz-landing-system : lando

browser/components/touchbar/MacTouchBar.js

@@ -313,6 +313,13 @@ class TouchBarHelper {
     return BrowserWindowTracker.getTopWindow();
   }
 
+  get document() {
+    if (!TouchBarHelper.window) {
+      return null;
+    }
+    return TouchBarHelper.window.document;
+  }
+
   get isUrlbarFocused() {
     if (!TouchBarHelper.window || !TouchBarHelper.window.gURLBar) {
       return false;
@@ -560,12 +567,6 @@ class TouchBarInput {
   set image(image) {
     this._image = image;
   }
-  // Required as context to load our input icons.
-  get document() {
-    return BrowserWindowTracker.getTopWindow()
-      ? BrowserWindowTracker.getTopWindow().document
-      : null;
-  }
   get type() {
     return this._type == "" ? "button" : this._type;
   }

widget/cocoa/nsIconLoaderService.h

@@ -35,10 +35,14 @@ class nsIconLoaderService : public imgINotificationObserver {
 
   // LoadIcon will start a load request for the icon.
   // The request may not complete until after LoadIcon returns.
-  nsresult LoadIcon(nsIURI* aIconURI);
+  // If aIsInternalIcon is true, the document and principal will not be
+  // used when loading.
+  nsresult LoadIcon(nsIURI* aIconURI, bool aIsInternalIcon);
 
   NSImage* GetNativeIconImage();
 
+  void ReleaseJSObjects() { mContent = nil; }
+
   void Destroy();
 
  protected:

widget/cocoa/nsIconLoaderService.mm

@@ -70,7 +70,7 @@ void nsIconLoaderService::Destroy() {
   mCompletionHandler = nil;
 }
 
-nsresult nsIconLoaderService::LoadIcon(nsIURI* aIconURI) {
+nsresult nsIconLoaderService::LoadIcon(nsIURI* aIconURI, bool aIsInternalIcon = false) {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT;
 
   if (mIconRequest) {
@@ -97,10 +97,17 @@ nsresult nsIconLoaderService::LoadIcon(nsIURI* aIconURI) {
     return NS_ERROR_FAILURE;
   }
 
-  nsresult rv = loader->LoadImage(
+  nsresult rv;
-      aIconURI, nullptr, nullptr, mContent->NodePrincipal(), 0, loadGroup, this, mContent, document,
+  if (aIsInternalIcon) {
-      nsIRequest::LOAD_NORMAL, nullptr, mContentType, EmptyString(),
+    rv = loader->LoadImage(aIconURI, nullptr, nullptr, nullptr, 0, loadGroup, this, nullptr,
+                           nullptr, nsIRequest::LOAD_NORMAL, nullptr, mContentType, EmptyString(),
                            /* aUseUrgentStartForChannel */ false, getter_AddRefs(mIconRequest));
+  } else {
+    rv = loader->LoadImage(aIconURI, nullptr, nullptr, mContent->NodePrincipal(), 0, loadGroup,
+                           this, mContent, document, nsIRequest::LOAD_NORMAL, nullptr, mContentType,
+                           EmptyString(),
+                           /* aUseUrgentStartForChannel */ false, getter_AddRefs(mIconRequest));
+  }
   if (NS_FAILED(rv)) {
     return rv;
   }

widget/cocoa/nsTouchBar.h

@@ -115,6 +115,11 @@
  */
 - (void)touchBarAction:(id)aSender;
 
+/**
+ * Helper function to initialize a new nsTouchBarInputIcon and load an icon.
+ */
+- (void)loadIconForInput:(TouchBarInput*)aInput forItem:(NSTouchBarItem*)aItem;
+
 - (NSArray*)itemsForSharingServicePickerTouchBarItem:
     (NSSharingServicePickerTouchBarItem*)aPickerTouchBarItem;
 

widget/cocoa/nsTouchBar.mm

@@ -309,23 +309,13 @@ static const uint32_t kInputIconSize = 16;
   }
 
   NSButton* button = (NSButton*)[aButton view];
-
   button.title = [input title];
-  if (![input isIconPositionSet]) {
-    [button setImagePosition:NSImageOnly];
-    [input setIconPositionSet:true];
-  }
-
   if ([input imageURI]) {
-    RefPtr<nsTouchBarInputIcon> icon = [input icon];
+    [button setImagePosition:NSImageOnly];
-    if (!icon) {
+    [self loadIconForInput:input forItem:aButton];
-      icon = new nsTouchBarInputIcon([input document], button);
-      [input setIcon:icon];
   }
-    icon->SetupIcon([input imageURI]);
-  }
-  [button setEnabled:![input isDisabled]];
 
+  [button setEnabled:![input isDisabled]];
   if ([input color]) {
     button.bezelColor = [input color];
   }
@@ -345,6 +335,7 @@ static const uint32_t kInputIconSize = 16;
     return;
   }
 
+  [self updateButton:aMainButton withIdentifier:aIdentifier];
   NSButton* button = (NSButton*)[aMainButton view];
 
   // If empty, string is still being localized. Display a blank input instead.
@@ -354,10 +345,6 @@ static const uint32_t kInputIconSize = 16;
     [button setImagePosition:NSImageLeft];
   }
   button.imageHugsTitle = YES;
-  [input setIconPositionSet:true];
-
-  [self updateButton:aMainButton withIdentifier:aIdentifier];
-
   [button.widthAnchor constraintGreaterThanOrEqualToConstant:MAIN_BUTTON_WIDTH].active = YES;
   [button setContentHuggingPriority:1.0 forOrientation:NSLayoutConstraintOrientationHorizontal];
 }
@@ -375,12 +362,7 @@ static const uint32_t kInputIconSize = 16;
 
   aPopoverItem.showsCloseButton = YES;
   if ([input imageURI]) {
-    RefPtr<nsTouchBarInputIcon> icon = [input icon];
+    [self loadIconForInput:input forItem:aPopoverItem];
-    if (!icon) {
-      icon = new nsTouchBarInputIcon([input document], nil, nil, aPopoverItem);
-      [input setIcon:icon];
-    }
-    icon->SetupIcon([input imageURI]);
   } else if ([input title]) {
     aPopoverItem.collapsedRepresentationLabel = [input title];
   }
@@ -487,14 +469,8 @@ static const uint32_t kInputIconSize = 16;
   // buttonImage needs to be set to nil while we wait for our icon to load.
   // Otherwise, the default Apple share icon is automatically loaded.
   servicesItem.buttonImage = nil;
-  if ([input imageURI]) {
+
-    RefPtr<nsTouchBarInputIcon> icon = [input icon];
+  [self loadIconForInput:input forItem:servicesItem];
-    if (!icon) {
-      icon = new nsTouchBarInputIcon([input document], nil, servicesItem);
-      [input setIcon:icon];
-    }
-    icon->SetupIcon([input imageURI]);
-  }
 
   servicesItem.delegate = self;
   return servicesItem;
@@ -537,6 +513,24 @@ static const uint32_t kInputIconSize = 16;
   callback->OnCommand();
 }
 
+- (void)loadIconForInput:(TouchBarInput*)aInput forItem:(NSTouchBarItem*)aItem {
+  if (!aInput || ![aInput imageURI] || !aItem) {
+    return;
+  }
+
+  RefPtr<nsTouchBarInputIcon> icon = [aInput icon];
+  if (!icon && mTouchBarHelper) {
+    RefPtr<Document> document;
+    nsresult rv = mTouchBarHelper->GetDocument(getter_AddRefs(document));
+    if (NS_FAILED(rv) || !document) {
+      return;
+    }
+    icon = new nsTouchBarInputIcon(document, aInput, aItem);
+    [aInput setIcon:icon];
+  }
+  icon->SetupIcon([aInput imageURI]);
+}
+
 - (void)releaseJSObjects {
   mTouchBarHelper = nil;
 

widget/cocoa/nsTouchBarInput.h

@@ -36,8 +36,6 @@ class nsTouchBarInputIcon;
   NSColor* mColor;
   BOOL mDisabled;
   nsCOMPtr<nsITouchBarInputCallback> mCallback;
-  RefPtr<Document> mDocument;
-  BOOL mIsIconPositionSet;
   NSMutableArray<TouchBarInput*>* mChildren;
 }
 
@@ -51,8 +49,6 @@ class nsTouchBarInputIcon;
 - (BOOL)isDisabled;
 - (NSTouchBarItemIdentifier)nativeIdentifier;
 - (nsCOMPtr<nsITouchBarInputCallback>)callback;
-- (RefPtr<Document>)document;
-- (BOOL)isIconPositionSet;
 - (NSMutableArray<TouchBarInput*>*)children;
 - (void)setKey:(NSString*)aKey;
 - (void)setTitle:(NSString*)aTitle;
@@ -62,8 +58,6 @@ class nsTouchBarInputIcon;
 - (void)setColor:(NSColor*)aColor;
 - (void)setDisabled:(BOOL)aDisabled;
 - (void)setCallback:(nsCOMPtr<nsITouchBarInputCallback>)aCallback;
-- (void)setDocument:(RefPtr<Document>)aDocument;
-- (void)setIconPositionSet:(BOOL)aIsIconPositionSet;
 - (void)setChildren:(NSMutableArray<TouchBarInput*>*)aChildren;
 
 - (id)initWithKey:(NSString*)aKey
@@ -73,7 +67,6 @@ class nsTouchBarInputIcon;
          callback:(nsCOMPtr<nsITouchBarInputCallback>)aCallback
             color:(uint32_t)aColor
          disabled:(BOOL)aDisabled
-         document:(RefPtr<Document>)aDocument
          children:(nsCOMPtr<nsIArray>)aChildren;
 
 - (TouchBarInput*)initWithXPCOM:(nsCOMPtr<nsITouchBarInput>)aInput;

widget/cocoa/nsTouchBarInput.mm

@@ -38,12 +38,6 @@
 - (nsCOMPtr<nsITouchBarInputCallback>)callback {
   return mCallback;
 }
-- (RefPtr<Document>)document {
-  return mDocument;
-}
-- (BOOL)isIconPositionSet {
-  return mIsIconPositionSet;
-}
 - (NSMutableArray<TouchBarInput*>*)children {
   return mChildren;
 }
@@ -100,18 +94,6 @@
   mCallback = aCallback;
 }
 
-- (void)setDocument:(RefPtr<Document>)aDocument {
-  if (mIcon) {
-    mIcon->Destroy();
-    mIcon = nil;
-  }
-  mDocument = aDocument;
-}
-
-- (void)setIconPositionSet:(BOOL)aIsIconPositionSet {
-  mIsIconPositionSet = aIsIconPositionSet;
-}
-
 - (void)setChildren:(NSMutableArray<TouchBarInput*>*)aChildren {
   [aChildren retain];
   for (TouchBarInput* child in mChildren) {
@@ -129,7 +111,6 @@
          callback:(nsCOMPtr<nsITouchBarInputCallback>)aCallback
             color:(uint32_t)aColor
          disabled:(BOOL)aDisabled
-         document:(RefPtr<Document>)aDocument
          children:(nsCOMPtr<nsIArray>)aChildren {
   if (self = [super init]) {
     [self setKey:aKey];
@@ -137,8 +118,6 @@
     [self setImageURI:aImageURI];
     [self setType:aType];
     [self setCallback:aCallback];
-    [self setDocument:aDocument];
-    [self setIconPositionSet:false];
     [self setDisabled:aDisabled];
     if (aColor) {
       [self setColor:[NSColor colorWithDisplayP3Red:((aColor >> 16) & 0xFF) / 255.0
@@ -210,12 +189,6 @@
     return nil;
   }
 
-  RefPtr<Document> document;
-  rv = aInput->GetDocument(getter_AddRefs(document));
-  if (NS_FAILED(rv)) {
-    return nil;
-  }
-
   nsCOMPtr<nsIArray> children;
   rv = aInput->GetChildren(getter_AddRefs(children));
   if (NS_FAILED(rv)) {
@@ -229,18 +202,16 @@
                   callback:callback
                      color:colorInt
                   disabled:(BOOL)disabled
-                  document:document
                   children:children];
 }
 
 - (void)releaseJSObjects {
   if (mIcon) {
-    mIcon->ReleaseJSObjects();
+    mIcon->Destroy();
+    mIcon = nil;
   }
-
   [self setCallback:nil];
   [self setImageURI:nil];
-  [self setDocument:nil];
   for (TouchBarInput* child in mChildren) {
     [child releaseJSObjects];
   }

widget/cocoa/nsTouchBarInputIcon.h

@@ -26,10 +26,8 @@ class imgRequestProxy;
 
 class nsTouchBarInputIcon : public nsIconLoaderObserver {
  public:
-  explicit nsTouchBarInputIcon(
+  explicit nsTouchBarInputIcon(RefPtr<Document> aDocument,
-      RefPtr<Document> aDocument, NSButton* aButton,
+                               TouchBarInput* aInput, NSTouchBarItem* aItem);
-      NSSharingServicePickerTouchBarItem* aShareScrubber = nil,
-      NSPopoverTouchBarItem* aPopoverItem = nil);
 
  private:
   virtual ~nsTouchBarInputIcon();
@@ -49,7 +47,7 @@ class nsTouchBarInputIcon : public nsIconLoaderObserver {
   // this from happening.
   void Destroy();
 
-  void ReleaseJSObjects() { mDocument = nil; }
+  void ReleaseJSObjects();
 
  protected:
   RefPtr<Document> mDocument;

widget/cocoa/nsTouchBarInputIcon.mm

@@ -22,14 +22,20 @@ using namespace mozilla;
 static const uint32_t kIconSize = 16;
 static const CGFloat kHiDPIScalingFactor = 2.0f;
 
-nsTouchBarInputIcon::nsTouchBarInputIcon(RefPtr<Document> aDocument, NSButton* aButton,
+nsTouchBarInputIcon::nsTouchBarInputIcon(RefPtr<Document> aDocument, TouchBarInput* aInput,
-                                         NSSharingServicePickerTouchBarItem* aShareScrubber,
+                                         NSTouchBarItem* aItem)
-                                         NSPopoverTouchBarItem* aPopoverItem)
+    : mDocument(aDocument), mSetIcon(false), mButton(nil), mShareScrubber(nil), mPopoverItem(nil) {
-    : mDocument(aDocument),
+  if ([[aInput nativeIdentifier] isEqualToString:ShareScrubberIdentifier]) {
-      mSetIcon(false),
+    mShareScrubber = (NSSharingServicePickerTouchBarItem*)aItem;
-      mButton(aButton),
+  } else if ([aInput baseType] == TouchBarInputBaseType::kPopover) {
-      mShareScrubber(aShareScrubber),
+    mPopoverItem = (NSPopoverTouchBarItem*)aItem;
-      mPopoverItem(aPopoverItem) {
+  } else if ([aInput baseType] == TouchBarInputBaseType::kButton ||
+             [aInput baseType] == TouchBarInputBaseType::kMainButton) {
+    mButton = (NSButton*)[aItem view];
+  } else {
+    NS_ERROR("Incompatible Touch Bar input passed to nsTouchBarInputIcon.");
+  }
+  aInput = nil;
   MOZ_COUNT_CTOR(nsTouchBarInputIcon);
 }
 
@@ -42,6 +48,7 @@ nsTouchBarInputIcon::~nsTouchBarInputIcon() {
 // (as might otherwise happen if calls to our imgINotificationObserver methods
 // are still outstanding).  nsTouchBar owns our mTouchBarInput.
 void nsTouchBarInputIcon::Destroy() {
+  ReleaseJSObjects();
   if (mIconLoader) {
     mIconLoader->Destroy();
     mIconLoader = nullptr;
@@ -62,7 +69,7 @@ nsresult nsTouchBarInputIcon::SetupIcon(nsCOMPtr<nsIURI> aIconURI) {
   }
 
   if (!(mButton || mShareScrubber || mPopoverItem)) {
-    NS_ERROR("No Touch Bar button");
+    NS_ERROR("No Touch Bar input provided.");
     return NS_ERROR_FAILURE;
   }
 
@@ -83,7 +90,7 @@ nsresult nsTouchBarInputIcon::SetupIcon(nsCOMPtr<nsIURI> aIconURI) {
     [mPopoverItem setCollapsedRepresentationImage:mIconLoader->GetNativeIconImage()];
   }
 
-  nsresult rv = mIconLoader->LoadIcon(aIconURI);
+  nsresult rv = mIconLoader->LoadIcon(aIconURI, true /* aIsInternalIcon */);
   if (NS_FAILED(rv)) {
     // There is no icon for this menu item, as an error occurred while loading it.
     // An icon might have been set earlier or the place holder icon may have
@@ -100,6 +107,13 @@ nsresult nsTouchBarInputIcon::SetupIcon(nsCOMPtr<nsIURI> aIconURI) {
   NS_OBJC_END_TRY_ABORT_BLOCK_NSRESULT;
 }
 
+void nsTouchBarInputIcon::ReleaseJSObjects() {
+  if (mIconLoader) {
+    mIconLoader->ReleaseJSObjects();
+  }
+  mDocument = nil;
+}
+
 //
 // nsIconLoaderObserver
 //

widget/nsITouchBarHelper.idl

@@ -6,6 +6,8 @@
 #include "nsISupports.idl"
 #include "nsITouchBarInput.idl"
 
+webidl Document;
+
 /**
  * Back-to-frontend communication for the Touch Bar
  */
@@ -33,6 +35,12 @@ interface nsITouchBarHelper : nsISupports
    */
   attribute nsIArray allItems;
 
+  /**
+   * The context in which this nsITouchBarHelper exists. Required to create
+   * an imgLoader to load our SVG icons.
+   */
+  readonly attribute Document document;
+
   /**
    * Returns the requested TouchBarInput.
    * Exposed for testing.

widget/nsITouchBarInput.idl

@@ -6,8 +6,6 @@
 #include "nsISupports.idl"
 #include "nsIURI.idl"
 
-webidl Document;
-
 [scriptable, function, uuid(001ab07c-1b3a-4dbf-a657-fada0065ff55)]
 interface nsITouchBarInputCallback : nsISupports
 {
@@ -72,12 +70,6 @@ interface nsITouchBarInput : nsISupports
    */
   attribute boolean disabled;
 
-  /**
-   * The context in which this nsITouchBarInput exists. Required to create
-   * an imgLoader to load our SVG icons.
-   */
-  readonly attribute Document document;
-
   /**
    * An array containing an input's children.
    * Available for  type = ("scrollView" || "popover").