Use new Traversal<> API more in the code base

Use new Traversal<> API more in the code base

Use new Traversal<> API more in the code base to simplify code a bit and
increase readability.

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

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

[adamk, 2014-07-28 21:06:38]

https://codereview.chromium.org/420193007/diff/1/Source/core/html/track/vtt/V...
File Source/core/html/track/vtt/VTTRegion.cpp (right):

https://codereview.chromium.org/420193007/diff/1/Source/core/html/track/vtt/V...
Source/core/html/track/vtt/VTTRegion.cpp:376: for (HTMLDivElement* child =
Traversal<HTMLDivElement>::firstChild(*m_cueContainer); child &&
!m_scrollTimer.isActive(); child =
Traversal<HTMLDivElement>::nextSibling(*child)) {
It's not clear to me why this code was calling toHTMLDivElement() before;
getBoundingClientRect() is on Element. It seems like it might be better off
sticking with ElementTraversal.

https://codereview.chromium.org/420193007/diff/1/Source/core/html/track/vtt/V...
Source/core/html/track/vtt/VTTRegion.cpp:378: float childBottom =
child->getBoundingClientRect()->bottom();
Sad that this code calls getBoundingClientRect() twice. While you're here, would
you mind storing the return value and accessing top and bottom from it?

[Inactive, 2014-07-28 21:13:34]

https://codereview.chromium.org/420193007/diff/1/Source/core/html/track/vtt/V...
File Source/core/html/track/vtt/VTTRegion.cpp (right):

https://codereview.chromium.org/420193007/diff/1/Source/core/html/track/vtt/V...
Source/core/html/track/vtt/VTTRegion.cpp:376: for (HTMLDivElement* child =
Traversal<HTMLDivElement>::firstChild(*m_cueContainer); child &&
!m_scrollTimer.isActive(); child =
Traversal<HTMLDivElement>::nextSibling(*child)) {
On 2014/07/28 21:06:38, adamk wrote:
> It's not clear to me why this code was calling toHTMLDivElement() before;
> getBoundingClientRect() is on Element. It seems like it might be better off
> sticking with ElementTraversal.

Yes, this looked unsafe. OK, I'll keep using ElementTraversal here and merely
get rid of the toHMTMLDivElement() cast then.

[Inactive, 2014-07-28 21:41:06]

https://codereview.chromium.org/420193007/diff/1/Source/core/html/track/vtt/V...
File Source/core/html/track/vtt/VTTRegion.cpp (right):

https://codereview.chromium.org/420193007/diff/1/Source/core/html/track/vtt/V...
Source/core/html/track/vtt/VTTRegion.cpp:376: for (HTMLDivElement* child =
Traversal<HTMLDivElement>::firstChild(*m_cueContainer); child &&
!m_scrollTimer.isActive(); child =
Traversal<HTMLDivElement>::nextSibling(*child)) {
On 2014/07/28 21:06:38, adamk wrote:
> It's not clear to me why this code was calling toHTMLDivElement() before;
> getBoundingClientRect() is on Element. It seems like it might be better off
> sticking with ElementTraversal.

Done.

https://codereview.chromium.org/420193007/diff/1/Source/core/html/track/vtt/V...
Source/core/html/track/vtt/VTTRegion.cpp:378: float childBottom =
child->getBoundingClientRect()->bottom();
On 2014/07/28 21:06:38, adamk wrote:
> Sad that this code calls getBoundingClientRect() twice. While you're here,
would
> you mind storing the return value and accessing top and bottom from it?

Done.

[adamk, 2014-07-28 21:42:04]

lgtm

[Inactive, 2014-07-28 21:44:12]

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

[commit-bot: I haz the power, 2014-07-28 21:45:18]

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

[commit-bot: I haz the power, 2014-07-28 23:08:13]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/17405)

[commit-bot: I haz the power, 2014-07-28 23:51:35]

Change committed as 179067