Oilpan: Use weak pointers in StyleSheetContents caches.

Oilpan: Use weak pointers in StyleSheetContents caches.

This fixes use-after-free that could occur when StyleSheetContents
dies when an allocation happens during insertion of a new
StyleSheetContents. Using the built-in weak processing this will
not happen because the iterator used during insertion will keep
the StyleSheetContents alive.

R=erik.corry@gmail.com, haraken@chromium.org, vegorov@chromium.org
NOTRY=true

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

[Vyacheslav Egorov (Chromium), 2014-03-04 13:50:53]

lgtm

[haraken, 2014-03-04 13:55:32]

LGTM

[Mads Ager (chromium), 2014-03-04 13:56:18]

The CQ bit was checked by ager@chromium.org

[commit-bot: I haz the power, 2014-03-04 13:56:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ager@chromium.org/185403016/1

[zerny-chromium, 2014-03-04 13:59:45]

lgtm

https://codereview.chromium.org/185403016/diff/1/Source/core/dom/StyleEngine.cpp
File Source/core/dom/StyleEngine.cpp (right):

https://codereview.chromium.org/185403016/diff/1/Source/core/dom/StyleEngine....
Source/core/dom/StyleEngine.cpp:57: static WillBeHeapHashMap<AtomicString,
RawPtrWillBeWeakMember<StyleSheetContents> >& textToSheetCache()
Nit: use the typedef here too.

https://codereview.chromium.org/185403016/diff/1/Source/core/dom/StyleEngine....
Source/core/dom/StyleEngine.cpp:69: static
WillBeHeapHashMap<RawPtrWillBeWeakMember<StyleSheetContents>, AtomicString>&
sheetToTextCache()
nit: ditto.

[Mads Ager (chromium), 2014-03-04 14:03:33]

https://codereview.chromium.org/185403016/diff/1/Source/core/dom/StyleEngine.cpp
File Source/core/dom/StyleEngine.cpp (right):

https://codereview.chromium.org/185403016/diff/1/Source/core/dom/StyleEngine....
Source/core/dom/StyleEngine.cpp:57: static WillBeHeapHashMap<AtomicString,
RawPtrWillBeWeakMember<StyleSheetContents> >& textToSheetCache()
On 2014/03/04 13:59:46, zerny-chromium wrote:
> Nit: use the typedef here too.

The typedefs are local to the methods. And typedefs have proven to be a little
dangerous for us in the transition period because it is no longer completely
clear that there is something garbage collected going on. So I'd prefer not to.

[Erik Corry, 2014-03-04 14:09:27]

lgtm

https://codereview.chromium.org/185403016/diff/1/Source/core/dom/StyleEngine.cpp
File Source/core/dom/StyleEngine.cpp (right):

https://codereview.chromium.org/185403016/diff/1/Source/core/dom/StyleEngine....
Source/core/dom/StyleEngine.cpp:61:
DEFINE_STATIC_LOCAL(Persistent<TextToSheetCache>, cache, (new
TextToSheetCache));
I think in another change we could use WillBePersistentHeapHashMap which avoids
the cache vs. *cache difference.

[Mads Ager (chromium), 2014-03-04 14:13:36]

https://codereview.chromium.org/185403016/diff/1/Source/core/dom/StyleEngine.cpp
File Source/core/dom/StyleEngine.cpp (right):

https://codereview.chromium.org/185403016/diff/1/Source/core/dom/StyleEngine....
Source/core/dom/StyleEngine.cpp:61:
DEFINE_STATIC_LOCAL(Persistent<TextToSheetCache>, cache, (new
TextToSheetCache));
On 2014/03/04 14:09:27, Erik Corry wrote:
> I think in another change we could use WillBePersistentHeapHashMap which
avoids
> the cache vs. *cache difference.

Yeah, it would be nice if we could do that. Let's get this in and try to do that
for the 4 or 5 places where we use this pattern now.

[commit-bot: I haz the power, 2014-03-04 15:02:10]

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

[commit-bot: I haz the power, 2014-03-04 15:02:10]

Retried try job too often on linux_blink_rel for step(s) blink_heap_unittests,
blink_platform_unittests, webkit_lint, webkit_python_tests, webkit_tests,
webkit_unit_tests, wtf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[Mads Ager (chromium), 2014-03-04 15:19:48]

The CQ bit was checked by ager@chromium.org

[commit-bot: I haz the power, 2014-03-04 15:19:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ager@chromium.org/185403016/10001

[Mads Ager (chromium), 2014-03-05 07:50:17]

The CQ bit was unchecked by ager@chromium.org

[Mads Ager (chromium), 2014-03-05 07:50:40]

The CQ bit was checked by ager@chromium.org

[commit-bot: I haz the power, 2014-03-05 07:51:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ager@chromium.org/185403016/10001

[commit-bot: I haz the power, 2014-03-05 07:53:32]

Change committed as 168444

[Mads Ager (chromium), 2014-03-05 09:37:18]

A revert of this CL has been created in
https://codereview.chromium.org/187473003/ by ager@chromium.org.

The reason for reverting is: This patch seems to have made a number of layout
tests fail on the oilpan bots. Reverting while I investigate..