Commit 9ddc4229 authored by Emilio Cobos Álvarez's avatar Emilio Cobos Álvarez
Browse files

Bug 1760839 - Use more automatic memory management in GTK code. r=stransky 

This fixes a couple potential leaks (e.g., see the UPowerClient changes,
we weren't freeing all the devices properly).

Differential Revision: https://phabricator.services.mozilla.com/D141775
parent 77029463

browser/components/shell/nsGNOMEShellService.cpp

+2 −3
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ 
#include "nsIImageLoadingContent.h"

#include "imgIRequest.h"

#include "imgIContainer.h"

#include "mozilla/GRefPtr.h"

#include "mozilla/Sprintf.h"

#include "mozilla/WidgetUtils.h"

#include "mozilla/WidgetUtilsGtk.h"
@@ -357,14 +358,12 @@ nsGNOMEShellService::GetCanSetDesktopBackground(bool* aResult) { 
}



static nsresult WriteImage(const nsCString& aPath, imgIContainer* aImage) {

  GdkPixbuf* pixbuf = nsImageToPixbuf::ImageToPixbuf(aImage);

  RefPtr<GdkPixbuf> pixbuf = nsImageToPixbuf::ImageToPixbuf(aImage);

  if (!pixbuf) {

    return NS_ERROR_NOT_AVAILABLE;

  }



  gboolean res = gdk_pixbuf_save(pixbuf, aPath.get(), "png", nullptr, nullptr);



  g_object_unref(pixbuf);

  return res ? NS_OK : NS_ERROR_FAILURE;

}

hal/linux/UPowerClient.cpp

+46 −72
Original line number Diff line number Diff line
@@ -9,26 +9,13 @@ 
#include <dbus/dbus-glib-lowlevel.h>

#include <mozilla/Attributes.h>

#include <mozilla/dom/battery/Constants.h>

#include "nsAutoRef.h"

#include "mozilla/GRefPtr.h"

#include "mozilla/GUniquePtr.h"

#include <cmath>



/*

 * Helper that manages the destruction of glib objects as soon as they leave

 * the current scope.

 *

 * We are specializing nsAutoRef class.

 */



template <>

class nsAutoRefTraits<GHashTable> : public nsPointerRefTraits<GHashTable> {

 public:

  static void Release(GHashTable* ptr) { g_hash_table_unref(ptr); }

};



using namespace mozilla::dom::battery;



namespace mozilla {

namespace hal_impl {

namespace mozilla::hal_impl {



/**

 * This is the declaration of UPowerClient class. This class is listening and
@@ -69,9 +56,8 @@ class UPowerClient { 


  /**

   * Returns a hash table with the properties of aDevice.

   * Note: the caller has to unref the hash table.

   */

  GHashTable* GetDevicePropertiesSync(DBusGProxy* aProxy);

  already_AddRefed<GHashTable> GetDevicePropertiesSync(DBusGProxy* aProxy);

  void GetDevicePropertiesAsync(DBusGProxy* aProxy);

  static void GetDevicePropertiesCallback(DBusGProxy* aProxy,

                                          DBusGProxyCall* aCall, void* aData);
@@ -104,16 +90,16 @@ class UPowerClient { 
                                                  void* aData);



  // The DBus connection object.

  DBusGConnection* mDBusConnection;

  RefPtr<DBusGConnection> mDBusConnection;



  // The DBus proxy object to upower.

  DBusGProxy* mUPowerProxy;

  RefPtr<DBusGProxy> mUPowerProxy;



  // The path of the tracked device.

  gchar* mTrackedDevice;

  GUniquePtr<gchar> mTrackedDevice;



  // The DBusGProxy for the tracked device.

  DBusGProxy* mTrackedDeviceProxy;

  RefPtr<DBusGProxy> mTrackedDeviceProxy;



  double mLevel;

  bool mCharging;
@@ -163,11 +149,7 @@ UPowerClient* UPowerClient::GetInstance() { 
}



UPowerClient::UPowerClient()

    : mDBusConnection(nullptr),

      mUPowerProxy(nullptr),

      mTrackedDevice(nullptr),

      mTrackedDeviceProxy(nullptr),

      mLevel(kDefaultLevel),

    : mLevel(kDefaultLevel),

      mCharging(kDefaultCharging),

      mRemainingTime(kDefaultRemainingTime) {}
@@ -179,12 +161,12 @@ UPowerClient::~UPowerClient() { 
}



void UPowerClient::BeginListening() {

  GError* error = nullptr;

  mDBusConnection = dbus_g_bus_get(DBUS_BUS_SYSTEM, &error);

  GUniquePtr<GError> error;

  mDBusConnection =

      dont_AddRef(dbus_g_bus_get(DBUS_BUS_SYSTEM, getter_Transfers(error)));



  if (!mDBusConnection) {

    HAL_LOG("Failed to open connection to bus: %s\n", error->message);

    g_error_free(error);

    return;

  }
@@ -199,9 +181,9 @@ void UPowerClient::BeginListening() { 
  dbus_connection_add_filter(dbusConnection, ConnectionSignalFilter, this,

                             nullptr);



  mUPowerProxy = dbus_g_proxy_new_for_name(

  mUPowerProxy = dont_AddRef(dbus_g_proxy_new_for_name(

      mDBusConnection, "org.freedesktop.UPower", "/org/freedesktop/UPower",

      "org.freedesktop.UPower");

      "org.freedesktop.UPower"));



  UpdateTrackedDeviceSync();
@@ -231,21 +213,15 @@ void UPowerClient::StopListening() { 
  dbus_g_proxy_disconnect_signal(mUPowerProxy, "DeviceChanged",

                                 G_CALLBACK(DeviceChanged), this);



  g_free(mTrackedDevice);

  mTrackedDevice = nullptr;



  if (mTrackedDeviceProxy) {

    dbus_g_proxy_disconnect_signal(mTrackedDeviceProxy, "PropertiesChanged",

                                   G_CALLBACK(PropertiesChanged), this);



    g_object_unref(mTrackedDeviceProxy);

    mTrackedDeviceProxy = nullptr;

  }



  g_object_unref(mUPowerProxy);

  mUPowerProxy = nullptr;



  dbus_g_connection_unref(mDBusConnection);

  mDBusConnection = nullptr;



  // We should now show the default values, not the latest we got.
@@ -258,27 +234,23 @@ void UPowerClient::UpdateTrackedDeviceSync() { 
  GType typeGPtrArray =

      dbus_g_type_get_collection("GPtrArray", DBUS_TYPE_G_OBJECT_PATH);

  GPtrArray* devices = nullptr;

  GError* error = nullptr;



  // Reset the current tracked device:

  g_free(mTrackedDevice);

  mTrackedDevice = nullptr;



  // Reset the current tracked device proxy:

  if (mTrackedDeviceProxy) {

    dbus_g_proxy_disconnect_signal(mTrackedDeviceProxy, "PropertiesChanged",

                                   G_CALLBACK(PropertiesChanged), this);



    g_object_unref(mTrackedDeviceProxy);

    mTrackedDeviceProxy = nullptr;

  }



  GUniquePtr<GError> error;

  // If that fails, that likely means upower isn't installed.

  if (!dbus_g_proxy_call(mUPowerProxy, "EnumerateDevices", &error,

                         G_TYPE_INVALID, typeGPtrArray, &devices,

                         G_TYPE_INVALID)) {

  if (!dbus_g_proxy_call(mUPowerProxy, "EnumerateDevices",

                         getter_Transfers(error), G_TYPE_INVALID, typeGPtrArray,

                         &devices, G_TYPE_INVALID)) {

    HAL_LOG("Error: %s\n", error->message);

    g_error_free(error);

    return;

  }
@@ -287,23 +259,25 @@ void UPowerClient::UpdateTrackedDeviceSync() { 
   * TODO: we could try to combine more than one battery.

   */

  for (guint i = 0; i < devices->len; ++i) {

    gchar* devicePath = static_cast<gchar*>(g_ptr_array_index(devices, i));

    GUniquePtr<gchar> devicePath(

        static_cast<gchar*>(g_ptr_array_index(devices, i)));

    if (mTrackedDevice) {

      continue;

    }



    DBusGProxy* proxy = dbus_g_proxy_new_from_proxy(

        mUPowerProxy, "org.freedesktop.DBus.Properties", devicePath);

    RefPtr<DBusGProxy> proxy = dont_AddRef(dbus_g_proxy_new_from_proxy(

        mUPowerProxy, "org.freedesktop.DBus.Properties", devicePath.get()));



    nsAutoRef<GHashTable> hashTable(GetDevicePropertiesSync(proxy));

    RefPtr<GHashTable> hashTable(GetDevicePropertiesSync(proxy));



    if (g_value_get_uint(static_cast<const GValue*>(

            g_hash_table_lookup(hashTable, "Type"))) == sDeviceTypeBattery) {

      UpdateSavedInfo(hashTable);

      mTrackedDevice = devicePath;

      mTrackedDeviceProxy = proxy;

      break;

      mTrackedDevice = std::move(devicePath);

      mTrackedDeviceProxy = std::move(proxy);

      // Can't break here because we still need to iterate over all other

      // devices to free them.

    }



    g_object_unref(proxy);

    g_free(devicePath);

  }



  if (mTrackedDeviceProxy) {
@@ -326,9 +300,9 @@ void UPowerClient::DeviceChanged(DBusGProxy* aProxy, const gchar* aObjectPath, 
  }



#if GLIB_MAJOR_VERSION >= 2 && GLIB_MINOR_VERSION >= 16

  if (g_strcmp0(aObjectPath, aListener->mTrackedDevice)) {

  if (g_strcmp0(aObjectPath, aListener->mTrackedDevice.get())) {

#else

  if (g_ascii_strcasecmp(aObjectPath, aListener->mTrackedDevice)) {

  if (g_ascii_strcasecmp(aObjectPath, aListener->mTrackedDevice.get())) {

#endif

    return;

  }
@@ -355,34 +329,35 @@ DBusHandlerResult UPowerClient::ConnectionSignalFilter( 
  return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;

}



GHashTable* UPowerClient::GetDevicePropertiesSync(DBusGProxy* aProxy) {

  GError* error = nullptr;

  GHashTable* hashTable = nullptr;

already_AddRefed<GHashTable> UPowerClient::GetDevicePropertiesSync(

    DBusGProxy* aProxy) {

  GUniquePtr<GError> error;

  RefPtr<GHashTable> hashTable;

  GType typeGHashTable =

      dbus_g_type_get_map("GHashTable", G_TYPE_STRING, G_TYPE_VALUE);

  if (!dbus_g_proxy_call(aProxy, "GetAll", &error, G_TYPE_STRING,

                         "org.freedesktop.UPower.Device", G_TYPE_INVALID,

                         typeGHashTable, &hashTable, G_TYPE_INVALID)) {

  if (!dbus_g_proxy_call(aProxy, "GetAll", getter_Transfers(error),

                         G_TYPE_STRING, "org.freedesktop.UPower.Device",

                         G_TYPE_INVALID, typeGHashTable,

                         hashTable.StartAssignment(), G_TYPE_INVALID)) {

    HAL_LOG("Error: %s\n", error->message);

    g_error_free(error);

    return nullptr;

  }



  return hashTable;

  return hashTable.forget();

}



/* static */

void UPowerClient::GetDevicePropertiesCallback(DBusGProxy* aProxy,

                                               DBusGProxyCall* aCall,

                                               void* aData) {

  GError* error = nullptr;

  GHashTable* hashTable = nullptr;

  GUniquePtr<GError> error;

  RefPtr<GHashTable> hashTable;

  GType typeGHashTable =

      dbus_g_type_get_map("GHashTable", G_TYPE_STRING, G_TYPE_VALUE);

  if (!dbus_g_proxy_end_call(aProxy, aCall, &error, typeGHashTable, &hashTable,

  if (!dbus_g_proxy_end_call(aProxy, aCall, getter_Transfers(error),

                             typeGHashTable, hashTable.StartAssignment(),

                             G_TYPE_INVALID)) {

    HAL_LOG("Error: %s\n", error->message);

    g_error_free(error);

  } else {

    sInstance->UpdateSavedInfo(hashTable);

    hal::NotifyBatteryChange(hal::BatteryInformation(
@@ -470,5 +445,4 @@ bool UPowerClient::IsCharging() { return mCharging; } 


double UPowerClient::GetRemainingTime() { return mRemainingTime; }



}  // namespace hal_impl

}  // namespace mozilla
 
}  // namespace mozilla::hal_impl

hal/moz.build

+3 −0
Original line number Diff line number Diff line
@@ -45,6 +45,7 @@ if CONFIG["MOZ_WIDGET_TOOLKIT"] == "android": 
        "android/AndroidHal.cpp",

    ]

elif CONFIG["OS_TARGET"] == "Linux":



    UNIFIED_SOURCES += [

        "fallback/FallbackProcessPriority.cpp",

        "fallback/FallbackScreenConfiguration.cpp",
@@ -127,6 +128,8 @@ LOCAL_INCLUDES += [ 
]



CFLAGS += CONFIG["GLIB_CFLAGS"]

CFLAGS += CONFIG["MOZ_GTK3_CFLAGS"]

CFLAGS += CONFIG["MOZ_DBUS_GLIB_CFLAGS"]

CXXFLAGS += CONFIG["GLIB_CFLAGS"]

CXXFLAGS += CONFIG["MOZ_GTK3_CFLAGS"]

CXXFLAGS += CONFIG["MOZ_DBUS_GLIB_CFLAGS"]

toolkit/system/gnome/nsAlertsIconListener.cpp

+13 −16
Original line number Diff line number Diff line
@@ -16,11 +16,15 @@ 
#include "nsIObserverService.h"

#include "nsCRT.h"

#include "mozilla/XREAppData.h"

#include "mozilla/GRefPtr.h"

#include "mozilla/GUniquePtr.h"

#include "mozilla/UniquePtrExtensions.h"



#include <dlfcn.h>

#include <gdk/gdk.h>



extern const mozilla::StaticXREAppData* gAppData;

using namespace mozilla;

extern const StaticXREAppData* gAppData;



static bool gHasActions = false;

static bool gHasCaps = false;
@@ -64,7 +68,8 @@ static void notify_closed_marshal(GClosure* closure, GValue* return_value, 
  NS_RELEASE(alert);

}



static GdkPixbuf* GetPixbufFromImgRequest(imgIRequest* aRequest) {

static already_AddRefed<GdkPixbuf> GetPixbufFromImgRequest(

    imgIRequest* aRequest) {

  nsCOMPtr<imgIContainer> image;

  nsresult rv = aRequest->GetImage(getter_AddRefs(image));

  if (NS_FAILED(rv)) {
@@ -145,14 +150,8 @@ nsAlertsIconListener::OnImageMissing(nsISupports*) { 


NS_IMETHODIMP

nsAlertsIconListener::OnImageReady(nsISupports*, imgIRequest* aRequest) {

  GdkPixbuf* imagePixbuf = GetPixbufFromImgRequest(aRequest);

  if (!imagePixbuf) {

    ShowAlert(nullptr);

  } else {

  RefPtr<GdkPixbuf> imagePixbuf = GetPixbufFromImgRequest(aRequest);

  ShowAlert(imagePixbuf);

    g_object_unref(imagePixbuf);

  }



  return NS_OK;

}
@@ -202,10 +201,9 @@ nsresult nsAlertsIconListener::ShowAlert(GdkPixbuf* aPixbuf) { 
  g_closure_set_marshal(closure, notify_closed_marshal);

  mClosureHandler =

      g_signal_connect_closure(mNotification, "closed", closure, FALSE);

  GError* error = nullptr;

  if (!notify_notification_show(mNotification, &error)) {

  GUniquePtr<GError> error;

  if (!notify_notification_show(mNotification, getter_Transfers(error))) {

    NS_WARNING(error->message);

    g_error_free(error);

    return NS_ERROR_FAILURE;

  }
@@ -253,10 +251,9 @@ nsresult nsAlertsIconListener::Close() { 
    return NS_OK;

  }



  GError* error = nullptr;

  if (!notify_notification_close(mNotification, &error)) {

  GUniquePtr<GError> error;

  if (!notify_notification_close(mNotification, getter_Transfers(error))) {

    NS_WARNING(error->message);

    g_error_free(error);

    return NS_ERROR_FAILURE;

  }

toolkit/system/gnome/nsGIOService.cpp

+99 −120

File changed.

Preview size limit exceeded, changes collapsed.