Issue 1490 - Enable pre-configurable properties in Internet Explorer

src/engine/Main.cpp

Index: src/engine/Main.cpp
===================================================================
--- a/src/engine/Main.cpp
+++ b/src/engine/Main.cpp
@@ -31,6 +31,7 @@
 #include "AdblockPlus.h"
 #include "Debug.h"
 #include "Updater.h"
+#include "Registry.h"
 
 namespace
 {
@@ -430,6 +431,19 @@
   }
 }
 
+std::wstring LoadPreconfigurationValue(const std::wstring& preconfigName)

[Eric, 2015/06/19 14:51:40]

This name would be better as "RegistryPreconfigurationValue" or
"PreconfigurationValueFromRegistry".

The name present now is an imperative verb (what action to do). This name would
work better as a declarative noun (what result to return). As a rule, when you
care about the return value, declaratives (nouns) work better.

[Eric, 2015/06/19 14:51:40]

Also. This function can go in the anonymous namespace immediately above.

[Oleksandr, 2015/06/22 08:27:53]

Done.

[sergei, 2015/06/22 09:17:06]

However our functions usually contain the verb in the name, so should it be like
"GetPreconfigurationValueFromRegistry"?

+{
+  try
+  {
+    AdblockPlus::RegistryKey regKey(HKEY_CURRENT_USER, L"Software\\AdblockPlus");
+    return regKey.value_wstring(preconfigName);
+  }
+  catch (std::runtime_error&)

[sergei, 2015/06/19 12:30:28]

`const`

[sergei, 2015/06/19 12:31:47]

And `std::exception` would be better than `std::runtime_error`.

[Eric, 2015/06/19 14:51:41]

"runtime_error" is correct. If the constructor were to throw logic_error, we
would not want to mask it here; it would be better to let that exception
propagate upward and handle it more generically.

[Oleksandr, 2015/06/22 08:27:52]

Acknowledged.

+  {
+    return L"";

[sergei, 2015/06/19 12:30:28]

I would put it after try-catch.

[Eric, 2015/06/19 14:51:40]

The code reads better to have the return statement here. It's much clearer that
the empty string is a substitute value for a registry key that's absent.

Compiler path analysis will catch a missing return statement just fine. It's an
artifact of old, less competent compilers that the practice arose of putting a
return statement at the end of a function block.

[Oleksandr, 2015/06/22 08:27:53]

Acknowledged.

+  }
+}
+
 std::auto_ptr<AdblockPlus::FilterEngine> CreateFilterEngine(const std::wstring& locale)
 {
   AdblockPlus::AppInfo appInfo;
@@ -453,7 +467,12 @@
 
   std::string dataPath = ToUtf8String(GetAppDataPath());
   dynamic_cast<AdblockPlus::DefaultFileSystem*>(jsEngine->GetFileSystem().get())->SetBasePath(dataPath);
-  std::auto_ptr<AdblockPlus::FilterEngine> filterEngine(new AdblockPlus::FilterEngine(jsEngine));
+  std::map<std::string, AdblockPlus::JsValuePtr> preconfig;
+  preconfig["disable_auto_updates"] = jsEngine->NewValue(
+    LoadPreconfigurationValue(L"disable_auto_updates") == L"true");
+  preconfig["suppress_first_run_page"] = jsEngine->NewValue(
+    LoadPreconfigurationValue(L"suppress_first_run_page") == L"true");
+  std::auto_ptr<AdblockPlus::FilterEngine> filterEngine(new AdblockPlus::FilterEngine(jsEngine, preconfig));
   return filterEngine;
 }