Issue #1467, #3397 - Rewrite the map from threads to tabs

src/plugin/Instances.h

+/*
+ * This file is part of Adblock Plus <https://adblockplus.org/>,
+ * Copyright (C) 2006-2016 Eyeo GmbH
+ *
+ * Adblock Plus is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * Adblock Plus is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _INSTANCES_H_
+#define _INSTANCES_H_
+
+#include <mutex>
+#include <map>
+
+/**
+ * A base class for a synchronized map from threads to BHO instances.
+ *
+ * The member functions here not simply forwarded versions of the container functions.
+ * Rather, they are specialized for tracking BHO calls to SetSite().
+ * Their semantics allow for verification that the call pattern is as expected.
+ *
+ * The key to the map (in the subclass) is the thread ID, thus operations are serialized on a per-key basis.
+ * Calls to SetSite() bracket all other calls, so on a per-key basis
+ *   the order of operations is always either (insert / find-success / erase) or (find-failure).
+ * The library guarantees for std::map seem to indicate that operations on different keys
+ *   do not interfer with each other, but there's some ambiguity there.
+ * This class is synchronized as a matter of defensive programming.
+ */
+template<class Key, class T, T nullValue>
+class SyncMap
+{
+  typedef std::lock_guard<std::mutex> SentryType;
+
+  /**
+   * Underlying map container
+   */
+  std::map<Key, T> idMap;
+
+  /**
+   * Synchronization primitive
+   */
+  mutable std::mutex mutex;
+
+public:
+  /**
+   * Returns true if (as expected) no key of value 'id' was present.
+   * Returns false otherwise.
+   */
+  bool AddIfAbsent(Key id, T p)
+  {
+    SentryType sentry(mutex);
+    auto it = idMap.find(id);
+    if (it != idMap.end())
+    {
+      return false;
+    }
+    idMap[id] = p;
+    return true;
+  }
+
+  /**
+   * Returns true if (as expected) a key of value 'id' was already present.
+   * Returns false otherwise.
+   */
+  bool RemoveAndCheck(Key id)

[sergei, 2016/02/08 13:35:36]

I think that only "Remove" would be better here.

[Eric, 2016/02/08 18:45:30]

OK. I don't.

[Oleksandr, 2016/02/10 10:58:48]

What about RemoveIfPresent? Just for the consistency with the Add method.

[Eric, 2016/05/19 17:23:58]

OK. Changed.

In retrospect, these might better be 'CheckAbsenceThenAdd' and
'CheckPresenceThenRemove', but that's a little wordy.

+  {
+    SentryType sentry(mutex);
+    auto it = idMap.find(id);
+    if (it == idMap.end())
+    {
+      return false;
+    }
+    idMap.erase(it);
+    return true;
+  }
+
+  /**
+   * Returns a non-nullValue if a key of value 'id' is present.
+   * Returns nullValue otherwise.
+   */
+  T Locate(Key id) const
+  {
+    SentryType sentry(mutex);
+    auto it = idMap.find(id);
+    return (it != idMap.end()) ? it->second : nullValue;
+  }
+};
+
+#endif