Bug 1470540 - Improve performance of DataTransfer::CacheExternalClipboardFormats, r=baku, r=mccr8 draft
authorAnny Gakhokidze <agakhokidze@mozilla.com>
Fri, 22 Jun 2018 14:28:27 -0400
changeset 812198 bc6c5545e2c95b1acc84a75a88a6b21b9ad2c500
parent 810787 c78c5959f35bebd377341443f6648f836d136fc8
child 813136 be35afaa61eb31a8540354b86deef8d1c047eae3
child 813146 e8a396da9e33d310b2b68b00a7161c0dbc073143
child 813774 09dab68ad52c9977ae13bbffd572dd116d4cb7c0
push id114490
push userbmo:agakhokidze@mozilla.com
push dateThu, 28 Jun 2018 20:09:55 +0000
reviewersbaku, mccr8
bugs1470540
milestone62.0a1
Bug 1470540 - Improve performance of DataTransfer::CacheExternalClipboardFormats, r=baku, r=mccr8 Currently, in order to retrieve supported clipboard formats DataTransfer::CacheExternalClipboardFormats repeatedly makes the same calls to clipboard->HasDataMatchingFlavors. In the case when aPlainTextOnly == true only 1 call is made - clipboard->HasDataMatchingFlavors(kUnicodeMime, ...), and when aPlainTextOnly == false we have 1 call made for every member of the list { kCustomTypesMime, kFileMime, kHTMLMime, kRTFMime, kURLMime, kURLDataMime, kUnicodeMime, kPNGImageMime } - a total of 8 calls. We can see that in nsClipboardProxy::HasDataMatchingFlavors, there is a call to ContentChild::GetSingleton()->SendClipboardHasType. So when aPlainTextOnly == true, we will have 1 sync message, and when aPlainTextOnly == false, we will have 8 sync messages. With the proposed solution, in DataTransfer::CacheExternalClipboardFormats we will only have 1 sync message regardless of the case because GetExternalClipboardFormats() will retrieve all supported clipboard formats at once. MozReview-Commit-ID: CAmBfqB459v
dom/events/DataTransfer.cpp
dom/events/DataTransfer.h
dom/ipc/ContentParent.cpp
dom/ipc/ContentParent.h
dom/ipc/PContent.ipdl
ipc/ipdl/sync-messages.ini
--- a/dom/events/DataTransfer.cpp
+++ b/dom/events/DataTransfer.cpp
@@ -604,16 +604,58 @@ DataTransfer::MozCloneForEvent(const nsA
   nsresult rv = Clone(mParent, eventMessage, false, false, getter_AddRefs(dt));
   if (NS_FAILED(rv)) {
     aRv.Throw(rv);
     return nullptr;
   }
   return dt.forget();
 }
 
+/* static */
+void
+DataTransfer::GetExternalClipboardFormats(const int32_t& aWhichClipboard,
+                                         const bool& aPlainTextOnly,
+                                         nsTArray<nsCString>* aResult)
+{
+  MOZ_ASSERT(aResult);
+  nsCOMPtr<nsIClipboard> clipboard =
+    do_GetService("@mozilla.org/widget/clipboard;1");
+  if (!clipboard || aWhichClipboard < 0) {
+    return;
+  }
+
+  if (aPlainTextOnly) {
+    bool hasType;
+    static const char * unicodeMime[] = { kUnicodeMime };
+    nsresult rv = clipboard->HasDataMatchingFlavors(unicodeMime,
+                                                    /* number of flavors to check */ 1,
+                                                    aWhichClipboard, &hasType);
+    NS_SUCCEEDED(rv);
+    if (hasType) {
+      aResult->AppendElement(kUnicodeMime);
+    }
+    return;
+  }
+
+  // If not plain text only, then instead check all the other types
+  static const char * formats[] = { kCustomTypesMime, kFileMime, kHTMLMime, kRTFMime,
+                                  kURLMime, kURLDataMime, kUnicodeMime, kPNGImageMime };
+
+  for (uint32_t f = 0; f < mozilla::ArrayLength(formats); ++f) {
+    bool hasType;
+    nsresult rv = clipboard->HasDataMatchingFlavors(&(formats[f]),
+                                                    /* number of flavors to check */ 1,
+                                                    aWhichClipboard, &hasType);
+    NS_SUCCEEDED(rv);
+    if (hasType) {
+      aResult->AppendElement(formats[f]);
+    }
+  }
+}
+
 nsresult
 DataTransfer::SetDataAtInternal(const nsAString& aFormat, nsIVariant* aData,
                                 uint32_t aIndex,
                                 nsIPrincipal* aSubjectPrincipal)
 {
   if (aFormat.IsEmpty()) {
     return NS_OK;
   }
@@ -1335,83 +1377,60 @@ DataTransfer::CacheExternalDragFormats()
       }
     }
   }
 }
 
 void
 DataTransfer::CacheExternalClipboardFormats(bool aPlainTextOnly)
 {
-  NS_ASSERTION(mEventMessage == ePaste,
-               "caching clipboard data for invalid event");
-
   // Called during the constructor for paste events to cache the formats
   // available on the clipboard. As with CacheExternalDragFormats, the
   // data will only be retrieved when needed.
-
-  nsCOMPtr<nsIClipboard> clipboard =
-    do_GetService("@mozilla.org/widget/clipboard;1");
-  if (!clipboard || mClipboardType < 0) {
-    return;
-  }
+  NS_ASSERTION(mEventMessage == ePaste,
+               "caching clipboard data for invalid event");
 
   nsCOMPtr<nsIPrincipal> sysPrincipal = nsContentUtils::GetSystemPrincipal();
 
+  nsTArray<nsCString> typesArray;
+
+  if (XRE_IsContentProcess()) {
+    ContentChild::GetSingleton()->SendGetExternalClipboardFormats(mClipboardType, aPlainTextOnly, &typesArray);
+  } else {
+    GetExternalClipboardFormats(mClipboardType, aPlainTextOnly, &typesArray);
+  }
+
   if (aPlainTextOnly) {
-    bool supported;
-    const char* unicodeMime[] = { kUnicodeMime };
-    clipboard->HasDataMatchingFlavors(unicodeMime, 1, mClipboardType,
-                                      &supported);
-    if (supported) {
+    // The only thing that will be in types is kUnicodeMime
+    MOZ_ASSERT(typesArray.IsEmpty() || typesArray.Length() == 1);
+    if (typesArray.Length() == 1) {
       CacheExternalData(kUnicodeMime, 0, sysPrincipal, false);
     }
     return;
   }
 
-  // Check if the clipboard has any files
   bool hasFileData = false;
-  const char *fileMime[] = { kFileMime };
-  clipboard->HasDataMatchingFlavors(fileMime, 1, mClipboardType, &hasFileData);
-
-  // We will be ignoring any application/x-moz-file files found in the paste
-  // datatransfer within e10s, as they will fail to be sent over IPC. Because of
-  // that, we will unset hasFileData, whether or not it would have been set.
-  // (bug 1308007)
-  if (XRE_IsContentProcess()) {
-    hasFileData = false;
-  }
-
-  // there isn't a way to get a list of the formats that might be available on
-  // all platforms, so just check for the types that can actually be imported.
-  // NOTE: kCustomTypesMime must have index 0, kFileMime index 1
-  const char* formats[] = { kCustomTypesMime, kFileMime, kHTMLMime, kRTFMime,
-                            kURLMime, kURLDataMime, kUnicodeMime, kPNGImageMime };
-
-  for (uint32_t f = 0; f < mozilla::ArrayLength(formats); ++f) {
-    // check each format one at a time
-    bool supported;
-    clipboard->HasDataMatchingFlavors(&(formats[f]), 1, mClipboardType,
-                                      &supported);
-    // if the format is supported, add an item to the array with null as
-    // the data. When retrieved, GetRealData will read the data.
-    if (supported) {
-      if (f == 0) {
-        FillInExternalCustomTypes(0, sysPrincipal);
-      } else {
-        // In non-e10s we support pasting files from explorer.exe.
-        // Unfortunately, we fail to send that data over IPC in e10s, so we
-        // don't want to add the item to the DataTransfer and end up producing a
-        // null `application/x-moz-file`. (bug 1308007)
-        if (XRE_IsContentProcess() && f == 1) {
-          continue;
-        }
-
-        // If we aren't the file data, and we have file data, we want to be hidden
-        CacheExternalData(formats[f], 0, sysPrincipal, /* hidden = */ f != 1 && hasFileData);
+  for (const nsCString& type: typesArray) {
+    if (type.EqualsLiteral(kCustomTypesMime)) {
+      FillInExternalCustomTypes(0, sysPrincipal);
+    } else if (type.EqualsLiteral(kFileMime) && XRE_IsContentProcess()) {
+      // We will be ignoring any application/x-moz-file files found in the paste
+      // datatransfer within e10s, as they will fail top be sent over IPC. Because of
+      // that, we will unset hasFileData, whether or not it would have been set.
+      // (bug 1308007)
+      hasFileData = false;
+      continue;
+    } else {
+      // We expect that if kFileMime is supported, then it will be the either at
+      // index 0 or at index 1 in the typesArray returned by GetExternalClipboardFormats
+      if (type.EqualsLiteral(kFileMime) && !XRE_IsContentProcess()) {
+        hasFileData = true;
       }
+      // If we aren't the file data, and we have file data, we want to be hidden
+      CacheExternalData(type.get(), 0, sysPrincipal, /* hidden = */ !type.EqualsLiteral(kFileMime) && hasFileData);
     }
   }
 }
 
 void
 DataTransfer::FillAllExternalData()
 {
   if (mIsExternal) {
--- a/dom/events/DataTransfer.h
+++ b/dom/events/DataTransfer.h
@@ -414,16 +414,25 @@ public:
   // GetTypes being added or removed or changing item kinds.
   void TypesListMayHaveChanged();
 
   // Testing method used to emulate internal DataTransfer management.
   // NOTE: Please don't use this. See the comments in the webidl for more.
   already_AddRefed<DataTransfer> MozCloneForEvent(const nsAString& aEvent,
                                                   ErrorResult& aRv);
 
+  // Retrieve a list of clipboard formats supported
+  //
+  // If kFileMime is supported, then it will be placed either at
+  // index 0 or at index 1 in aResult
+  static void
+  GetExternalClipboardFormats(const int32_t& aWhichClipboard,
+                              const bool& aPlainTextOnly,
+                              nsTArray<nsCString>* aResult);
+
 protected:
 
   // caches text and uri-list data formats that exist in the drag service or
   // clipboard for retrieval later.
   nsresult CacheExternalData(const char* aFormat, uint32_t aIndex,
                              nsIPrincipal* aPrincipal, bool aHidden);
 
   // caches the formats that exist in the drag service that were added by an
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -2696,16 +2696,26 @@ ContentParent::RecvClipboardHasType(nsTA
   clipboard->HasDataMatchingFlavors(typesChrs, aTypes.Length(),
                                     aWhichClipboard, aHasType);
 
   delete [] typesChrs;
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
+ContentParent::RecvGetExternalClipboardFormats(const int32_t& aWhichClipboard,
+                                     const bool& aPlainTextOnly,
+                                     nsTArray<nsCString>* aTypes)
+{
+  MOZ_ASSERT(aTypes);
+  DataTransfer::GetExternalClipboardFormats(aWhichClipboard, aPlainTextOnly, aTypes);
+  return IPC_OK();
+}
+
+mozilla::ipc::IPCResult
 ContentParent::RecvPlaySound(const URIParams& aURI)
 {
   nsCOMPtr<nsIURI> soundURI = DeserializeURI(aURI);
   bool isChrome = false;
   // If the check here fails, it can only mean that this message was spoofed.
   if (!soundURI || NS_FAILED(soundURI->SchemeIs("chrome", &isChrome)) || !isChrome) {
     // PlaySound only accepts a valid chrome URI.
     return IPC_FAIL_NO_REASON(this);
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -985,16 +985,20 @@ private:
                                                    IPCDataTransfer* aDataTransfer) override;
 
   virtual mozilla::ipc::IPCResult RecvEmptyClipboard(const int32_t& aWhichClipboard) override;
 
   virtual mozilla::ipc::IPCResult RecvClipboardHasType(nsTArray<nsCString>&& aTypes,
                                                        const int32_t& aWhichClipboard,
                                                        bool* aHasType) override;
 
+  virtual mozilla::ipc::IPCResult RecvGetExternalClipboardFormats(const int32_t& aWhichClipboard,
+                                                                  const bool& aPlainTextOnly,
+                                                                  nsTArray<nsCString>* aTypes) override;
+
   virtual mozilla::ipc::IPCResult RecvPlaySound(const URIParams& aURI) override;
   virtual mozilla::ipc::IPCResult RecvBeep() override;
   virtual mozilla::ipc::IPCResult RecvPlayEventSound(const uint32_t& aEventId) override;
 
   virtual mozilla::ipc::IPCResult RecvGetSystemColors(const uint32_t& colorsCount,
                                                       InfallibleTArray<uint32_t>* colors) override;
 
   virtual mozilla::ipc::IPCResult RecvGetIconForExtension(const nsCString& aFileExt,
--- a/dom/ipc/PContent.ipdl
+++ b/dom/ipc/PContent.ipdl
@@ -848,16 +848,19 @@ parent:
                        uint32_t aContentPolicyType,
                        int32_t aWhichClipboard);
 
     // Given a list of supported types, returns the clipboard data for the
     // first type that matches.
     sync GetClipboard(nsCString[] aTypes, int32_t aWhichClipboard)
         returns (IPCDataTransfer dataTransfer);
 
+    // Returns a list of formats supported by the clipboard
+    sync GetExternalClipboardFormats(int32_t aWhichClipboard, bool aPlainTextOnly) returns (nsCString[] aTypes);
+
     // Clears the clipboard.
     async EmptyClipboard(int32_t aWhichClipboard);
 
     // Returns true if data of one of the specified types is on the clipboard.
     sync ClipboardHasType(nsCString[] aTypes, int32_t aWhichClipboard)
         returns (bool hasType);
 
     // 'Play', 'Beep' and 'PlayEventSound' are the only nsISound methods used in
--- a/ipc/ipdl/sync-messages.ini
+++ b/ipc/ipdl/sync-messages.ini
@@ -864,16 +864,18 @@ description =
 [PContent::GetGfxVars]
 description =
 [PContent::ReadFontList]
 description =
 [PContent::GetClipboard]
 description =
 [PContent::ClipboardHasType]
 description =
+[PContent::GetExternalClipboardFormats]
+description = Retrieve supported clipboard formats synchronously
 [PContent::GetSystemColors]
 description =
 [PContent::GetIconForExtension]
 description =
 [PContent::GetShowPasswordSetting]
 description =
 [PContent::KeywordToURI]
 description =