Add a ReadProductEvents() method to RlzValueStore.

Change GetProductEventsAsCgiHelper() to use it.

Also address review comments on earlier CLs.
Review URL: https://codereview.appspot.com/5837049

git-svn-id: http://rlz.googlecode.com/svn/trunk@88 10bc0f33-e4bf-9a86-80cf-af638054f0c4
diff --git a/lib/rlz_lib2.cc b/lib/rlz_lib2.cc
index 0f8743d..dee0247 100644
--- a/lib/rlz_lib2.cc
+++ b/lib/rlz_lib2.cc
@@ -192,7 +192,7 @@
     return true;
   }
 
-  // Write the new event to registry.
+  // Write the new event to the value store.
   return store->AddProductEvent(product, new_event_value.c_str());
 }
 
@@ -202,7 +202,7 @@
   if (!store || !store->HasAccess(RlzValueStore::kWriteAccess))
     return false;
 
-  // Get the event's registry value and delete it.
+  // Get the event's value store value and delete it.
   const char* point_name = GetAccessPointName(point);
   const char* event_name = GetEventName(event);
   if (!point_name || !event_name)
diff --git a/lib/rlz_value_store.h b/lib/rlz_value_store.h
index 2a90f5c..e54337e 100644
--- a/lib/rlz_value_store.h
+++ b/lib/rlz_value_store.h
@@ -13,6 +13,9 @@
 #include "rlz/win/lib/lib_mutex.h"
 #endif
 
+#include <string>
+#include <vector>
+
 namespace rlz_lib {
 
 // Abstracts away rlz's key value store. On windows, this usually writes to
@@ -40,6 +43,9 @@
   // Product events.
   // Stores |event_rlz| for product |product| as product event.
   virtual bool AddProductEvent(Product product, const char* event_rlz) = 0;
+  // Appends all events for |product| to |events|, in arbirtrary order.
+  virtual bool ReadProductEvents(Product product,
+                                 std::vector<std::string>* events) = 0;
   // Removes the stored event |event_rlz| for |product| if it exists.
   virtual bool ClearProductEvent(Product product, const char* event_rlz) = 0;
 
diff --git a/mac/lib/rlz_value_store_mac.h b/mac/lib/rlz_value_store_mac.h
index a421ac1..c3d2acf 100644
--- a/mac/lib/rlz_value_store_mac.h
+++ b/mac/lib/rlz_value_store_mac.h
@@ -28,6 +28,8 @@
   virtual bool ClearAccessPointRlz(AccessPoint access_point) OVERRIDE;
 
   virtual bool AddProductEvent(Product product, const char* event_rlz) OVERRIDE;
+  virtual bool ReadProductEvents(Product product,
+                                 std::vector<std::string>* events) OVERRIDE;
   virtual bool ClearProductEvent(Product product,
                                  const char* event_rlz) OVERRIDE;
 
diff --git a/mac/lib/rlz_value_store_mac.mm b/mac/lib/rlz_value_store_mac.mm
index 5363da0..9f5a711 100644
--- a/mac/lib/rlz_value_store_mac.mm
+++ b/mac/lib/rlz_value_store_mac.mm
@@ -51,6 +51,12 @@
   return false;
 }
 
+bool RlzValueStoreMac::ReadProductEvents(Product product,
+                                         std::vector<std::string>* events) {
+  NOTIMPLEMENTED();
+  return false;
+}
+
 bool RlzValueStoreMac::ClearProductEvent(Product product,
                                          const char* event_rlz) {
   NOTIMPLEMENTED();
diff --git a/win/lib/rlz_lib.cc b/win/lib/rlz_lib.cc
index 35d5404..26abb04 100644
--- a/win/lib/rlz_lib.cc
+++ b/win/lib/rlz_lib.cc
@@ -23,6 +23,7 @@
 #include "rlz/lib/assert.h"
 #include "rlz/lib/financial_ping.h"
 #include "rlz/lib/lib_values.h"
+#include "rlz/lib/rlz_value_store.h"
 #include "rlz/lib/string_utils.h"
 #include "rlz/win/lib/lib_mutex.h"
 #include "rlz/win/lib/machine_deal.h"
@@ -64,7 +65,8 @@
 }
 
 LONG GetProductEventsAsCgiHelper(rlz_lib::Product product, char* cgi,
-                                 size_t cgi_size) {
+                                 size_t cgi_size,
+                                 rlz_lib::RlzValueStore* store) {
   // Prepend the CGI param key to the buffer.
   std::string cgi_arg;
   base::StringAppendF(&cgi_arg, "%s=", rlz_lib::kEventsCgiVariable);
@@ -75,19 +77,17 @@
   for (index = 0; index < cgi_arg.size(); ++index)
     cgi[index] = cgi_arg[index];
 
-  // Open the events key.
-  base::win::RegKey events;
-  GetEventsRegKey(rlz_lib::kEventsSubkeyName, &product, KEY_READ,
-                  &events);
-  if (!events.Valid())
+  // Read stored events.
+  std::vector<std::string> events;
+  if (!store->ReadProductEvents(product, &events))
     return ERROR_PATH_NOT_FOUND;
 
   // Append the events to the buffer.
   size_t buffer_size = cgi_size - cgi_arg.size();
-  int num_values = 0;
+  size_t num_values = 0;
   LONG result = ERROR_SUCCESS;
 
-  for (num_values = 0; result == ERROR_SUCCESS; ++num_values) {
+  for (num_values = 0; num_values < events.size(); ++num_values) {
     cgi[index] = '\0';
 
     int divider = num_values > 0 ? 1 : 0;
@@ -95,24 +95,19 @@
     if (size <= 0)
       return ERROR_MORE_DATA;
 
-    result = RegEnumValueA(events.Handle(), num_values, cgi + index + divider,
-                           &size, NULL, NULL, NULL, NULL);
-    if (result == ERROR_SUCCESS) {
-      if (divider)
-        cgi[index] = rlz_lib::kEventsCgiSeparator;
+    strncpy(cgi + index + divider, events[num_values].c_str(), size);
+    if (divider)
+      cgi[index] = rlz_lib::kEventsCgiSeparator;
 
-      index += size + divider;
-    }
+    index += std::min((DWORD)events[num_values].length(), size) + divider;
   }
 
-  num_values--;
   cgi[index] = '\0';
 
   if (result == ERROR_MORE_DATA)
     return result;
 
-  return (result == ERROR_NO_MORE_ITEMS && num_values > 0) ?
-    ERROR_SUCCESS : ERROR_FILE_NOT_FOUND;
+  return num_values > 0 ? ERROR_SUCCESS : ERROR_FILE_NOT_FOUND;
 }
 
 bool ClearAllProductEventValues(rlz_lib::Product product, const wchar_t* key) {
@@ -172,18 +167,15 @@
 
   cgi[0] = 0;
 
-  LibMutex lock;
-  if (lock.failed())
-    return false;
-
-  UserKey user_key;
-  if (!user_key.HasAccess(false))
+  ScopedRlzValueStoreLock lock;
+  RlzValueStore* store = lock.GetStore();
+  if (!store || !store->HasAccess(RlzValueStore::kReadAccess))
     return false;
 
   DWORD size_local =
       cgi_size <= kMaxCgiLength + 1 ? cgi_size : kMaxCgiLength + 1;
   UINT length = 0;
-  LONG result = GetProductEventsAsCgiHelper(product, cgi, size_local);
+  LONG result = GetProductEventsAsCgiHelper(product, cgi, size_local, store);
   if (result == ERROR_MORE_DATA && cgi_size >= (kMaxCgiLength + 1))
     result = ERROR_SUCCESS;
 
diff --git a/win/lib/rlz_value_store_registry.cc b/win/lib/rlz_value_store_registry.cc
index 6b2dc14..748c075 100644
--- a/win/lib/rlz_value_store_registry.cc
+++ b/win/lib/rlz_value_store_registry.cc
@@ -110,11 +110,9 @@
 bool RlzValueStoreRegistry::AddProductEvent(Product product,
                                             const char* event_rlz) {
   std::wstring event_rlz_wide(ASCIIToWide(event_rlz));
-  DWORD value = 1;
   base::win::RegKey reg_key;
-  rlz_lib::GetEventsRegKey(kEventsSubkeyName, &product,
-                           KEY_WRITE, &reg_key);
-  if (reg_key.WriteValue(event_rlz_wide.c_str(), value) != ERROR_SUCCESS) {
+  GetEventsRegKey(kEventsSubkeyName, &product, KEY_WRITE, &reg_key);
+  if (reg_key.WriteValue(event_rlz_wide.c_str(), 1) != ERROR_SUCCESS) {
     ASSERT_STRING("AddProductEvent: Could not write the new event value");
     return false;
   }
@@ -122,6 +120,32 @@
   return true;
 }
 
+bool RlzValueStoreRegistry::ReadProductEvents(Product product,
+                                             std::vector<std::string>* events) {
+  // Open the events key.
+  base::win::RegKey events_key;
+  GetEventsRegKey(kEventsSubkeyName, &product, KEY_READ, &events_key);
+  if (!events_key.Valid())
+    return false;
+
+  // Append the events to the buffer.
+  int num_values = 0;
+  LONG result = ERROR_SUCCESS;
+  for (num_values = 0; result == ERROR_SUCCESS; ++num_values) {
+    // Max 32767 bytes according to MSDN, but we never use that much.
+    const size_t kMaxValueNameLength = 2048;
+    char buffer[kMaxValueNameLength];
+    DWORD size = arraysize(buffer);
+
+    result = RegEnumValueA(events_key.Handle(), num_values, buffer, &size,
+                           NULL, NULL, NULL, NULL);
+    if (result == ERROR_SUCCESS)
+      events->push_back(std::string(buffer));
+  }
+
+  return result == ERROR_NO_MORE_ITEMS;
+}
+
 bool RlzValueStoreRegistry::ClearProductEvent(Product product,
                                               const char* event_rlz) {
   std::wstring event_rlz_wide(ASCIIToWide(event_rlz));
@@ -141,13 +165,10 @@
 
 bool RlzValueStoreRegistry::AddStatefulEvent(Product product,
                                              const char* event_rlz) {
-  DWORD data = 1;
-
   base::win::RegKey key;
   std::wstring event_rlz_wide(ASCIIToWide(event_rlz));
-  if (!GetEventsRegKey(rlz_lib::kStatefulEventsSubkeyName,
-                       &product, KEY_WRITE, &key) ||
-      key.WriteValue(event_rlz_wide.c_str(), data) != ERROR_SUCCESS) {
+  if (!GetEventsRegKey(kStatefulEventsSubkeyName, &product, KEY_WRITE, &key) ||
+      key.WriteValue(event_rlz_wide.c_str(), 1) != ERROR_SUCCESS) {
     ASSERT_STRING(
         "AddStatefulEvent: Could not write the new stateful event");
     return false;
@@ -160,8 +181,7 @@
                                             const char* event_rlz) {
   DWORD value;
   base::win::RegKey key;
-  rlz_lib::GetEventsRegKey(kStatefulEventsSubkeyName, &product,
-                           KEY_READ, &key);
+  GetEventsRegKey(kStatefulEventsSubkeyName, &product, KEY_READ, &key);
   std::wstring event_rlz_wide(ASCIIToWide(event_rlz));
   return key.ReadValueDW(event_rlz_wide.c_str(), &value) == ERROR_SUCCESS;
 }
diff --git a/win/lib/rlz_value_store_registry.h b/win/lib/rlz_value_store_registry.h
index 5f6d6df..430cd35 100644
--- a/win/lib/rlz_value_store_registry.h
+++ b/win/lib/rlz_value_store_registry.h
@@ -27,6 +27,8 @@
   virtual bool ClearAccessPointRlz(AccessPoint access_point) OVERRIDE;
 
   virtual bool AddProductEvent(Product product, const char* event_rlz) OVERRIDE;
+  virtual bool ReadProductEvents(Product product,
+                                 std::vector<std::string>* events) OVERRIDE;
   virtual bool ClearProductEvent(Product product,
                                  const char* event_rlz) OVERRIDE;