Make sure named item cache is always valid HTMLCollection::updateIdNameCache()

Make sure named item cache is always valid HTMLCollection::updateIdNameCache()

Make sure named item cache is always valid HTMLCollection::updateIdNameCache().
The method was previously creating and setting the cache map first and then
traverse the DOM tree to populate it. However, finding matches in the DOM tree
can cause HTMLCollection::invalidateCache() to be called under certain
circumstances. Therefore, this patch updates updateIdNameCache() so that the
cache map is set *after* traversing the DOM tree.

BUG=360474
R=jochen@chromium.org, eseidel@chromium.org
TEST=fast/dom/htmlcollection-selectedOptions-namedItem-crash.html

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

[eseidel, 2014-04-08 02:12:54]

lgtm.  Does this affect WebKit?  (perhaps we should report it to them).

https://codereview.chromium.org/225023034/diff/20001/Source/core/html/HTMLCol...
File Source/core/html/HTMLCollection.h (right):

https://codereview.chromium.org/225023034/diff/20001/Source/core/html/HTMLCol...
Source/core/html/HTMLCollection.h:81: NamedItemCache() { }
Best to put all functions in the .cpp file when possible.  Just less code for
the linker to need to de-dupe.

[Inactive, 2014-04-08 02:21:08]

Yes, this likely affects WebKit as well although I haven't verified yet. I'll
verify soon and report the issue if necessary.

https://codereview.chromium.org/225023034/diff/20001/Source/core/html/HTMLCol...
File Source/core/html/HTMLCollection.h (right):

https://codereview.chromium.org/225023034/diff/20001/Source/core/html/HTMLCol...
Source/core/html/HTMLCollection.h:81: NamedItemCache() { }
On 2014/04/08 02:12:55, eseidel wrote:
> Best to put all functions in the .cpp file when possible.  Just less code for
> the linker to need to de-dupe.

Done.

[Inactive, 2014-04-08 02:22:12]

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

[commit-bot: I haz the power, 2014-04-08 02:22:18]

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

[commit-bot: I haz the power, 2014-04-08 02:45:49]

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

[commit-bot: I haz the power, 2014-04-08 02:45:50]

Try jobs failed on following builders:
  tryserver.blink on win_blink_rel

[Inactive, 2014-04-08 03:03:21]

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

[commit-bot: I haz the power, 2014-04-08 03:03:27]

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

[commit-bot: I haz the power, 2014-04-08 04:38:07]

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

[commit-bot: I haz the power, 2014-04-08 04:38:08]

Try jobs failed on following builders:
  tryserver.blink on win_blink_rel

[jochen (gone - plz use gerrit), 2014-04-08 07:30:24]

lgtm

does WebKit have the same issue? Can you contribute the fix back?

[Inactive, 2014-04-08 14:40:22]

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

[commit-bot: I haz the power, 2014-04-08 14:40:31]

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

[commit-bot: I haz the power, 2014-04-08 14:40:56]

Change committed as 171062

[Inactive, 2014-05-05 19:39:04]

On 2014/04/08 07:30:24, jochen wrote:
> lgtm
> 
> does WebKit have the same issue? Can you contribute the fix back?

I have confirmed that WebKit has the same issue and I am going to contribute the
patch at:
https://bugs.webkit.org/show_bug.cgi?id=132575