Use stricter typing for NodeLists throughout the code base

Use stricter typing for NodeLists throughout the code base

Use stricter typing for NodeLists throughout the code base. This makes it
clearer what exact type we are actually dealing with. This also allows the
compiler to de-virtualize some function calls with C++11.

R=adamk@chromium.org, esprehn@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176427

[Inactive, 2014-06-16 13:55:18]

https://codereview.chromium.org/334713006/diff/40001/Source/core/dom/Document.h
File Source/core/dom/Document.h (left):

https://codereview.chromium.org/334713006/diff/40001/Source/core/dom/Document...
Source/core/dom/Document.h:348: PassRefPtrWillBeRawPtr<NodeList>
nodesFromRect(int centerX, int centerY,
Dead code.

https://codereview.chromium.org/334713006/diff/40001/Source/core/dom/Document...
Source/core/dom/Document.h:1159: PassRefPtrWillBeRawPtr<NodeList>
handleZeroPadding(const HitTestRequest&, HitTestResult&) const;
Dead code.

[adamk, 2014-06-16 20:23:17]

https://codereview.chromium.org/334713006/diff/40001/Source/core/accessibilit...
File Source/core/accessibility/AXTable.cpp (right):

https://codereview.chromium.org/334713006/diff/40001/Source/core/accessibilit...
Source/core/accessibility/AXTable.cpp:142: unsigned rowCount = rows->length();
Nit: this reads like over-optimizing to me, considering there's code in the
HTMLCollection infrastructure to make this cached already.

https://codereview.chromium.org/334713006/diff/40001/Source/core/dom/ParentNo...
File Source/core/dom/ParentNode.h (right):

https://codereview.chromium.org/334713006/diff/40001/Source/core/dom/ParentNo...
Source/core/dom/ParentNode.h:38: #include "core/dom/TagCollection.h"
Why are these additional #includes needed? I understand that ParentNode.h isn't
included many places, but adding more includes to a header unnecessarily is
generally likely to slow down compilation.

https://codereview.chromium.org/334713006/diff/40001/Source/core/dom/shadow/S...
File Source/core/dom/shadow/ShadowRoot.h (right):

https://codereview.chromium.org/334713006/diff/40001/Source/core/dom/shadow/S...
Source/core/dom/shadow/ShadowRoot.h:35: #include "core/dom/TagCollection.h"
Again, why are these includes needed? ShadowRoot.h has many more includers than
ParentNode.h

https://codereview.chromium.org/334713006/diff/40001/Source/core/html/HTMLTab...
File Source/core/html/HTMLTableElement.h (right):

https://codereview.chromium.org/334713006/diff/40001/Source/core/html/HTMLTab...
Source/core/html/HTMLTableElement.h:30: #include
"core/html/HTMLTableRowsCollection.h"
Again interested why the include was needed. Perhaps you can just give an
overview of all those changes? (not going to comment on any more #include
changes)

[Inactive, 2014-06-16 20:35:51]

https://codereview.chromium.org/334713006/diff/40001/Source/core/accessibilit...
File Source/core/accessibility/AXTable.cpp (right):

https://codereview.chromium.org/334713006/diff/40001/Source/core/accessibilit...
Source/core/accessibility/AXTable.cpp:142: unsigned rowCount = rows->length();
On 2014/06/16 20:23:17, adamk wrote:
> Nit: this reads like over-optimizing to me, considering there's code in the
> HTMLCollection infrastructure to make this cached already.

This avoids "if (isCachedNodeCountValid())" check for every loop iteration. It
is not uncommon that we cache HTMLCollection::length() before the loops.

https://codereview.chromium.org/334713006/diff/40001/Source/core/dom/ParentNo...
File Source/core/dom/ParentNode.h (right):

https://codereview.chromium.org/334713006/diff/40001/Source/core/dom/ParentNo...
Source/core/dom/ParentNode.h:38: #include "core/dom/TagCollection.h"
On 2014/06/16 20:23:17, adamk wrote:
> Why are these additional #includes needed? I understand that ParentNode.h
isn't
> included many places, but adding more includes to a header unnecessarily is
> generally likely to slow down compilation.

This is needed so that the generated V8Document.cpp compiles. The generated
V8Document.cpp currently includes V8NodeList.h which is no longer sufficient.
One common workaround for this is to include the necessary includes to the
implementation header (here Document.h). However, I cannot include these in
Document.h at this pointer because they include "Document.h" themselves and I
get a circular dependency. One solution I have found, for now, is to include
those here as V8Document.cpp include this header (Document implements
ParentNode). Ideally, we would have a way to tell the bindings generator to
include specific headers, e.g.:
[IncludeAs=TagCollection] HTMLCollection getElementsByTagName(DOMString
localName);

Another workaround would be to duplicate the methods on native side:
class TagCollection; // Forward declaration is enough.
TagCollection* getElementsByTagName(const String&);
HTMLCollection* getElementsByTagNameForBindings(const String&); // The one for
bindings needs to return the IDL type.

https://codereview.chromium.org/334713006/diff/40001/Source/core/dom/shadow/S...
File Source/core/dom/shadow/ShadowRoot.h (right):

https://codereview.chromium.org/334713006/diff/40001/Source/core/dom/shadow/S...
Source/core/dom/shadow/ShadowRoot.h:35: #include "core/dom/TagCollection.h"
On 2014/06/16 20:23:17, adamk wrote:
> Again, why are these includes needed? ShadowRoot.h has many more includers
than
> ParentNode.h

To make the generated V8ShadowRoot.cpp build. V8ShadowRoot.cpp does not include
those headers and therefore is not able to convert an ClassCollection* into an
HTMLCollection* (forward declaration is not sufficient for that).

https://codereview.chromium.org/334713006/diff/40001/Source/core/html/HTMLTab...
File Source/core/html/HTMLTableElement.h (right):

https://codereview.chromium.org/334713006/diff/40001/Source/core/html/HTMLTab...
Source/core/html/HTMLTableElement.h:30: #include
"core/html/HTMLTableRowsCollection.h"
On 2014/06/16 20:23:17, adamk wrote:
> Again interested why the include was needed. Perhaps you can just give an
> overview of all those changes? (not going to comment on any more #include
> changes)

Same reason. The V8TableElement.cpp wouldn't build otherwise for the reasons
explained earlier. I don't know of a better solution at the moment.
Alternatively I could add an additional IDL extended attribute, or add a
specific rowsForBindings() method that returns an HTMLCollection.

[Inactive, 2014-06-16 20:39:05]

+haraken/nbarth in case he has an opinion on how to fix the bindings build in
this case (see my earlier comment).

[haraken, 2014-06-17 01:40:20]

On 2014/06/16 20:39:05, Chris Dumez wrote:
> +haraken/nbarth in case he has an opinion on how to fix the bindings build in
> this case (see my earlier comment).

How about including TagCollection.h in V8NodeList.h? You can hard-code a list of
header files that should be included in V8NodeList.h in INCLUDES_FOR_TYPE in
v8_types.py.

[Inactive, 2014-06-17 13:37:35]

On 2014/06/17 01:40:20, haraken wrote:
> On 2014/06/16 20:39:05, Chris Dumez wrote:
> > +haraken/nbarth in case he has an opinion on how to fix the bindings build
in
> > this case (see my earlier comment).
> 
> How about including TagCollection.h in V8NodeList.h? You can hard-code a list
of
> header files that should be included in V8NodeList.h in INCLUDES_FOR_TYPE in
> v8_types.py.

Thanks Kentaro. I wasn't aware we had a "solution" for this in the bindings
generator. I updated the patch as suggested.

[adamk, 2014-06-17 18:03:07]

lgtm now that headers in core/ are unaffected.

[haraken, 2014-06-17 23:37:24]

LGTM for bindings/

[Inactive, 2014-06-18 02:03:26]

The CQ bit was checked by ch.dumez@samsung.com

[commit-bot: I haz the power, 2014-06-18 02:04:15]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/334713006/80001

[commit-bot: I haz the power, 2014-06-18 02:04:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-18 02:04:35]

Failed to apply patch for Source/core/inspector/InspectorCSSAgent.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/inspector/InspectorCSSAgent.cpp
  Hunk #1 FAILED at 47.
  1 out of 1 hunk FAILED -- saving rejects to file
Source/core/inspector/InspectorCSSAgent.cpp.rej

Patch:       Source/core/inspector/InspectorCSSAgent.cpp
Index: Source/core/inspector/InspectorCSSAgent.cpp
diff --git a/Source/core/inspector/InspectorCSSAgent.cpp
b/Source/core/inspector/InspectorCSSAgent.cpp
index
ae6ebd82298d9e159a55f95a230fbda1bdebd1cb..a1c0c51a079aab442b3676062a9e146c3a6a6e73
100644
--- a/Source/core/inspector/InspectorCSSAgent.cpp
+++ b/Source/core/inspector/InspectorCSSAgent.cpp
@@ -47,7 +47,10 @@
 #include "core/css/StyleSheetList.h"
 #include "core/css/resolver/StyleResolver.h"
 #include "core/dom/Node.h"
-#include "core/dom/NodeList.h"
+#include "core/fetch/CSSStyleSheetResource.h"
+#include "core/fetch/ResourceClient.h"
+#include "core/fetch/ResourceFetcher.h"
+#include "core/fetch/StyleSheetResourceClient.h"
 #include "core/frame/LocalFrame.h"
 #include "core/html/HTMLHeadElement.h"
 #include "core/html/VoidCallback.h"

[Inactive, 2014-06-18 12:39:39]

The CQ bit was checked by ch.dumez@samsung.com

[commit-bot: I haz the power, 2014-06-18 12:40:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/334713006/100001

[commit-bot: I haz the power, 2014-06-18 14:43:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-18 14:43:58]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[Inactive, 2014-06-18 14:46:57]

The CQ bit was checked by ch.dumez@samsung.com

[commit-bot: I haz the power, 2014-06-18 14:47:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/334713006/100001

[commit-bot: I haz the power, 2014-06-18 15:51:05]

Change committed as 176427