Change [ConstructorCallWith={Document => ExecutionContext}]

Source/core/dom/Comment.h

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2003, 2009 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
@@ skipping 13 matching lines @@
 #define Comment_h
 
 #include "core/dom/CharacterData.h"
 
 namespace WebCore {
 
 class ExecutionContext;
 
 class Comment FINAL : public CharacterData {
 public:
     static PassRefPtrWillBeRawPtr<Comment> create(Document&, const String&);

[haraken, 2014/06/18 10:49:21]

Would it be possible to drop this constructor? It doesn't make sense to have
both Document version and ExecutionContext version.

The same comment for other parts in this CL.

[Jens Widell, 2014/06/18 11:00:02]

All cases where I overloaded instead of just changing the argument type of an
existing function were cases where the existing function is used from
(non-generated) C++ code.

Naturally, all these create() functions that take a Document argument, create
objects that are connected to a document. When a constructor is called from a
script, a (single / the primary) document can be implied from the scope, but in
other situations, that might not be the right document to use.

For instance, Document::createComment() passes in *this, which might not be what
toDocument() returns for any execution context.

So I'd say that Comment, Text, DocumentFragment and Range at least need their
::create(Document&, ...) overloads for correctness.  That's not saying there
aren't calls to those create()s that couldn't be changed to use the
::create(ExecutionContext*, ...) variant instead, but it doesn't seem
worth-while to me to analyze them all and change them.

WDYT?

[haraken, 2014/06/18 11:16:34]

hmm, I cannot come up with any case where we cannot replace Document& with
ExecutionContext*.
I'm sure it's guaranteed that toDocument(executionContext) returns the same
Document as |this|. In this case, Document::createComment() can pass in |this|,
and catch it with Comment::create(ExecutionContext*).

My point is that it wouldn't make sense to have both Document version and
ExecutionContext version, because these two are always the same thing. (So I
don't think you need to investigate the correctness one by one.)

+    static PassRefPtrWillBeRawPtr<Comment> create(ExecutionContext*, const String&);
 
 private:
     Comment(Document&, const String&);
 
     virtual String nodeName() const OVERRIDE;
     virtual NodeType nodeType() const OVERRIDE;
     virtual PassRefPtrWillBeRawPtr<Node> cloneNode(bool deep = true) OVERRIDE;
 };
 
 DEFINE_NODE_TYPE_CASTS(Comment, nodeType() == Node::COMMENT_NODE);
 
 } // namespace WebCore
 
 #endif // Comment_h