Bug 1810850 - Part 2: Move clipboard cache code for GetData to nsBaseClipboard; r=cmartin,mstange

This patch introduce a preference for getting the data from cache
directly which is enabled on Mac only. And now we really support getting
data from cache for each type simultaneously, instead of only using the
one that has the latest cached transferable.

Depends on D178777

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

modules/libpref/init/StaticPrefList.yaml

@@ -15670,6 +15670,16 @@
   value: true
   mirror: once
 
+# Whether the clipboard cached are used while getting system clipboard data.
+- name: widget.clipboard.use-cached-data.enabled
+  type: bool
+#if defined(XP_MACOSX)
+  value: true
+#else
+  value: false
+#endif
+  mirror: always
+
 #---------------------------------------------------------------------------
 # Prefs starting with "zoom."
 #---------------------------------------------------------------------------

widget/cocoa/nsClipboard.mm

@@ -323,8 +323,10 @@ NS_IMETHODIMP
 nsClipboard::GetNativeClipboardData(nsITransferable* aTransferable, int32_t aWhichClipboard) {
   NS_OBJC_BEGIN_TRY_BLOCK_RETURN;
 
-  if ((aWhichClipboard != kGlobalClipboard && aWhichClipboard != kFindClipboard) || !aTransferable)
-    return NS_ERROR_FAILURE;
+  MOZ_DIAGNOSTIC_ASSERT(aTransferable);
+  // XXX we only support the set operation on kSelectionCache type, see bug 1835059.
+  MOZ_DIAGNOSTIC_ASSERT(kSelectionCache != aWhichClipboard);
+  MOZ_DIAGNOSTIC_ASSERT(nsIClipboard::IsClipboardTypeSupported(aWhichClipboard));
 
   NSPasteboard* cocoaPasteboard = GetPasteboard(aWhichClipboard);
   if (!cocoaPasteboard) {
@@ -335,35 +337,10 @@ nsClipboard::GetNativeClipboardData(nsITransferable* aTransferable, int32_t aWhi
   // conversion)
   nsTArray<nsCString> flavors;
   nsresult rv = aTransferable->FlavorsTransferableCanImport(flavors);
-  if (NS_FAILED(rv)) return NS_ERROR_FAILURE;
-
-  // If we were the last ones to put something on the pasteboard, then just use the cached
-  // transferable. Otherwise clear it because it isn't relevant any more.
-  if (mCachedClipboard == aWhichClipboard) {
-    const auto& clipboardCache = mCaches[aWhichClipboard];
-    MOZ_ASSERT(clipboardCache);
-    if (mChangeCount == [cocoaPasteboard changeCount]) {
-      if (nsITransferable* cachedTrans = clipboardCache->GetTransferable()) {
-        for (uint32_t i = 0; i < flavors.Length(); i++) {
-          nsCString& flavorStr = flavors[i];
-
-          nsCOMPtr<nsISupports> dataSupports;
-          rv = cachedTrans->GetTransferData(flavorStr.get(), getter_AddRefs(dataSupports));
-          if (NS_SUCCEEDED(rv)) {
-            aTransferable->SetTransferData(flavorStr.get(), dataSupports);
-            return NS_OK;  // maybe try to fill in more types? Is there a point?
-          }
-        }
-      }
-    } else {
-      // Remove transferable cache only. Don't clear system clipboard.
-      clipboardCache->Clear();
-    }
+  if (NS_FAILED(rv)) {
+    return NS_ERROR_FAILURE;
   }
 
-  // at this point we can't satisfy the request from cache data so let's look
-  // for things other people put on the system clipboard
-
   return nsClipboard::TransferableFromPasteboard(aTransferable, cocoaPasteboard);
 
   NS_OBJC_END_TRY_BLOCK_RETURN(NS_ERROR_FAILURE);

widget/nsBaseClipboard.cpp

@@ -5,6 +5,7 @@
 
 #include "nsBaseClipboard.h"
 
+#include "mozilla/StaticPrefs_widget.h"
 #include "nsIClipboardOwner.h"
 #include "nsError.h"
 #include "nsXPCOM.h"
@@ -204,25 +205,60 @@ NS_IMETHODIMP nsBaseClipboard::SetData(nsITransferable* aTransferable,
 }
 
 /**
- * Gets the transferable object
- *
+ * Gets the transferable object from system clipboard.
  */
 NS_IMETHODIMP nsBaseClipboard::GetData(nsITransferable* aTransferable,
                                        int32_t aWhichClipboard) {
-  NS_ASSERTION(aTransferable, "clipboard given a null transferable");
+  CLIPBOARD_LOG("%s: clipboard=%d", __FUNCTION__, aWhichClipboard);
 
-  CLIPBOARD_LOG("%s", __FUNCTION__);
-
-  if (!nsIClipboard::IsClipboardTypeSupported(kSelectionClipboard) &&
-      !nsIClipboard::IsClipboardTypeSupported(kFindClipboard) &&
-      aWhichClipboard != kGlobalClipboard) {
+  if (!aTransferable) {
+    NS_ASSERTION(false, "clipboard given a null transferable");
     return NS_ERROR_FAILURE;
   }
 
-  if (aTransferable)
-    return GetNativeClipboardData(aTransferable, aWhichClipboard);
+  // XXX as of now, we only support the set operation on kSelectionCache type,
+  // should we also support the get operation? See also bug 1835059.
+  if (kSelectionCache == aWhichClipboard ||
+      !nsIClipboard::IsClipboardTypeSupported(aWhichClipboard)) {
+    CLIPBOARD_LOG("%s: clipboard %d is not supported.", __FUNCTION__,
+                  aWhichClipboard);
+    return NS_ERROR_FAILURE;
+  }
 
-  return NS_ERROR_FAILURE;
+  if (mozilla::StaticPrefs::widget_clipboard_use_cached_data_enabled()) {
+    // If we were the last ones to put something on the navtive clipboard, then
+    // just use the cached transferable. Otherwise clear it because it isn't
+    // relevant any more.
+    if (auto* clipboardCache = GetClipboardCacheIfValid(aWhichClipboard)) {
+      MOZ_ASSERT(clipboardCache->GetTransferable());
+
+      // get flavor list that includes all acceptable flavors (including ones
+      // obtained through conversion)
+      nsTArray<nsCString> flavors;
+      nsresult rv = aTransferable->FlavorsTransferableCanImport(flavors);
+      if (NS_FAILED(rv)) {
+        return NS_ERROR_FAILURE;
+      }
+
+      for (const auto& flavor : flavors) {
+        nsCOMPtr<nsISupports> dataSupports;
+        rv = clipboardCache->GetTransferable()->GetTransferData(
+            flavor.get(), getter_AddRefs(dataSupports));
+        if (NS_SUCCEEDED(rv)) {
+          CLIPBOARD_LOG("%s: getting %s from cache.", __FUNCTION__,
+                        flavor.get());
+          aTransferable->SetTransferData(flavor.get(), dataSupports);
+          // maybe try to fill in more types? Is there a point?
+          return NS_OK;
+        }
+      }
+    }
+
+    // at this point we can't satisfy the request from cache data so let's look
+    // for things other people put on the system clipboard
+  }
+
+  return GetNativeClipboardData(aTransferable, aWhichClipboard);
 }
 
 RefPtr<GenericPromise> nsBaseClipboard::AsyncGetData(
@@ -301,6 +337,32 @@ nsBaseClipboard::IsClipboardTypeSupported(int32_t aWhichClipboard,
   }
 }
 
+nsBaseClipboard::ClipboardCache* nsBaseClipboard::GetClipboardCacheIfValid(
+    int32_t aClipboardType) {
+  MOZ_ASSERT(nsIClipboard::IsClipboardTypeSupported(aClipboardType));
+
+  const mozilla::UniquePtr<ClipboardCache>& cache = mCaches[aClipboardType];
+  MOZ_ASSERT(cache);
+
+  if (!cache->GetTransferable()) {
+    MOZ_ASSERT(cache->GetSequenceNumber() == -1);
+    return nullptr;
+  }
+
+  auto changeCountOrError = GetNativeClipboardSequenceNumber(aClipboardType);
+  if (changeCountOrError.isErr()) {
+    return nullptr;
+  }
+
+  if (changeCountOrError.unwrap() != cache->GetSequenceNumber()) {
+    // Clipboard cache is invalid, clear it.
+    cache->Clear();
+    return nullptr;
+  }
+
+  return cache.get();
+}
+
 void nsBaseClipboard::ClipboardCache::Clear() {
   if (mClipboardOwner) {
     mClipboardOwner->LosingOwnership(mTransferable);

widget/nsBaseClipboard.h

@@ -106,7 +106,7 @@ class nsBaseClipboard : public ClipboardSetDataHelper {
   NS_IMETHOD SetData(nsITransferable* aTransferable, nsIClipboardOwner* anOwner,
                      int32_t aWhichClipboard) override final;
   NS_IMETHOD GetData(nsITransferable* aTransferable,
-                     int32_t aWhichClipboard) override;
+                     int32_t aWhichClipboard) override final;
   NS_IMETHOD EmptyClipboard(int32_t aWhichClipboard) override;
   NS_IMETHOD HasDataMatchingFlavors(const nsTArray<nsCString>& aFlavorList,
                                     int32_t aWhichClipboard,
@@ -114,7 +114,7 @@ class nsBaseClipboard : public ClipboardSetDataHelper {
   NS_IMETHOD IsClipboardTypeSupported(int32_t aWhichClipboard,
                                       bool* aRetval) override final;
   RefPtr<mozilla::GenericPromise> AsyncGetData(
-      nsITransferable* aTransferable, int32_t aWhichClipboard) override;
+      nsITransferable* aTransferable, int32_t aWhichClipboard) override final;
   RefPtr<DataFlavorsPromise> AsyncHasDataMatchingFlavors(
       const nsTArray<nsCString>& aFlavorList, int32_t aWhichClipboard) override;
 
@@ -149,6 +149,7 @@ class nsBaseClipboard : public ClipboardSetDataHelper {
     }
     nsITransferable* GetTransferable() const { return mTransferable; }
     nsIClipboardOwner* GetClipboardOwner() const { return mClipboardOwner; }
+    int32_t GetSequenceNumber() const { return mSequenceNumber; }
 
    private:
     nsCOMPtr<nsITransferable> mTransferable;
@@ -160,6 +161,10 @@ class nsBaseClipboard : public ClipboardSetDataHelper {
   bool mEmptyingForSetData = false;
 
  private:
+  // Return clipboard cache if the cached data is valid, otherwise clear the
+  // cached data and returns null.
+  ClipboardCache* GetClipboardCacheIfValid(int32_t aClipboardType);
+
   const mozilla::dom::ClipboardCapabilities mClipboardCaps;
   bool mIgnoreEmptyNotification = false;
 };

widget/tests/clipboard_helper.js

@@ -24,6 +24,32 @@ function generateRandomString() {
   return "random number: " + Math.random();
 }
 
+function generateNewTransferable(aFlavor, aStr) {
+  let trans = Cc["@mozilla.org/widget/transferable;1"].createInstance(
+    Ci.nsITransferable
+  );
+  trans.init(null);
+  trans.addDataFlavor(aFlavor);
+
+  let supportsStr = Cc["@mozilla.org/supports-string;1"].createInstance(
+    Ci.nsISupportsString
+  );
+  supportsStr.data = aStr;
+  trans.setTransferData(aFlavor, supportsStr);
+
+  return trans;
+}
+
+function addStringToTransferable(aFlavor, aStr, aTrans) {
+  aTrans.addDataFlavor(aFlavor);
+
+  let supportsStr = Cc["@mozilla.org/supports-string;1"].createInstance(
+    Ci.nsISupportsString
+  );
+  supportsStr.data = aStr;
+  aTrans.setTransferData(aFlavor, supportsStr);
+}
+
 function writeStringToClipboard(
   aStr,
   aFlavor,
@@ -77,7 +103,12 @@ function getClipboardData(aFlavor, aClipboardType) {
   trans.addDataFlavor(aFlavor);
   clipboard.getData(trans, aClipboardType);
 
-  var data = {};
-  trans.getTransferData(aFlavor, data);
-  return data.value.QueryInterface(SpecialPowers.Ci.nsISupportsString).data;
+  try {
+    var data = {};
+    trans.getTransferData(aFlavor, data);
+    return data.value.QueryInterface(SpecialPowers.Ci.nsISupportsString).data;
+  } catch (ex) {
+    // If the clipboard is empty getTransferData will throw.
+    return null;
+  }
 }

widget/tests/test_clipboard_cache.xhtml

@@ -20,10 +20,69 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1812543
 <script class="testbody" type="application/javascript">
 <![CDATA[
 
-let supportOtherClipboardTypes = false;
+const isSupportGetFromCachedTransferable =
+  Services.prefs.getBoolPref("widget.clipboard.use-cached-data.enabled");
+
+function testClipboardCache(aClipboardType, aAsync) {
+  add_task(function test_clipboard_get() {
+    const string = generateRandomString();
+    const trans = generateNewTransferable("text/plain", string);
+
+    info(`Write text/plain data to clipboard ${aClipboardType}`);
+    if (aAsync) {
+      let request = clipboard.asyncSetData(aClipboardType);
+      request.setData(trans, null);
+    } else {
+      clipboard.setData(trans, null, aClipboardType);
+    }
+    is(getClipboardData("text/plain", aClipboardType), string,
+       `Check text/plain data on clipboard ${aClipboardType}`);
+
+    info(`Add text/foo data to transerable`);
+    addStringToTransferable("text/foo", string, trans);
+    is(getClipboardData("text/foo", aClipboardType),
+       isSupportGetFromCachedTransferable ? string : null,
+       `Check text/foo data on clipboard ${aClipboardType}`);
+
+    info(`Should not get the data from other clipboard type`);
+    clipboardTypes.forEach(function(otherType) {
+      if (otherType != clipboard.kSelectionCache &&
+          otherType != aClipboardType &&
+          clipboard.isClipboardTypeSupported(otherType)) {
+        is(getClipboardData("text/plain", otherType), null,
+           `Check text/plain data on clipboard ${otherType}`);
+        is(getClipboardData("text/foo", otherType), null,
+           `Check text/foo data on clipboard ${otherType}`);
+
+        info(`Write text/plain data to clipboard ${otherType}`);
+        writeRandomStringToClipboard("text/plain", otherType);
+      }
+    });
+
+    info(`Check data on clipboard ${aClipboardType} again`);
+    is(getClipboardData("text/plain", aClipboardType), string,
+       `Check text/plain data on clipboard ${aClipboardType} again`);
+    is(getClipboardData("text/foo", aClipboardType),
+       isSupportGetFromCachedTransferable ? string : null,
+       `Check text/foo data on clipboard ${aClipboardType} again`);
+
+    info(`Clean all clipboard data`);
+    cleanupAllClipboard();
+  });
+}
+
+add_setup(function init() {
+  cleanupAllClipboard();
+});
+
 clipboardTypes.forEach(function(type) {
-  if (clipboard.kGlobalClipboard != type && clipboard.isClipboardTypeSupported(type)) {
-    supportOtherClipboardTypes = true;
+  // Selection cache clipboard, which is available only on Cocoa widget, supports
+  // set operation only, see bug 1835059.
+  if (clipboard.kSelectionCache == type || !clipboard.isClipboardTypeSupported(type)) {
+    return;
+  }
+
+  if (clipboard.kGlobalClipboard != type) {
     add_task(function test_clipboard_hasDataMatchingFlavors() {
       info(`Test write data to clipboard type ${type}`);
 
@@ -45,11 +104,13 @@ clipboardTypes.forEach(function(type) {
       cleanupAllClipboard();
     });
   }
-});
 
-if (!supportOtherClipboardTypes) {
-  ok(true, "Don't support other clipboard types, skip tests");
-}
+  // Test sync set clipboard data.
+  testClipboardCache(type, false);
+
+  // Test async set clipboard data.
+  testClipboardCache(type, true);
+});
 
 ]]>
 </script>

widget/windows/nsClipboard.cpp

@@ -1318,16 +1318,14 @@ bool nsClipboard ::IsInternetShortcut(const nsAString& inFileName) {
 NS_IMETHODIMP
 nsClipboard::GetNativeClipboardData(nsITransferable* aTransferable,
                                     int32_t aWhichClipboard) {
+  MOZ_DIAGNOSTIC_ASSERT(aTransferable);
+  MOZ_DIAGNOSTIC_ASSERT(
+      nsIClipboard::IsClipboardTypeSupported(aWhichClipboard));
+
   MOZ_LOG(gWin32ClipboardLog, LogLevel::Debug,
           ("%s aWhichClipboard=%i", __FUNCTION__, aWhichClipboard));
 
-  // make sure we have a good transferable
-  if (!aTransferable || aWhichClipboard != kGlobalClipboard) {
-    return NS_ERROR_FAILURE;
-  }
-
   nsresult res;
-
   // This makes sure we can use the OLE functionality for the clipboard
   IDataObject* dataObj;
   if (S_OK == RepeatedlyTryOleGetClipboard(&dataObj)) {