Fix MemoryCache and related classes on FlipServer.

MemoryCache and FileData have memory leak, so I fixed it in this CL.
I added some unittests for MemoryCache and fixed some problems detected by the tests.

R=rch@chromium.org
BUG=267354
TESTS=net_unittests

Review URL: https://chromiumcodereview.appspot.com/22325012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@216668 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 189 insertions, 66 deletions

diff --git a/net/tools/flip_server/http_interface.cc b/net/tools/flip_server/http_interface.cc
index 9cd1331..7a44c03 100644
--- a/net/tools/flip_server/http_interface.cc
+++ b/net/tools/flip_server/http_interface.cc
@@ -332,13 +332,13 @@ void HttpSM::GetOutput() {
   }
   if (!mci->transformed_header) {
     mci->bytes_sent = SendSynReply(mci->stream_id,
-                                   *(mci->file_data->headers));
+                                   *(mci->file_data->headers()));
     mci->transformed_header = true;
     VLOG(2) << ACCEPTOR_CLIENT_IDENT << "HttpSM: GetOutput transformed "
             << "header stream_id: [" << mci->stream_id << "]";
     return;
   }
-  if (mci->body_bytes_consumed >= mci->file_data->body.size()) {
+  if (mci->body_bytes_consumed >= mci->file_data->body().size()) {
     SendEOF(mci->stream_id);
     output_ordering_.RemoveStreamId(mci->stream_id);
     VLOG(2) << ACCEPTOR_CLIENT_IDENT << "GetOutput remove_stream_id: ["
@@ -346,12 +346,12 @@ void HttpSM::GetOutput() {
     return;
   }
   size_t num_to_write =
-    mci->file_data->body.size() - mci->body_bytes_consumed;
+      mci->file_data->body().size() - mci->body_bytes_consumed;
   if (num_to_write > mci->max_segment_size)
     num_to_write = mci->max_segment_size;
 
   SendDataFrame(mci->stream_id,
-                mci->file_data->body.data() + mci->body_bytes_consumed,
+                mci->file_data->body().data() + mci->body_bytes_consumed,
                 num_to_write, 0, true);
   VLOG(2) << ACCEPTOR_CLIENT_IDENT << "HttpSM: GetOutput SendDataFrame["
           << mci->stream_id << "]: " << num_to_write;
@@ -360,4 +360,3 @@ void HttpSM::GetOutput() {
 }
 
 }  // namespace net
-
diff --git a/net/tools/flip_server/mem_cache.cc b/net/tools/flip_server/mem_cache.cc
index 0af21e0..9249208 100644
--- a/net/tools/flip_server/mem_cache.cc
+++ b/net/tools/flip_server/mem_cache.cc
@@ -13,15 +13,19 @@
 #include <unistd.h>
 
 #include <deque>
+#include <map>
+#include <string>
 
-#include "base/strings/string_piece.h"
+#include "base/strings/string_util.h"
 #include "net/tools/dump_cache/url_to_filename_encoder.h"
 #include "net/tools/dump_cache/url_utilities.h"
 #include "net/tools/flip_server/balsa_frame.h"
 #include "net/tools/flip_server/balsa_headers.h"
 
+namespace {
 // The directory where cache locates);
-std::string FLAGS_cache_base_dir = ".";
+const char FLAGS_cache_base_dir[] = ".";
+}  // namespace
 
 namespace net {
 
@@ -46,40 +50,36 @@ void StoreBodyAndHeadersVisitor::HandleBodyError(BalsaFrame* framer) {
   HandleError();
 }
 
-FileData::FileData(BalsaHeaders* h, const std::string& b)
-    : headers(h), body(b) {
+FileData::FileData(const BalsaHeaders* headers,
+                   const std::string& filename,
+                   const std::string& body)
+    : filename_(filename)
+    , body_(body) {
+  if (headers) {
+    headers_.reset(new BalsaHeaders);
+    headers_->CopyFrom(*headers);
+  }
 }
 
 FileData::FileData() {}
 
 FileData::~FileData() {}
 
-void FileData::CopyFrom(const FileData& file_data) {
-    headers = new BalsaHeaders;
-    headers->CopyFrom(*(file_data.headers));
-    filename = file_data.filename;
-    related_files = file_data.related_files;
-    body = file_data.body;
-  }
-
-MemoryCache::MemoryCache() {}
+MemoryCache::MemoryCache() : cwd_(FLAGS_cache_base_dir) {}
 
-MemoryCache::~MemoryCache() {}
+MemoryCache::~MemoryCache() {
+  ClearFiles();
+}
 
 void MemoryCache::CloneFrom(const MemoryCache& mc) {
-  for (Files::const_iterator i = mc.files_.begin();
-       i != mc.files_.end();
-       ++i) {
-    Files::iterator out_i =
-        files_.insert(make_pair(i->first, FileData())).first;
-    out_i->second.CopyFrom(i->second);
-    cwd_ = mc.cwd_;
-  }
+  DCHECK_NE(this, &mc);
+  ClearFiles();
+  files_ = mc.files_;
+  cwd_ = mc.cwd_;
 }
 
 void MemoryCache::AddFiles() {
   std::deque<std::string> paths;
-  cwd_ = FLAGS_cache_base_dir;
   paths.push_back(cwd_ + "/GET_");
   DIR* current_dir = NULL;
   while (!paths.empty()) {
@@ -192,24 +192,33 @@ void MemoryCache::ReadAndStoreFileContents(const char* filename) {
                                "Fri, 30 Aug, 2019 12:00:00 GMT");
   }
 #endif
-  BalsaHeaders* headers = new BalsaHeaders;
-  headers->CopyFrom(visitor.headers);
+  DCHECK_GE(std::string(filename).size(), cwd_.size() + 1);
+  DCHECK_EQ(std::string(filename).substr(0, cwd_.size()), cwd_);
+  DCHECK_EQ(filename[cwd_.size()], '/');
   std::string filename_stripped = std::string(filename).substr(cwd_.size() + 1);
   LOG(INFO) << "Adding file (" << visitor.body.length() << " bytes): "
             << filename_stripped;
-  files_[filename_stripped] = FileData();
-  FileData& fd = files_[filename_stripped];
-  fd = FileData(headers, visitor.body);
-  fd.filename = std::string(filename_stripped,
-                            filename_stripped.find_first_of('/'));
+  size_t slash_pos = filename_stripped.find('/');
+  if (slash_pos == std::string::npos) {
+    slash_pos = filename_stripped.size();
+  }
+  FileData* data =
+      new FileData(&visitor.headers,
+                   filename_stripped.substr(0, slash_pos),
+                   visitor.body);
+  Files::iterator it = files_.find(filename_stripped);
+  if (it != files_.end()) {
+    delete it->second;
+    it->second = data;
+  } else {
+    files_.insert(std::make_pair(filename_stripped, data));
+  }
 }
 
 FileData* MemoryCache::GetFileData(const std::string& filename) {
   Files::iterator fi = files_.end();
-  if (filename.compare(filename.length() - 5, 5, ".html", 5) == 0) {
-    std::string new_filename(filename.data(), filename.size() - 5);
-    new_filename += ".http";
-    fi = files_.find(new_filename);
+  if (EndsWith(filename, ".html", true)) {
+    fi = files_.find(filename.substr(0, filename.size() - 5) + ".http");
   }
   if (fi == files_.end())
     fi = files_.find(filename);
@@ -217,7 +226,7 @@ FileData* MemoryCache::GetFileData(const std::string& filename) {
   if (fi == files_.end()) {
     return NULL;
   }
-  return &(fi->second);
+  return fi->second;
 }
 
 bool MemoryCache::AssignFileData(const std::string& filename,
@@ -230,5 +239,11 @@ bool MemoryCache::AssignFileData(const std::string& filename,
   return true;
 }
 
-}  // namespace net
+void MemoryCache::ClearFiles() {
+  for (Files::const_iterator i = files_.begin(); i != files_.end(); ++i) {
+    delete i->second;
+  }
+  files_.clear();
+}
 
+}  // namespace net
diff --git a/net/tools/flip_server/mem_cache.h b/net/tools/flip_server/mem_cache.h
index cf5b9bd..806ae53 100644
--- a/net/tools/flip_server/mem_cache.h
+++ b/net/tools/flip_server/mem_cache.h
@@ -7,9 +7,9 @@
 
 #include <map>
 #include <string>
-#include <vector>
 
 #include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
 #include "net/tools/flip_server/balsa_headers.h"
 #include "net/tools/flip_server/balsa_visitor_interface.h"
 #include "net/tools/flip_server/constants.h"
@@ -61,18 +61,26 @@ class StoreBodyAndHeadersVisitor: public BalsaVisitorInterface {
 };
 
 ////////////////////////////////////////////////////////////////////////////////
-
-struct FileData {
+class FileData {
+ public:
   FileData();
-  FileData(BalsaHeaders* h, const std::string& b);
+  FileData(const BalsaHeaders* headers,
+           const std::string& filename,
+           const std::string& body);
   ~FileData();
-  void CopyFrom(const FileData& file_data);
 
-  BalsaHeaders* headers;
-  std::string filename;
-  // priority, filename
-  std::vector< std::pair<int, std::string> > related_files;
-  std::string body;
+  BalsaHeaders* headers() { return headers_.get(); }
+  const BalsaHeaders* headers() const { return headers_.get(); }
+
+  const std::string& filename() { return filename_; }
+  const std::string& body() { return body_; }
+
+ private:
+  scoped_ptr<BalsaHeaders> headers_;
+  std::string filename_;
+  std::string body_;
+
+  DISALLOW_COPY_AND_ASSIGN(FileData);
 };
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -108,17 +116,18 @@ class MemCacheIter {
 
 class MemoryCache {
  public:
-  typedef std::map<std::string, FileData> Files;
+  typedef std::map<std::string, FileData*> Files;
 
  public:
   MemoryCache();
-  ~MemoryCache();
+  virtual ~MemoryCache();
 
   void CloneFrom(const MemoryCache& mc);
 
   void AddFiles();
 
-  void ReadToString(const char* filename, std::string* output);
+  // virtual for unittests
+  virtual void ReadToString(const char* filename, std::string* output);
 
   void ReadAndStoreFileContents(const char* filename);
 
@@ -126,6 +135,9 @@ class MemoryCache {
 
   bool AssignFileData(const std::string& filename, MemCacheIter* mci);
 
+ private:
+  void ClearFiles();
+
   Files files_;
   std::string cwd_;
 };
@@ -139,4 +151,3 @@ class NotifierInterface {
 }  // namespace net
 
 #endif  // NET_TOOLS_FLIP_SERVER_MEM_CACHE_H_
-
diff --git a/net/tools/flip_server/mem_cache_test.cc b/net/tools/flip_server/mem_cache_test.cc
new file mode 100644
index 0000000..d5601ac
--- /dev/null
+++ b/net/tools/flip_server/mem_cache_test.cc
@@ -0,0 +1,98 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/tools/flip_server/mem_cache.h"
+
+#include "net/tools/flip_server/balsa_headers.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+
+namespace {
+
+class MemoryCacheWithFakeReadToString : public MemoryCache {
+ public:
+  virtual ~MemoryCacheWithFakeReadToString() {}
+
+  virtual void ReadToString(const char* filename, std::string* output)
+      OVERRIDE {
+    *output = data_map_[filename];
+  }
+
+  std::map<std::string, std::string> data_map_;
+};
+
+class FlipMemoryCacheTest : public ::testing::Test {
+ public:
+  FlipMemoryCacheTest(): mem_cache_(new MemoryCacheWithFakeReadToString) {}
+
+ protected:
+  scoped_ptr<MemoryCacheWithFakeReadToString> mem_cache_;
+};
+
+TEST_F(FlipMemoryCacheTest, EmptyCache) {
+  MemCacheIter mci;
+  mci.stream_id = 0;
+  ASSERT_EQ(NULL, mem_cache_->GetFileData("./foo"));
+  ASSERT_EQ(NULL, mem_cache_->GetFileData("./bar"));
+  ASSERT_FALSE(mem_cache_->AssignFileData("./hello", &mci));
+}
+
+TEST_F(FlipMemoryCacheTest, ReadAndStoreFileContents) {
+  FileData* foo;
+  FileData* hello;
+
+  mem_cache_->data_map_["./foo"] = "bar";
+  mem_cache_->data_map_["./hello"] = "HTTP/1.0 200 OK\r\n"
+      "key1: value1\r\n"
+      "key2: value2\r\n\r\n"
+      "body: body\r\n";
+  mem_cache_->ReadAndStoreFileContents("./foo");
+  mem_cache_->ReadAndStoreFileContents("./hello");
+
+  foo = mem_cache_->GetFileData("foo");
+  hello = mem_cache_->GetFileData("hello");
+
+  // "./foo" content is broken.
+  ASSERT_EQ(NULL, foo);
+  ASSERT_FALSE(NULL == hello);
+  ASSERT_EQ(hello, mem_cache_->GetFileData("hello"));
+
+  // "HTTP/1.0" is rewritten to "HTTP/1.1".
+  ASSERT_EQ("HTTP/1.1", hello->headers()->response_version());
+  ASSERT_EQ("200", hello->headers()->response_code());
+  ASSERT_EQ("OK", hello->headers()->response_reason_phrase());
+  ASSERT_EQ(4, std::distance(hello->headers()->header_lines_begin(),
+                             hello->headers()->header_lines_end()));
+  ASSERT_TRUE(hello->headers()->HasHeader("key1"));
+  ASSERT_TRUE(hello->headers()->HasHeader("key2"));
+  ASSERT_TRUE(hello->headers()->HasHeader("transfer-encoding"));
+  ASSERT_TRUE(hello->headers()->HasHeader("connection"));
+  ASSERT_EQ("value1", hello->headers()->GetHeaderPosition("key1")->second);
+  ASSERT_EQ("value2", hello->headers()->GetHeaderPosition("key2")->second);
+  ASSERT_EQ("chunked",
+            hello->headers()->GetHeaderPosition("transfer-encoding")->second);
+  ASSERT_EQ("keep-alive",
+            hello->headers()->GetHeaderPosition("connection")->second);
+  ASSERT_EQ("body: body\r\n", hello->body());
+  ASSERT_EQ("hello", hello->filename());
+}
+
+TEST_F(FlipMemoryCacheTest, GetFileDataForHtmlFile) {
+  FileData* hello_html;
+
+  mem_cache_->data_map_["./hello.http"] = "HTTP/1.0 200 OK\r\n"
+      "key1: value1\r\n"
+      "key2: value2\r\n\r\n"
+      "body: body\r\n";
+
+  mem_cache_->ReadAndStoreFileContents("./hello.http");
+  hello_html = mem_cache_->GetFileData("hello.html");
+  ASSERT_FALSE(NULL == hello_html);
+  ASSERT_EQ(hello_html, mem_cache_->GetFileData("hello.http"));
+}
+
+}  // namespace
+
+}  // namespace net
diff --git a/net/tools/flip_server/output_ordering.cc b/net/tools/flip_server/output_ordering.cc
index dd11962..22fd08ac 100644
--- a/net/tools/flip_server/output_ordering.cc
+++ b/net/tools/flip_server/output_ordering.cc
@@ -99,14 +99,14 @@ void OutputOrdering::AddToOutputOrder(const MemCacheIter& mci) {
 
   double think_time_in_s = server_think_time_in_s_;
   std::string x_server_latency =
-    mci.file_data->headers->GetHeader("X-Server-Latency").as_string();
+      mci.file_data->headers()->GetHeader("X-Server-Latency").as_string();
   if (!x_server_latency.empty()) {
     char* endp;
     double tmp_think_time_in_s = strtod(x_server_latency.c_str(), &endp);
     if (endp != x_server_latency.c_str() + x_server_latency.size()) {
       LOG(ERROR) << "Unable to understand X-Server-Latency of: "
                  << x_server_latency << " for resource: "
-                 <<  mci.file_data->filename.c_str();
+                 <<  mci.file_data->filename().c_str();
     } else {
       think_time_in_s = tmp_think_time_in_s;
     }
diff --git a/net/tools/flip_server/spdy_interface.cc b/net/tools/flip_server/spdy_interface.cc
index 16023a5..b4e36be 100644
--- a/net/tools/flip_server/spdy_interface.cc
+++ b/net/tools/flip_server/spdy_interface.cc
@@ -521,43 +521,43 @@ void SpdySM::GetOutput() {
         // this is a server initiated stream.
         // Ideally, we'd do a 'syn-push' here, instead of a syn-reply.
         BalsaHeaders headers;
-        headers.CopyFrom(*(mci->file_data->headers));
+        headers.CopyFrom(*(mci->file_data->headers()));
         headers.ReplaceOrAppendHeader("status", "200");
         headers.ReplaceOrAppendHeader("version", "http/1.1");
         headers.SetRequestFirstlineFromStringPieces("PUSH",
-                                                    mci->file_data->filename,
+                                                    mci->file_data->filename(),
                                                     "");
         mci->bytes_sent = SendSynStream(mci->stream_id, headers);
       } else {
         BalsaHeaders headers;
-        headers.CopyFrom(*(mci->file_data->headers));
+        headers.CopyFrom(*(mci->file_data->headers()));
         mci->bytes_sent = SendSynReply(mci->stream_id, headers);
       }
       return;
     }
-    if (mci->body_bytes_consumed >= mci->file_data->body.size()) {
+    if (mci->body_bytes_consumed >= mci->file_data->body().size()) {
       VLOG(2) << ACCEPTOR_CLIENT_IDENT << "SpdySM: GetOutput "
               << "remove_stream_id: [" << mci->stream_id << "]";
       SendEOF(mci->stream_id);
       return;
     }
     size_t num_to_write =
-      mci->file_data->body.size() - mci->body_bytes_consumed;
+        mci->file_data->body().size() - mci->body_bytes_consumed;
     if (num_to_write > mci->max_segment_size)
       num_to_write = mci->max_segment_size;
 
     bool should_compress = false;
-    if (!mci->file_data->headers->HasHeader("content-encoding")) {
-      if (mci->file_data->headers->HasHeader("content-type")) {
+    if (!mci->file_data->headers()->HasHeader("content-encoding")) {
+      if (mci->file_data->headers()->HasHeader("content-type")) {
         std::string content_type =
-            mci->file_data->headers->GetHeader("content-type").as_string();
+            mci->file_data->headers()->GetHeader("content-type").as_string();
         if (content_type.find("image") == content_type.npos)
           should_compress = true;
       }
     }
 
     SendDataFrame(mci->stream_id,
-                  mci->file_data->body.data() + mci->body_bytes_consumed,
+                  mci->file_data->body().data() + mci->body_bytes_consumed,
                   num_to_write, 0, should_compress);
     VLOG(2) << ACCEPTOR_CLIENT_IDENT << "SpdySM: GetOutput SendDataFrame["
             << mci->stream_id << "]: " << num_to_write;