Demistify DocumentParser::append

Source/core/dom/DocumentParser.h

Index: Source/core/dom/DocumentParser.h
diff --git a/Source/core/dom/DocumentParser.h b/Source/core/dom/DocumentParser.h
index 2863cb73eb4b33c234f49164834758cd82d74c79..693d796a3a938279b1837df3f50fd99df0f616d8 100644
--- a/Source/core/dom/DocumentParser.h
+++ b/Source/core/dom/DocumentParser.h
@@ -56,10 +56,7 @@ public:
     virtual TextResourceDecoder* decoder();
     virtual void setHasAppendedData() { }
 
-    // FIXME: append() should be private, but DocumentLoader::replaceDocumentWhileExecutingJavaScriptURL uses it for now.

[kouhei (in TOK), 2014/12/18 04:34:35]

Why should this FIXME be removed?

[kbalazs, 2014/12/18 23:28:28]

I answered that in the changelog :)

"Also remove outdated comment that it should not be public. Both DocumentWriter
and DOMPatchSupport uses it and there is nothing particularly wrong with that."

So first of all the comment says it's only used by DocumentWriter but in fact
it's used by DOMPatchsSupport as well. Second now that append is only used on
sync path I think there is nothing wrong with it.

[kouhei (in TOK), 2014/12/19 04:44:27]

I think this comment is still relevant. So would you keep it for this CL?

DocumentParser has 3 methods (appendBytes, append, insert) to add content, so
I'm a bit concerned about the current state. I know that each method does things
a bit differently, but I feel this is overly complicated and hard to understand.
DOMPatchSupport and DocumentWriter should really be using appendBytes here.

-    // FIXME: This really should take a PassOwnPtr to signify that it expects to take
-    // ownership of the buffer. The parser expects the PassRefPtr to hold the only ref of the StringImpl.

[kouhei (in TOK), 2014/12/18 04:34:35]

I'm ok with removing this FIXME.

-    virtual void append(PassRefPtr<StringImpl>) = 0;
+    virtual void append(const String&) = 0;
 
     virtual void finish() = 0;