Cleanup

src/plugin/AdblockPlusClient.cpp

 #include "PluginStdAfx.h"
 
 #include "PluginSettings.h"
 #include "PluginSystem.h"
 #include "PluginFilter.h"
 #include "PluginClientFactory.h"
 #include "PluginMutex.h"
 #include "PluginClass.h"
 
 #include "AdblockPlusClient.h"
@@ skipping 85 matching lines @@
       description.title = ToUtf16String(title);
       std::string specialization;
       message >> specialization;
       description.specialization = ToUtf16String(specialization);
       message >> description.listed;
       result.push_back(description);
     }
     return result;
   }
 
-  Communication::InputBuffer CallAdblockPlusEngineProcedure(Communication::OutputBuffer& message)
+  bool CallEngine(Communication::OutputBuffer& message, Communication::InputBuffer* inputBuffer = NULL)

[Felix Dahlke, 2013/07/25 13:52:33]

Since we always read the data anyway, I think we should use a reference instead
of a pointer and use an input buffer that is going to be ignored as the default
value.

   {
-    std::auto_ptr<Communication::Pipe> pipe = OpenAdblockPlusEnginePipe();
-    pipe->WriteMessage(message);
-    return pipe->ReadMessage();
+    try
+    {
+      std::auto_ptr<Communication::Pipe> pipe = OpenAdblockPlusEnginePipe();
+      pipe->WriteMessage(message);
+      if (inputBuffer != NULL)

[Felix Dahlke, 2013/07/25 13:52:33]

if (inputBuffer) does the same.

+      {
+        *inputBuffer = pipe->ReadMessage();
+      }
+      else
+      {
+        pipe->ReadMessage();
+      }
+    }
+    catch (const std::exception& e)
+    {
+      DEBUG_GENERAL(e.what());
+      return false;
+    }
+    return true;
   }
 
-  Communication::InputBuffer CallAdblockPlusEngineProcedure(Communication::ProcType proc)
+  bool CallEngine(Communication::ProcType proc, Communication::InputBuffer* inputBuffer = NULL)
   {
     Communication::OutputBuffer message;
     message << proc;
-    return CallAdblockPlusEngineProcedure(message);
+    return CallEngine(message, inputBuffer);
   }
 }
 
 CAdblockPlusClient* CAdblockPlusClient::s_instance = NULL;
 
 CAdblockPlusClient::CAdblockPlusClient() : CPluginClientBase()
 {
   m_filter = std::auto_ptr<CPluginFilter>(new CPluginFilter());
 }
 
@@ skipping 77 matching lines @@
   }
   m_criticalSectionFilter.Unlock();
   return isHidden;
 }
 
 bool CAdblockPlusClient::IsWhitelistedUrl(const std::wstring& url)
 {
   Communication::OutputBuffer request;
   request << Communication::PROC_IS_WHITELISTED_URL << ToUtf8String(url);
 
-  try
-  {
-    Communication::InputBuffer response = CallAdblockPlusEngineProcedure(request);
+  Communication::InputBuffer response;
+  if (!CallEngine(request, &response)) return false;;

[Felix Dahlke, 2013/07/25 13:52:33]

One semicolon will do :)

[Wladimir Palant, 2013/07/26 16:45:37]

Style nit: please put the return statement on the next line.

 
-    bool isWhitelisted;
-    response >> isWhitelisted;
-    return isWhitelisted;
-  }
-  catch (const std::exception& e)
-  {
-    DEBUG_GENERAL(e.what());
-    return false;
-  }
+  bool isWhitelisted;
+  response >> isWhitelisted;
+  return isWhitelisted;
 }
 
 int CAdblockPlusClient::GetIEVersion()
 {
   //HKEY_LOCAL_MACHINE\Software\Microsoft\Internet Explorer
   HKEY hKey;
   LSTATUS status = RegOpenKey(HKEY_LOCAL_MACHINE, L"Software\\Microsoft\\Internet Explorer", &hKey);
   if (status != 0)
   {
     return 0;
   }
   DWORD type, cbData;
   BYTE version[50];
   cbData = 50;
   status = RegQueryValueEx(hKey, L"Version", NULL, &type, (BYTE*)version, &cbData);
   if (status != 0)
   {
     return 0;
   }
   RegCloseKey(hKey);
   return (int)(version[0] - 48);
 }
 
 bool CAdblockPlusClient::Matches(const std::wstring& url, const std::wstring& contentType, const std::wstring& domain)
 {
   Communication::OutputBuffer request;
   request << Communication::PROC_MATCHES << ToUtf8String(url) << ToUtf8String(contentType) << ToUtf8String(domain);
 
-  try
-  {
-    Communication::InputBuffer response = CallAdblockPlusEngineProcedure(request);
+  Communication::InputBuffer response;
+  if (!CallEngine(request, &response)) return false;;

[Felix Dahlke, 2013/07/25 13:52:33]

As above, one semicolon will do.

[Wladimir Palant, 2013/07/26 16:45:37]

Style nit: please put the return statement on the next line.

 
-    bool match;
-    response >> match;
-    return match;
-  }
-  catch (const std::exception& e)
-  {
-    DEBUG_GENERAL(e.what());
-    return false;
-  }
+  bool match;
+  response >> match;
+  return match;
 }
 
 std::vector<std::wstring> CAdblockPlusClient::GetElementHidingSelectors(const std::wstring& domain)
 {
   Communication::OutputBuffer request;
   request << Communication::PROC_GET_ELEMHIDE_SELECTORS << ToUtf8String(domain);
 
-  try
-  {
-    Communication::InputBuffer response = CallAdblockPlusEngineProcedure(request);
-    return ReadStrings(response);
-  }
-  catch (const std::exception& e)
-  {
-    DEBUG_GENERAL(e.what());
-    return std::vector<std::wstring>();
-  }
+  Communication::InputBuffer response;
+  if (!CallEngine(request, &response)) return std::vector<std::wstring>();

[Wladimir Palant, 2013/07/26 16:45:37]

Style nit: please put the return statement on the next line.

+  return ReadStrings(response);
 }
 
 std::vector<SubscriptionDescription> CAdblockPlusClient::FetchAvailableSubscriptions()
 {
-  try
-  {
-    Communication::InputBuffer response = CallAdblockPlusEngineProcedure(Communication::PROC_AVAILABLE_SUBSCRIPTIONS);
-    return ReadSubscriptions(response);
-  }
-  catch (const std::exception& e)
-  {
-    DEBUG_GENERAL(e.what());
-    return std::vector<SubscriptionDescription>();
-  }
+  Communication::InputBuffer response;
+  if (!CallEngine(Communication::PROC_AVAILABLE_SUBSCRIPTIONS, &response)) return std::vector<SubscriptionDescription>();

[Wladimir Palant, 2013/07/26 16:45:37]

Style nit: please put the return statement on the next line.

+  return ReadSubscriptions(response);
 }
 
 std::vector<SubscriptionDescription> CAdblockPlusClient::GetListedSubscriptions()
 {
-  try
-  {
-    Communication::InputBuffer response = CallAdblockPlusEngineProcedure(Communication::PROC_LISTED_SUBSCRIPTIONS);
-    return ReadSubscriptions(response);
-  }
-  catch (const std::exception& e)
-  {
-    DEBUG_GENERAL(e.what());
-    return std::vector<SubscriptionDescription>();
-  }
+  Communication::InputBuffer response;
+  if (!CallEngine(Communication::PROC_LISTED_SUBSCRIPTIONS, &response)) return std::vector<SubscriptionDescription>();

[Wladimir Palant, 2013/07/26 16:45:37]

Style nit: please put the return statement on the next line.

+  return ReadSubscriptions(response);
 }
 
 void CAdblockPlusClient::SetSubscription(const std::wstring& url)
 {
   Communication::OutputBuffer request;
   request << Communication::PROC_SET_SUBSCRIPTION << ToUtf8String(url);
-
-  try
-  {
-    CallAdblockPlusEngineProcedure(request);
-  }
-  catch (const std::exception& e)
-  {
-    DEBUG_GENERAL(e.what());
-  }
+  CallEngine(request);
 }
 
 void CAdblockPlusClient::UpdateAllSubscriptions()
 {
-  try
-  {
-    CallAdblockPlusEngineProcedure(Communication::PROC_UPDATE_ALL_SUBSCRIPTIONS);
-  }
-  catch (const std::exception& e)
-  {
-    DEBUG_GENERAL(e.what());
-  }
+  CallEngine(Communication::PROC_UPDATE_ALL_SUBSCRIPTIONS);
 }
 
 std::vector<std::wstring> CAdblockPlusClient::GetExceptionDomains()
 {
-  try
-  {
-    Communication::InputBuffer response = CallAdblockPlusEngineProcedure(Communication::PROC_GET_EXCEPTION_DOMAINS);
-    return ReadStrings(response);
-  }
-  catch (const std::exception& e)
-  {
-    DEBUG_GENERAL(e.what());
-    return std::vector<std::wstring>();
-  }
-}
-
-// A helper method to reuse the code
-void CAdblockPlusClient::PostRequest(Communication::OutputBuffer request)
-{
-  try
-  {
-    CallAdblockPlusEngineProcedure(request);
-  }
-  catch (const std::exception& e)
-  {
-    DEBUG_GENERAL(e.what());
-  }
+  Communication::InputBuffer response;
+  if (!CallEngine(Communication::PROC_GET_EXCEPTION_DOMAINS)) return std::vector<std::wstring>();

[Wladimir Palant, 2013/07/26 16:45:37]

Style nit: please put the return statement on the next line.

+  return ReadStrings(response);
 }
 
 void CAdblockPlusClient::AddFilter(const std::wstring& text)
 {
   Communication::OutputBuffer request;
   request << Communication::PROC_ADD_FILTER << ToUtf8String(text);
-  PostRequest(request);
+  CallEngine(request);
 }
 
 void CAdblockPlusClient::RemoveFilter(const std::wstring& text)
 {
   Communication::OutputBuffer request;
   request << Communication::PROC_REMOVE_FILTER << ToUtf8String(text);
-  PostRequest(request);
+  CallEngine(request);
 }
 
 void CAdblockPlusClient::SetPref(const std::wstring& name, const std::wstring& value)
 {
   Communication::OutputBuffer request;
   request << Communication::PROC_SET_PREF << ToUtf8String(name) << ToUtf8String(value);
-  PostRequest(request);
+  CallEngine(request);
 }
 
 void CAdblockPlusClient::SetPref(const std::wstring& name, const int64_t & value)
 {
   Communication::OutputBuffer request;
   request << Communication::PROC_SET_PREF << ToUtf8String(name) << value;
-  PostRequest(request);
+  CallEngine(request);
 }
 
 void CAdblockPlusClient::SetPref(const std::wstring& name, bool value)
 {
   Communication::OutputBuffer request;
   request << Communication::PROC_SET_PREF << ToUtf8String(name) << value;
-  PostRequest(request);
+  CallEngine(request);
 }
 
 std::wstring CAdblockPlusClient::GetPref(const std::wstring& name, const wchar_t* defaultValue)
 {
   return GetPref(name, std::wstring(defaultValue));
 }
 std::wstring CAdblockPlusClient::GetPref(const std::wstring& name, const std::wstring& defaultValue)
 {
   Communication::OutputBuffer request;
   request << Communication::PROC_GET_PREF << ToUtf8String(name);
 
-  try
+  Communication::InputBuffer response;
+  if (!CallEngine(request, &response)) return defaultValue;

[Wladimir Palant, 2013/07/26 16:45:37]

Style nit: please put the return statement on the next line.

+  bool success;
+  response >> success;
+  if (success)
   {
-    Communication::InputBuffer response = CallAdblockPlusEngineProcedure(request);
-    bool success;
-    response >> success;
-    if (success)
-    {
-      std::string value;
-      response >> value;
-      return ToUtf16String(value);
-    }
-    else
-      return defaultValue;
+    std::string value;
+    response >> value;
+    return ToUtf16String(value);
   }
-  catch (const std::exception& e)
-  {
-    DEBUG_GENERAL(e.what());
+  else
     return defaultValue;
-  }
 }
 
 bool CAdblockPlusClient::GetPref(const std::wstring& name, bool defaultValue)
 {
   Communication::OutputBuffer request;
   request << Communication::PROC_GET_PREF << ToUtf8String(name);
 
-  try
+  Communication::InputBuffer response;
+  if (!CallEngine(request, &response)) return defaultValue;

[Wladimir Palant, 2013/07/26 16:45:37]

Style nit: please put the return statement on the next line.

+  bool success;
+  response >> success;
+  if (success)
   {
-    Communication::InputBuffer response = CallAdblockPlusEngineProcedure(request);
-    bool success;
-    response >> success;
-    if (success)
-    {
-      bool value;
-      response >> value;
-      return value;
-    }
-    else
-      return defaultValue;
+    bool value;
+    response >> value;
+    return value;
   }
-  catch (const std::exception& e)
-  {
-    DEBUG_GENERAL(e.what());
+  else
     return defaultValue;
-  }
 }
 int64_t CAdblockPlusClient::GetPref(const std::wstring& name, int64_t defaultValue)
 {
   Communication::OutputBuffer request;
   request << Communication::PROC_GET_PREF << ToUtf8String(name);
 
-  try
+  Communication::InputBuffer response;
+  if (!CallEngine(request, &response)) return defaultValue;

[Wladimir Palant, 2013/07/26 16:45:37]

Style nit: please put the return statement on the next line.

+    bool success;

[Wladimir Palant, 2013/07/26 16:45:37]

Wrong indentation?

+  response >> success;
+  if (success)
   {
-    Communication::InputBuffer response = CallAdblockPlusEngineProcedure(request);
-    bool success;
-    response >> success;
-    if (success)
-    {
-      int64_t value;
-      response >> value;
-      return value;
-    }
-    else
-      return defaultValue;
+    int64_t value;
+    response >> value;
+    return value;
   }
-  catch (const std::exception& e)
-  {
-    DEBUG_GENERAL(e.what());
+  else
     return defaultValue;
-  }
 }