ABANDONED: OOPIFs: Moving stitching of local paths from renderer to browser process.

content/renderer/dom_serializer_browsertest.cc

Index: content/renderer/dom_serializer_browsertest.cc
diff --git a/content/renderer/dom_serializer_browsertest.cc b/content/renderer/dom_serializer_browsertest.cc
index 781fee0f3ce189ae07de2e3ae5c7b3e06a28a538..c818fc5a75952ce9f785e840498f010662df00d8 100644
--- a/content/renderer/dom_serializer_browsertest.cc
+++ b/content/renderer/dom_serializer_browsertest.cc
@@ -159,8 +159,8 @@ class DomSerializerTests : public ContentBrowserTest,
                            public WebPageSerializerClient {
  public:
   DomSerializerTests()
-    : serialized_(false),
-      local_directory_name_(FILE_PATH_LITERAL("./dummy_files/")) {}
+      : got_end_of_frame_notification_(false),
+        local_directory_name_(FILE_PATH_LITERAL("./dummy_files/")) {}
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
     command_line->AppendSwitch(switches::kSingleProcess);

[ncarter (slow), 2015/10/08 20:57:55]

[FYI comment, no action required] When you write new tests in the future, know
that there are good alternatives to the pattern used here. Here, the whole
renderer stack is made to run in the browser process. That results in a .cc file
that #includes both "content/public/renderer" and "content/public/browser",
which is very strange and, outside of a test that works like this, totally
nonsensical.

A more conventional way to write this test (at least nowadays) would be to make
it a RenderViewTest, where test code runs in a renderer environment, there's no
browser process at all, and the IPC interactions with the browser process are
mocked out and validated.

It's totally fine, of course, to update/maintain the existing test. Just wanted
to let you know that this is a weird pattern, and probably not a great idea for
future tests.

[Łukasz Anforowicz, 2015/10/09 16:54:08]

Acknowledged.

@@ -175,51 +175,29 @@ class DomSerializerTests : public ContentBrowserTest,
         shell()->web_contents()->GetRenderViewHost()->GetRoutingID();
   }
 
-  // DomSerializerDelegate.
-  void didSerializeDataForFrame(const WebURL& frame_web_url,
-                                const WebCString& data,
-                                PageSerializationStatus status) override {
-    GURL frame_url(frame_web_url);
-    // If the all frames are finished saving, check all finish status
-    if (status == WebPageSerializerClient::AllFramesAreFinished) {
-      SerializationFinishStatusMap::iterator it =
-          serialization_finish_status_.begin();
-      for (; it != serialization_finish_status_.end(); ++it)
-        ASSERT_TRUE(it->second);
-      serialized_ = true;
-      return;
-    }
-
-    // Check finish status of current frame.
-    SerializationFinishStatusMap::iterator it =
-        serialization_finish_status_.find(frame_url.spec());
-    // New frame, set initial status as false.
-    if (it == serialization_finish_status_.end())
-      serialization_finish_status_[frame_url.spec()] = false;
-
-    it = serialization_finish_status_.find(frame_url.spec());
-    ASSERT_TRUE(it != serialization_finish_status_.end());
-    // In process frame, finish status should be false.
-    ASSERT_FALSE(it->second);
-
-    // Add data to corresponding frame's content.
-    serialized_frame_map_[frame_url.spec()] += data.data();
-
-    // Current frame is completed saving, change the finish status.
-    if (status == WebPageSerializerClient::CurrentFrameIsFinished)
-      it->second = true;
+  void writeHtmlFragment(const WebCString& data) override {
+    serialization_output_ += data.data();
   }
 
-  bool HasSerializedFrame(const GURL& frame_url) {
-    return serialized_frame_map_.find(frame_url.spec()) !=
-           serialized_frame_map_.end();
+  void writeLocalPathForSubFrame(const WebFrame& subFrame) override {
+    // Tests in content/renderer/dom_serializer_browsertest.cc do not verify
+    // local paths in the serializer html (they are verified by
+    // chrome/browser/download/save_page_browsertest.cc).  Therefore the body of
+    // writeLocalPathForSubFrame can be empty below.
   }
 
-  const std::string& GetSerializedContentForFrame(
-      const GURL& frame_url) {
-    return serialized_frame_map_[frame_url.spec()];
+  void writeLocalPathForSavableResource(
+      const WebURL& savableResourceURL) override {
+    // Tests in content/renderer/dom_serializer_browsertest.cc do not verify
+    // local paths in the serializer html (they are verified by
+    // chrome/browser/download/save_page_browsertest.cc).  Therefore the body of
+    // writeLocalPathForSavableResource can be empty below.
   }
 
+  void endOfFrame() override { got_end_of_frame_notification_ = true; }
+
+  const std::string& GetSerializedContent() { return serialization_output_; }
+
   RenderView* GetRenderView() {
     return RenderView::FromRoutingID(render_view_routing_id_);
   }
@@ -271,13 +249,11 @@ class DomSerializerTests : public ContentBrowserTest,
     WebVector<WebString> local_paths;
     local_paths.assign(&file_path, 1);
     // Start serializing DOM.
-    bool result = WebPageSerializer::serialize(web_frame->toWebLocalFrame(),
-       static_cast<WebPageSerializerClient*>(this),
-       links,
-       local_paths,
-       local_directory_name_.AsUTF16Unsafe());
+    bool result = WebPageSerializer::serialize(
+        web_frame->toWebLocalFrame(),
+        static_cast<WebPageSerializerClient*>(this));
     ASSERT_TRUE(result);
-    ASSERT_TRUE(serialized_);
+    ASSERT_TRUE(got_end_of_frame_notification_);
   }
 
   void SerializeHTMLDOMWithDocTypeOnRenderer(const GURL& file_url) {
@@ -289,9 +265,7 @@ class DomSerializerTests : public ContentBrowserTest,
     // Do serialization.
     SerializeDomForURL(file_url);
     // Load the serialized contents.
-    ASSERT_TRUE(HasSerializedFrame(file_url));
-    const std::string& serialized_contents =
-        GetSerializedContentForFrame(file_url);
+    const std::string& serialized_contents = GetSerializedContent();
     LoadContents(serialized_contents, file_url,
                  web_frame->document().encoding());
     // Make sure serialized contents still have document type.
@@ -309,9 +283,7 @@ class DomSerializerTests : public ContentBrowserTest,
     // Do serialization.
     SerializeDomForURL(file_url);
     // Load the serialized contents.
-    ASSERT_TRUE(HasSerializedFrame(file_url));
-    const std::string& serialized_contents =
-        GetSerializedContentForFrame(file_url);
+    const std::string& serialized_contents = GetSerializedContent();
     LoadContents(serialized_contents, file_url,
                  web_frame->document().encoding());
     // Make sure serialized contents do not have document type.
@@ -325,9 +297,7 @@ class DomSerializerTests : public ContentBrowserTest,
     // Do serialization.
     SerializeDomForURL(xml_file_url);
     // Compare the serialized contents with original contents.
-    ASSERT_TRUE(HasSerializedFrame(xml_file_url));
-    const std::string& serialized_contents =
-        GetSerializedContentForFrame(xml_file_url);
+    const std::string& serialized_contents = GetSerializedContent();
     ASSERT_EQ(original_contents, serialized_contents);
   }
 
@@ -344,9 +314,7 @@ class DomSerializerTests : public ContentBrowserTest,
     // Do serialization.
     SerializeDomForURL(file_url);
     // Make sure the serialized contents have MOTW ;
-    ASSERT_TRUE(HasSerializedFrame(file_url));
-    const std::string& serialized_contents =
-        GetSerializedContentForFrame(file_url);
+    const std::string& serialized_contents = GetSerializedContent();
     ASSERT_FALSE(std::string::npos ==
         serialized_contents.find(motw_declaration));
   }
@@ -371,9 +339,7 @@ class DomSerializerTests : public ContentBrowserTest,
     SerializeDomForURL(file_url);
 
     // Load the serialized contents.
-    ASSERT_TRUE(HasSerializedFrame(file_url));
-    const std::string& serialized_contents =
-        GetSerializedContentForFrame(file_url);
+    const std::string& serialized_contents = GetSerializedContent();
     LoadContents(serialized_contents, file_url,
                  web_frame->document().encoding());
     // Make sure the first child of HEAD element is META which has charset
@@ -427,9 +393,7 @@ class DomSerializerTests : public ContentBrowserTest,
     SerializeDomForURL(file_url);
 
     // Load the serialized contents.
-    ASSERT_TRUE(HasSerializedFrame(file_url));
-    const std::string& serialized_contents =
-        GetSerializedContentForFrame(file_url);
+    const std::string& serialized_contents = GetSerializedContent();
     LoadContents(serialized_contents, file_url,
                  web_frame->document().encoding());
     // Make sure only first child of HEAD element is META which has charset
@@ -484,9 +448,7 @@ class DomSerializerTests : public ContentBrowserTest,
     // Do serialization.
     SerializeDomForURL(file_url);
     // Compare the serialized contents with original contents.
-    ASSERT_TRUE(HasSerializedFrame(file_url));
-    const std::string& serialized_contents =
-        GetSerializedContentForFrame(file_url);
+    const std::string& serialized_contents = GetSerializedContent();
     // Compare the serialized contents with original contents to make sure
     // they are same.
     // Because we add MOTW when serializing DOM, so before comparison, we also
@@ -537,9 +499,7 @@ class DomSerializerTests : public ContentBrowserTest,
     // Do serialization.
     SerializeDomForURL(file_url);
     // Compare the serialized contents with original contents.
-    ASSERT_TRUE(HasSerializedFrame(file_url));
-    const std::string& serialized_contents =
-        GetSerializedContentForFrame(file_url);
+    const std::string& serialized_contents = GetSerializedContent();
     // Compare the serialized contents with original contents to make sure
     // they are same.
     std::string original_str =
@@ -578,9 +538,7 @@ class DomSerializerTests : public ContentBrowserTest,
     // Do serialization.
     SerializeDomForURL(file_url);
     // Check the serialized string.
-    ASSERT_TRUE(HasSerializedFrame(file_url));
-    const std::string& serialized_contents =
-        GetSerializedContentForFrame(file_url);
+    const std::string& serialized_contents = GetSerializedContent();
     // Confirm that the serialized string has no non-standard HTML entities.
     ASSERT_EQ(std::string::npos, serialized_contents.find("&percnt;"));
     ASSERT_EQ(std::string::npos, serialized_contents.find("&nsup;"));
@@ -632,9 +590,7 @@ class DomSerializerTests : public ContentBrowserTest,
     SerializeDomForURL(file_url);
 
     // Load the serialized contents.
-    ASSERT_TRUE(HasSerializedFrame(file_url));
-    const std::string& serialized_contents =
-        GetSerializedContentForFrame(file_url);
+    const std::string& serialized_contents = GetSerializedContent();
     LoadContents(serialized_contents, file_url,
                  web_frame->document().encoding());
 
@@ -701,9 +657,7 @@ class DomSerializerTests : public ContentBrowserTest,
     // Do serialization.
     SerializeDomForURL(file_url);
     // Make sure the serialized contents have META ;
-    ASSERT_TRUE(HasSerializedFrame(file_url));
-    const std::string& serialized_contents =
-        GetSerializedContentForFrame(file_url);
+    const std::string& serialized_contents = GetSerializedContent();
 
     // Reload serialized contents and make sure there is only one META tag.
     LoadContents(serialized_contents, file_url,
@@ -749,14 +703,12 @@ class DomSerializerTests : public ContentBrowserTest,
 
  private:
   int32 render_view_routing_id_;
-  // Map frame_url to corresponding serialized_content.
-  typedef base::hash_map<std::string, std::string> SerializedFrameContentMap;
-  SerializedFrameContentMap serialized_frame_map_;
+  std::string serialization_output_;
   // Map frame_url to corresponding status of serialization finish.
   typedef base::hash_map<std::string, bool> SerializationFinishStatusMap;
   SerializationFinishStatusMap serialization_finish_status_;
   // Flag indicates whether the process of serializing DOM is finished or not.
-  bool serialized_;
+  bool got_end_of_frame_notification_;
   // The local_directory_name_ is dummy relative path of directory which
   // contain all saved auxiliary files included all sub frames and resources.
   const base::FilePath local_directory_name_;