Bug 1424661 - refactor ncClipboard::GetData(), allocate all memory by moz_xmalloc() and release by free(), r?jhorak draft
authorMartin Stransky <stransky@redhat.com>
Mon, 11 Dec 2017 11:59:57 +0100
changeset 711694 95ab30ba347836c7ce3eb8c7ce1bfb8b43e6f616
parent 711692 8062887ff0d9382ea84177f2c21f62dc0e613d9e
child 743839 85a5be0f994c1f0b79432fc5a1f76658865ced98
push id93103
push userstransky@redhat.com
push dateThu, 14 Dec 2017 11:37:57 +0000
reviewersjhorak
bugs1424661
milestone59.0a1
Bug 1424661 - refactor ncClipboard::GetData(), allocate all memory by moz_xmalloc() and release by free(), r?jhorak Refactor ncClipboard::GetData() for better readability, add nsClipboard::SetTransferableData() to send clipboard data to nsITransferable. According to Gtk people [1] we can't mix free()/g_free() and malloc()/g_malloc() calls. Existing nsClipboard code mixes that on some places which can lead to issued on glib built with specific flags (ENABLE_MEM_PROFILE or ENABLE_MEM_CHECK). [1] https://mail.gnome.org/archives/gtk-list/2000-July/msg00002.html MozReview-Commit-ID: GvkUGSttVGO
widget/gtk/nsClipboard.cpp
widget/gtk/nsClipboard.h
widget/gtk/nsClipboardX11.cpp
widget/gtk/nsClipboardX11.h
--- a/widget/gtk/nsClipboard.cpp
+++ b/widget/gtk/nsClipboard.cpp
@@ -41,23 +41,23 @@ clipboard_get_cb(GtkClipboard *aGtkClipb
                  gpointer user_data);
 
 // Callback when someone asks us to clear a clipboard
 void
 clipboard_clear_cb(GtkClipboard *aGtkClipboard,
                    gpointer user_data);
                    
 static void
-ConvertHTMLtoUCS2          (guchar             *data,
+ConvertHTMLtoUCS2          (const char*         data,
                             int32_t             dataLength,
                             char16_t         **unicodeData,
                             int32_t            &outUnicodeLen);
 
 static void
-GetHTMLCharset             (guchar * data, int32_t dataLength, nsCString& str);
+GetHTMLCharset             (const char* data, int32_t dataLength, nsCString& str);
 
 GdkAtom
 GetSelectionAtom(int32_t aWhichClipboard)
 {
     if (aWhichClipboard == nsIClipboard::kGlobalClipboard)
         return GDK_SELECTION_CLIPBOARD;
 
     return GDK_SELECTION_PRIMARY;
@@ -218,27 +218,37 @@ nsClipboard::SetData(nsITransferable *aT
     }
 
     gtk_target_table_free(gtkTargets, numTargets);
     gtk_target_list_unref(list);
   
     return rv;
 }
 
+void
+nsClipboard::SetTransferableData(nsITransferable* aTransferable,
+                                 nsCString&       aFlavor,
+                                 const char*      aClipboardData,
+                                 uint32_t         aClipboardDataLength)
+{
+  nsCOMPtr<nsISupports> wrapper;
+  nsPrimitiveHelpers::CreatePrimitiveForData(aFlavor,
+                                             aClipboardData,
+                                             aClipboardDataLength,
+                                             getter_AddRefs(wrapper));
+  aTransferable->SetTransferData(aFlavor.get(),
+                                 wrapper, aClipboardDataLength);
+}
+
 NS_IMETHODIMP
 nsClipboard::GetData(nsITransferable *aTransferable, int32_t aWhichClipboard)
 {
     if (!aTransferable)
         return NS_ERROR_FAILURE;
 
-    guchar        *data = nullptr;
-    uint32_t       length = 0;
-    bool           foundData = false;
-    nsAutoCString  foundFlavor;
-
     // Get a list of flavors this transferable can import
     nsCOMPtr<nsIArray> flavors;
     nsresult rv;
     rv = aTransferable->FlavorsTransferableCanImport(getter_AddRefs(flavors));
     if (!flavors || NS_FAILED(rv))
         return NS_ERROR_FAILURE;
 
     uint32_t count;
@@ -247,103 +257,101 @@ nsClipboard::GetData(nsITransferable *aT
         nsCOMPtr<nsISupportsCString> currentFlavor;
         currentFlavor = do_QueryElementAt(flavors, i);
         if (!currentFlavor)
             continue;
 
         nsCString flavorStr;
         currentFlavor->ToString(getter_Copies(flavorStr));
 
-        // Special case text/unicode since we can convert any
-        // string into text/unicode
-        if (flavorStr.EqualsLiteral(kUnicodeMime)) {
-            guchar* rawData =
-                mContext->WaitForClipboardContext(GTK_DEFAULT_MIME_TEXT,
-                                                  aWhichClipboard,
-                                                  &length);
-            if (!rawData) {
-                // If the type was text/unicode and we couldn't get
-                // text off the clipboard, run the next loop
-                // iteration.
-                continue;
-            }
-
-            // Convert utf-8 into our unicode format.
-            NS_ConvertUTF8toUTF16 ucs2string((const char *)rawData, length);
-            data = (guchar *)ToNewUnicode(ucs2string);
-            length = ucs2string.Length() * 2;
-            g_free(rawData);
-
-            foundData = true;
-            foundFlavor = kUnicodeMime;
-            break;
-        }
-
         if (flavorStr.EqualsLiteral(kJPEGImageMime) ||
             flavorStr.EqualsLiteral(kJPGImageMime) ||
             flavorStr.EqualsLiteral(kPNGImageMime) ||
             flavorStr.EqualsLiteral(kGIFImageMime)) {
             // Emulate support for image/jpg
             if (flavorStr.EqualsLiteral(kJPGImageMime)) {
                 flavorStr.Assign(kJPEGImageMime);
             }
 
-            data = mContext->WaitForClipboardContext(flavorStr.get(),
-                aWhichClipboard, &length);
-            if (!data)
+            uint32_t    clipboardDataLength;
+            const char* clipboardData =
+                mContext->WaitForClipboardContext(flavorStr.get(),
+                                                  aWhichClipboard,
+                                                  &clipboardDataLength);
+            if (!clipboardData)
                 continue;
 
             nsCOMPtr<nsIInputStream> byteStream;
             NS_NewByteInputStream(getter_AddRefs(byteStream),
-                                  (const char*)data,
-                                  length,
+                                  clipboardData,
+                                  clipboardDataLength,
                                   NS_ASSIGNMENT_COPY);
-            aTransferable->SetTransferData(flavorStr.get(), byteStream, sizeof(nsIInputStream*));
-            g_free(data);
+            aTransferable->SetTransferData(flavorStr.get(), byteStream,
+                                           sizeof(nsIInputStream*));
+            free((void *)clipboardData);
             return NS_OK;
         }
 
-        data = mContext->WaitForClipboardContext(flavorStr.get(),
-            aWhichClipboard, &length);
-        if (!data)
-            continue;
+        // Special case text/unicode since we can convert any
+        // string into text/unicode
+        if (flavorStr.EqualsLiteral(kUnicodeMime)) {
+            uint32_t    clipboardDataLength;
+            const char* rawData =
+                mContext->WaitForClipboardContext(GTK_DEFAULT_MIME_TEXT,
+                                                  aWhichClipboard,
+                                                  &clipboardDataLength);
+            if (!rawData) {
+                // If the type was text/unicode and we couldn't get
+                // text off the clipboard, run the next loop
+                // iteration.
+                continue;
+            }
 
-        // Special case text/html since we can convert into UCS2
-        if (flavorStr.EqualsLiteral(kHTMLMime)) {
-            char16_t* htmlBody = nullptr;
-            int32_t htmlBodyLen = 0;
-            // Convert text/html into our unicode format
-            ConvertHTMLtoUCS2(data, length,
-                              &htmlBody, htmlBodyLen);
-            g_free(data);
-
-            // Try next data format?
-            if (!htmlBodyLen)
-                continue;
-            data = (guchar *)htmlBody;
-            length = htmlBodyLen * 2;
+            // Convert utf-8 into our unicode format.
+            NS_ConvertUTF8toUTF16 ucs2string(rawData, clipboardDataLength);
+            const char* clipboardData = (const char *)ToNewUnicode(ucs2string);
+            clipboardDataLength = ucs2string.Length() * 2;
+            SetTransferableData(aTransferable, flavorStr,
+                                clipboardData, clipboardDataLength);
+            free((void *)rawData);
+            free((void *)clipboardData);
+            return NS_OK;
         }
 
-        foundData = true;
-        foundFlavor = flavorStr;
-        break;
-    }
+
+        uint32_t clipboardDataLength;
+        const char* clipboardData = mContext->WaitForClipboardContext(
+            flavorStr.get(), aWhichClipboard, &clipboardDataLength);
+
+        if (clipboardData) {
+            // Special case text/html since we can convert into UCS2
+            if (flavorStr.EqualsLiteral(kHTMLMime)) {
+                char16_t* htmlBody = nullptr;
+                int32_t htmlBodyLen = 0;
+                // Convert text/html into our unicode format
+                ConvertHTMLtoUCS2(clipboardData, clipboardDataLength,
+                                  &htmlBody, htmlBodyLen);
+                free((void *)clipboardData);
 
-    if (foundData) {
-        nsCOMPtr<nsISupports> wrapper;
-        nsPrimitiveHelpers::CreatePrimitiveForData(foundFlavor,
-                                                   data, length,
-                                                   getter_AddRefs(wrapper));
-        aTransferable->SetTransferData(foundFlavor.get(),
-                                       wrapper, length);
+                // Try next data format?
+                if (!htmlBodyLen)
+                    continue;
+
+                SetTransferableData(aTransferable, flavorStr,
+                                    (const char*)htmlBody, htmlBodyLen * 2);
+                free(htmlBody);
+            } else {
+                SetTransferableData(aTransferable, flavorStr,
+                                    clipboardData, clipboardDataLength);
+                free((void *)clipboardData);
+            }
+            return NS_OK;
+        }
     }
 
-    if (data)
-        g_free(data);
-
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsClipboard::EmptyClipboard(int32_t aWhichClipboard)
 {
     if (aWhichClipboard == kSelectionClipboard) {
         if (mSelectionOwner) {
@@ -634,26 +642,26 @@ clipboard_clear_cb(GtkClipboard *aGtkCli
  *  3. From other app who use "text/html" when copy-paste
  *     "text/html", has "charset" info
  *
  * data      : got from GTK clipboard
  * dataLength: got from GTK clipboard
  * body      : pass to Mozilla
  * bodyLength: pass to Mozilla
  */
-void ConvertHTMLtoUCS2(guchar * data, int32_t dataLength,
+void ConvertHTMLtoUCS2(const char* data, int32_t dataLength,
                        char16_t** unicodeData, int32_t& outUnicodeLen)
 {
     nsAutoCString charset;
     GetHTMLCharset(data, dataLength, charset);// get charset of HTML
     if (charset.EqualsLiteral("UTF-16")) {//current mozilla
         outUnicodeLen = (dataLength / 2) - 1;
-        *unicodeData = reinterpret_cast<char16_t*>
-                                       (g_malloc((outUnicodeLen + sizeof('\0')) *
-                       sizeof(char16_t)));
+        *unicodeData =
+            reinterpret_cast<char16_t*>
+            (moz_xmalloc((outUnicodeLen + sizeof('\0')) * sizeof(char16_t)));
         if (*unicodeData) {
             memcpy(*unicodeData, data + sizeof(char16_t),
                    outUnicodeLen * sizeof(char16_t));
             (*unicodeData)[outUnicodeLen] = '\0';
         }
     } else if (charset.EqualsLiteral("UNKNOWN")) {
         outUnicodeLen = 0;
         return;
@@ -673,17 +681,17 @@ void ConvertHTMLtoUCS2(guchar * data, in
         if (!needed.isValid() || needed.value() > INT32_MAX) {
           outUnicodeLen = 0;
           return;
         }
 
         outUnicodeLen = 0;
         if (needed.value()) {
           *unicodeData = reinterpret_cast<char16_t*>(
-            g_malloc((needed.value() + 1) * sizeof(char16_t)));
+            moz_xmalloc((needed.value() + 1) * sizeof(char16_t)));
           if (*unicodeData) {
             uint32_t result;
             size_t read;
             size_t written;
             bool hadErrors;
             Tie(result, read, written, hadErrors) =
               decoder->DecodeToUTF16(AsBytes(MakeSpan(data, dataLength)),
                                      MakeSpan(*unicodeData, needed.value()),
@@ -706,26 +714,26 @@ void ConvertHTMLtoUCS2(guchar * data, in
 
 /*
  * get "charset" information from clipboard data
  * return value can be:
  *  1. "UTF-16":      mozilla or "text/html" with "charset=utf-16"
  *  2. "UNKNOWN":     mozilla can't detect what encode it use
  *  3. other:         "text/html" with other charset than utf-16
  */
-void GetHTMLCharset(guchar * data, int32_t dataLength, nsCString& str)
+void GetHTMLCharset(const char* data, int32_t dataLength, nsCString& str)
 {
     // if detect "FFFE" or "FEFF", assume UTF-16
     char16_t* beginChar =  (char16_t*)data;
     if ((beginChar[0] == 0xFFFE) || (beginChar[0] == 0xFEFF)) {
         str.AssignLiteral("UTF-16");
         return;
     }
     // no "FFFE" and "FEFF", assume ASCII first to find "charset" info
-    const nsDependentCString htmlStr((const char *)data, dataLength);
+    const nsDependentCString htmlStr(data, dataLength);
     nsACString::const_iterator start, end;
     htmlStr.BeginReading(start);
     htmlStr.EndReading(end);
     nsACString::const_iterator valueStart(start), valueEnd(start);
 
     if (CaseInsensitiveFindInReadable(
         NS_LITERAL_CSTRING("CONTENT=\"text/html;"),
         start, end)) {
--- a/widget/gtk/nsClipboard.h
+++ b/widget/gtk/nsClipboard.h
@@ -13,19 +13,20 @@
 #include "nsIBinaryOutputStream.h"
 #include <gtk/gtk.h>
 
 // Default Gtk MIME for text
 #define GTK_DEFAULT_MIME_TEXT "UTF8_STRING"
 
 class nsRetrievalContext {
 public:
-    virtual guchar* WaitForClipboardContext(const char* aMimeType,
-                                            int32_t aWhichClipboard,
-                                            uint32_t* aContentLength) = 0;
+    // Returned data must be released by free()
+    virtual const char* WaitForClipboardContext(const char* aMimeType,
+                                                int32_t aWhichClipboard,
+                                                uint32_t* aContentLength) = 0;
     virtual GdkAtom* GetTargets(int32_t aWhichClipboard,
                                 int* aTargetNum) = 0;
 
     nsRetrievalContext() {};
     virtual ~nsRetrievalContext() {};
 
 protected:
     // Idle timeout for receiving selection and property notify events (microsec)
@@ -50,21 +51,27 @@ public:
     void   SelectionGetEvent    (GtkClipboard     *aGtkClipboard,
                                  GtkSelectionData *aSelectionData);
     void   SelectionClearEvent  (GtkClipboard     *aGtkClipboard);
 
 private:
     virtual ~nsClipboard();
 
     // Save global clipboard content to gtk
-    nsresult                     Store            (void);
+    nsresult         Store            (void);
 
     // Get our hands on the correct transferable, given a specific
     // clipboard
-    nsITransferable             *GetTransferable  (int32_t aWhichClipboard);
+    nsITransferable *GetTransferable  (int32_t aWhichClipboard);
+
+    // Send clipboard data by nsITransferable
+    void             SetTransferableData(nsITransferable* aTransferable,
+                                         nsCString& aFlavor,
+                                         const char* aClipboardData,
+                                         uint32_t aClipboardDataLength);
 
     // Hang on to our owners and transferables so we can transfer data
     // when asked.
     nsCOMPtr<nsIClipboardOwner>    mSelectionOwner;
     nsCOMPtr<nsIClipboardOwner>    mGlobalOwner;
     nsCOMPtr<nsITransferable>      mSelectionTransferable;
     nsCOMPtr<nsITransferable>      mGlobalTransferable;
     nsAutoPtr<nsRetrievalContext>  mContext;
--- a/widget/gtk/nsClipboardX11.cpp
+++ b/widget/gtk/nsClipboardX11.cpp
@@ -276,29 +276,33 @@ nsRetrievalContextX11::GetTargets(int32_
   }
 
   gtk_selection_data_free(selection_data);
 
   *aTargetNums = n_targets;
   return targets;
 }
 
-guchar*
+const char*
 nsRetrievalContextX11::WaitForClipboardContext(const char* aMimeType,
                                                int32_t aWhichClipboard,
                                                uint32_t* aContentLength)
 {
     GtkClipboard *clipboard;
     clipboard = gtk_clipboard_get(GetSelectionAtom(aWhichClipboard));
 
     GtkSelectionData *selectionData = WaitForContents(clipboard, aMimeType);
     if (!selectionData)
         return nullptr;
 
+    char* clipboardData = nullptr;
     int contentLength = gtk_selection_data_get_length(selectionData);
-    guchar* data = reinterpret_cast<guchar*>(g_malloc(sizeof(guchar)*contentLength));
-    memcpy(data, gtk_selection_data_get_data(selectionData),
-           sizeof(guchar)*contentLength);
+    if (contentLength > 0) {
+        clipboardData = reinterpret_cast<char*>(
+            moz_xmalloc(sizeof(char)*contentLength));
+        memcpy(clipboardData, gtk_selection_data_get_data(selectionData),
+            sizeof(char)*contentLength);
+    }
     gtk_selection_data_free(selectionData);
 
     *aContentLength = contentLength;
-    return data;
+    return (const char*)clipboardData;
 }
--- a/widget/gtk/nsClipboardX11.h
+++ b/widget/gtk/nsClipboardX11.h
@@ -11,19 +11,18 @@
 #include "nsIClipboard.h"
 #include <gtk/gtk.h>
 
 class nsRetrievalContextX11 : public nsRetrievalContext
 {
 public:
     enum State { INITIAL, COMPLETED, TIMED_OUT };
 
-    virtual guchar* WaitForClipboardContext(const char* aMimeType,
-                                            int32_t aWhichClipboard,
-                                            uint32_t* aContentLength) override;
+    virtual const char* WaitForClipboardContext(const char* aMimeType,
+        int32_t aWhichClipboard, uint32_t* aContentLength) override;
     virtual GdkAtom* GetTargets(int32_t aWhichClipboard,
                                 int* aTargetNums) override;
 
     // Call this when data has been retrieved.
     void Complete(GtkSelectionData* aData, int aDataRequestNumber);
 
     nsRetrievalContextX11();
     virtual ~nsRetrievalContextX11() override;