Refactor ScopedAXObjectCache to remove ref count

Refactor ScopedAXObjectCache to remove ref count

ScopedAXObjectCache doesn't need to be ref counted.

BUG=None

[dmazzoni, 2015-04-28 06:31:00]

dmazzoni@chromium.org changed reviewers:
+ dmazzoni@chromium.org

[dmazzoni, 2015-04-28 06:31:00]

lgtm

Thanks, that's a lot cleaner

[haraken, 2015-04-28 11:36:24]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org, oilpan-reviews@chromium.org

[haraken, 2015-04-28 11:36:25]

https://codereview.chromium.org/1088473002/diff/60001/Source/core/dom/AXObjec...
File Source/core/dom/AXObjectCache.h (right):

https://codereview.chromium.org/1088473002/diff/60001/Source/core/dom/AXObjec...
Source/core/dom/AXObjectCache.h:144: class CORE_EXPORT ScopedAXObjectCache {

I'm not quite sure how much this change helps Oilpan.

In Oilpan, we don't want to mix stack-allocated objects (allocated on stack) and
on-heap objects (allocated with new) for the same object type. That works fine
if you're careful but is a bit dangerous because the stack-allocated object does
not have a valid header (A disaster happens if you store the pointer to the
stack-allocated object to Member<X> and trace it). To avoid the danger, we don't
want to mix them.

Given that WebScopedAXContext needs to allocate the ScopedAXObjectCache object
on-heap, we don't want to allow stack-allocated ScopedAXObjectCache objects. So
I'd suggest making ScopedAXObjectCache NoBaseWillBeGarbageCollectedFinalized.
(In this CL, you can change ScopedAXObjectCache from RefCounted to NoBase.)

https://codereview.chromium.org/1088473002/diff/60001/Source/modules/accessib...
File Source/modules/accessibility/InspectorAccessibilityAgent.cpp (right):

https://codereview.chromium.org/1088473002/diff/60001/Source/modules/accessib...
Source/modules/accessibility/InspectorAccessibilityAgent.cpp:428: if
(!document.axObjectCache())

Why do we need an early return here? According to the implementation of
ScopedAXObjectCache, it seems we want to create a new cache if the document
doesn't have an axObjectCache.

[keishi, 2015-05-28 06:49:13]

https://codereview.chromium.org/1088473002/diff/60001/Source/core/dom/AXObjec...
File Source/core/dom/AXObjectCache.h (right):

https://codereview.chromium.org/1088473002/diff/60001/Source/core/dom/AXObjec...
Source/core/dom/AXObjectCache.h:144: class CORE_EXPORT ScopedAXObjectCache {
On 2015/04/28 11:36:25, haraken wrote:
> 
> I'm not quite sure how much this change helps Oilpan.
> 
> In Oilpan, we don't want to mix stack-allocated objects (allocated on stack)
and
> on-heap objects (allocated with new) for the same object type. That works fine
> if you're careful but is a bit dangerous because the stack-allocated object
does
> not have a valid header (A disaster happens if you store the pointer to the
> stack-allocated object to Member<X> and trace it). To avoid the danger, we
don't
> want to mix them.
> 
> Given that WebScopedAXContext needs to allocate the ScopedAXObjectCache object
> on-heap, we don't want to allow stack-allocated ScopedAXObjectCache objects.
So
> I'd suggest making ScopedAXObjectCache NoBaseWillBeGarbageCollectedFinalized.
> (In this CL, you can change ScopedAXObjectCache from RefCounted to NoBase.)
> 

Removed stack-allocated ScopedAXObjectCache.

https://codereview.chromium.org/1088473002/diff/60001/Source/modules/accessib...
File Source/modules/accessibility/InspectorAccessibilityAgent.cpp (right):

https://codereview.chromium.org/1088473002/diff/60001/Source/modules/accessib...
Source/modules/accessibility/InspectorAccessibilityAgent.cpp:428: if
(!document.axObjectCache())
On 2015/04/28 11:36:25, haraken wrote:
> 
> Why do we need an early return here? According to the implementation of
> ScopedAXObjectCache, it seems we want to create a new cache if the document
> doesn't have an axObjectCache.
> 

My mistake. Removed.

[haraken, 2015-05-28 08:50:08]

LGTM

https://codereview.chromium.org/1088473002/diff/100001/Source/core/dom/AXObje...
File Source/core/dom/AXObjectCache.cpp (right):

https://codereview.chromium.org/1088473002/diff/100001/Source/core/dom/AXObje...
Source/core/dom/AXObjectCache.cpp:63: : m_ownedCache(nullptr)

This wouldn't be necessary.

[keishi, 2015-05-29 05:05:05]

https://codereview.chromium.org/1088473002/diff/100001/Source/core/dom/AXObje...
File Source/core/dom/AXObjectCache.cpp (right):

https://codereview.chromium.org/1088473002/diff/100001/Source/core/dom/AXObje...
Source/core/dom/AXObjectCache.cpp:63: : m_ownedCache(nullptr)
On 2015/05/28 08:50:08, haraken wrote:
> 
> This wouldn't be necessary.

Done.

[keishi, 2015-05-29 05:05:12]

The CQ bit was checked by keishi@chromium.org

[keishi, 2015-05-29 05:05:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org, haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1088473002/#ps120001
(title: "removed m_ownedCache init")

[commit-bot: I haz the power, 2015-05-29 05:05:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1088473002/120001

[commit-bot: I haz the power, 2015-05-29 05:10:37]

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

[commit-bot: I haz the power, 2015-05-29 05:10:38]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/3...)