SafeBrowising: Read and write V4Store from/to disk

components/safe_browsing_db/v4_database.cc

Index: components/safe_browsing_db/v4_database.cc
diff --git a/components/safe_browsing_db/v4_database.cc b/components/safe_browsing_db/v4_database.cc
index d6a8389a6dd41ea24aa0ca2ebfa0afaf795a90fb..127a470bccf79de86ad574413807dfb727f2ff29 100644
--- a/components/safe_browsing_db/v4_database.cc
+++ b/components/safe_browsing_db/v4_database.cc
@@ -6,6 +6,7 @@
 
 #include "base/callback.h"
 #include "base/debug/leak_annotations.h"
+#include "base/files/file_util.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "components/safe_browsing_db/v4_database.h"
@@ -54,6 +55,8 @@ void V4Database::CreateOnTaskRunner(
     ANNOTATE_LEAKING_OBJECT_PTR(factory_);
   }
 
+  DCHECK(base::CreateDirectory(base_path));

[Scott Hess - ex-Googler, 2016/06/22 22:58:38]

I don't think DCHECK and side effects mix well.  Maybe I'm wrong. 
if(!CreateDir()) NOTREACHED(); maybe?

[Or maybe you meant DCHECK(base::DirectoryExists(base_path))?]

[vakh (use Gerrit instead), 2016/06/23 23:09:31]

It ensures that the directory exists or gets created.
NOTREACHED == DCHECK(false);
So it's exactly the same.
The directory may or may not exist already.

[Scott Hess - ex-Googler, 2016/06/24 04:14:11]

What I'm saying is that (I think) you want the CreateDirectory() to run in all
cases, even in production, but in production it may be compiled away (see
definition of DCHECK).  So if it's like:
  if (!base::CreateDirectory(...))
    NOTREACHED();
then the CreateDirectory() happens in debug or production, but the NOTREACHED()
only happens in debug, in production the code will just continue.

[I'm not 100% certain of this - this _is_ a problem with assert(), though, and
IMHO you shouldn't venture into the subtle differences between assert() and
DCHECK() if you can help it.]

[vakh (use Gerrit instead), 2016/06/24 18:03:50]

Yes, didn't realize that. Thanks for catching. Done.
I agree.

+
   std::unique_ptr<StoreMap> store_map = base::MakeUnique<StoreMap>();
   for (const auto& store_info : store_file_name_map) {
     const UpdateListIdentifier& update_list_identifier = store_info.first;