Oilpan: have xml/ objects on the heap by default.

Oilpan: have xml/ objects on the heap by default.

Along with moving the XPath objects (and other objects handling
XML processing) to the heap, we're able to retire the XPath
parser's manual tracking of currently constructed XPath
sub-expressions; done so as to be able to destruct those in the
face of parse errors.

With Oilpan, such unreferenced sub-expressions will be unreferenced
and swept out whenever the next GC runs.

R=haraken,tkent
BUG=479467
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=194504

[sof, 2015-04-26 14:19:41]

sigbjornf@opera.com changed reviewers:
+ oilpan-reviews@chromium.org

[sof, 2015-04-26 14:19:41]

please take a look.

[haraken, 2015-04-26 15:36:22]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org, tkent@chromium.org

[haraken, 2015-04-26 15:36:23]

LGTM

- Let's cc relevant people when enabling oilpan for something.

- Let's update the spreadsheet to keep track of our progress 
(https://docs.google.com/a/chromium.org/spreadsheets/d/1sRRGrJ0oqbcwzlGGUAjo2W...).
Feel free to add/remove components.

https://codereview.chromium.org/1099613003/diff/1/Source/bindings/scripts/v8_...
File Source/bindings/scripts/v8_types.py (right):

https://codereview.chromium.org/1099613003/diff/1/Source/bindings/scripts/v8_...
Source/bindings/scripts/v8_types.py:115: 'XPathNSResolver':
'RawPtr<XPathNSResolver>',

Doesn't XPathNSResolver* work?

https://codereview.chromium.org/1099613003/diff/1/Source/bindings/tests/resul...
File Source/bindings/tests/results/core/V8TestObject.cpp (right):

https://codereview.chromium.org/1099613003/diff/1/Source/bindings/tests/resul...
Source/bindings/tests/results/core/V8TestObject.cpp:7776:
RawPtr<XPathNSResolver> xPathNSResolverArg;

Ditto.

https://codereview.chromium.org/1099613003/diff/1/Source/core/xml/XPathParser.h
File Source/core/xml/XPathParser.h (left):

https://codereview.chromium.org/1099613003/diff/1/Source/core/xml/XPathParser...
Source/core/xml/XPathParser.h:92: void deleteNodeTest(Step::NodeTest*);

Nit: You could explain these removals in the CL description.

https://codereview.chromium.org/1099613003/diff/1/Source/core/xml/XPathResult.h
File Source/core/xml/XPathResult.h (right):

https://codereview.chromium.org/1099613003/diff/1/Source/core/xml/XPathResult...
Source/core/xml/XPathResult.h:45: // TODO(Oilpan): remove Finalized when
transition type for m_document is.

One thing we could do is to introduce
GarbageCollectedFinalizedWillBeGarbageCollected, but I'd prefer just having a
comment instead of introducing such a mysterious WillBe type.

[sof, 2015-04-26 15:41:26]

https://codereview.chromium.org/1099613003/diff/1/Source/core/xml/XPathParser.h
File Source/core/xml/XPathParser.h (left):

https://codereview.chromium.org/1099613003/diff/1/Source/core/xml/XPathParser...
Source/core/xml/XPathParser.h:92: void deleteNodeTest(Step::NodeTest*);
On 2015/04/26 15:36:23, haraken wrote:
> 
> Nit: You could explain these removals in the CL description.

Will do, the switch to Oilpan allows letting go of the register&delete machinery
the parser previously depended on.

[peria, 2015-04-26 16:30:06]

I'm happy that you ship Oilpan so quickly, but could you share where you're
working for with others to avoid duplicates?
Haraken shared a spreadsheet below, and I'm updating it.
https://docs.google.com/a/chromium.org/spreadsheets/d/1sRRGrJ0oqbcwzlGGUAjo2W...
Your works are so fast that I may fail to catch up with your progress. :)

[sof, 2015-04-26 16:34:36]

On 2015/04/26 16:30:06, peria wrote:
> I'm happy that you ship Oilpan so quickly, but could you share where you're
> working for with others to avoid duplicates?
> Haraken shared a spreadsheet below, and I'm updating it.
>
https://docs.google.com/a/chromium.org/spreadsheets/d/1sRRGrJ0oqbcwzlGGUAjo2W...
> Your works are so fast that I may fail to catch up with your progress. :)

I'm sorry if this has led to overlap -- just flushing my backlog in this case (
https://codereview.chromium.org/698923002/ )

Should I take on any of the ones remaining there, I'll make sure to update that
spreadsheet (which I wasn't aware of, as crbug.com/479467 doesn't refer to it.)

[peria, 2015-04-26 16:49:20]

On 2015/04/26 16:34:36, sof wrote:
> On 2015/04/26 16:30:06, peria wrote:
> > I'm happy that you ship Oilpan so quickly, but could you share where you're
> > working for with others to avoid duplicates?
> > Haraken shared a spreadsheet below, and I'm updating it.
> >
>
https://docs.google.com/a/chromium.org/spreadsheets/d/1sRRGrJ0oqbcwzlGGUAjo2W...
> > Your works are so fast that I may fail to catch up with your progress. :)
> 
> I'm sorry if this has led to overlap -- just flushing my backlog in this case
(
> https://codereview.chromium.org/698923002/ )
> 
> Should I take on any of the ones remaining there, I'll make sure to update
that
> spreadsheet (which I wasn't aware of, as crbug.com/479467 doesn't refer to
it.)
It was my failure to put the link in the issue. I'll do it soon.

Thank you for your cooperation, and for your quick works!!

[sof, 2015-04-26 21:23:40]

https://codereview.chromium.org/1099613003/diff/1/Source/bindings/scripts/v8_...
File Source/bindings/scripts/v8_types.py (right):

https://codereview.chromium.org/1099613003/diff/1/Source/bindings/scripts/v8_...
Source/bindings/scripts/v8_types.py:115: 'XPathNSResolver':
'RawPtr<XPathNSResolver>',
On 2015/04/26 15:36:23, haraken wrote:
> 
> Doesn't XPathNSResolver* work?

That's the type we want in the end.

But in the general case it runs up against the code generator's assumption that
a pointer to an interface type is something you can call release() on. I think
it will have to wait until that assumption is dropped -- added another
XPathNSResolver usage to TestObject.idl.

[sof, 2015-04-27 05:20:39]

tkent@: acceptable to flip the switch for xml/ ?

[tkent, 2015-04-27 08:08:35]

lgtm.

The parser simplification is cool :)

https://codereview.chromium.org/1099613003/diff/20001/Source/core/xml/XPathRe...
File Source/core/xml/XPathResult.h (left):

https://codereview.chromium.org/1099613003/diff/20001/Source/core/xml/XPathRe...
Source/core/xml/XPathResult.h:47:
DECLARE_EMPTY_DESTRUCTOR_WILL_BE_REMOVED(XPathResult);
Should we keep this macro?

[sof, 2015-04-27 09:13:04]

https://codereview.chromium.org/1099613003/diff/20001/Source/core/xml/XPathRe...
File Source/core/xml/XPathResult.h (left):

https://codereview.chromium.org/1099613003/diff/20001/Source/core/xml/XPathRe...
Source/core/xml/XPathResult.h:47:
DECLARE_EMPTY_DESTRUCTOR_WILL_BE_REMOVED(XPathResult);
On 2015/04/27 08:08:35, tkent wrote:
> Should we keep this macro?

Could do that, as the dtor will be trivial with ENABLE(OILPAN). Done.

[sof, 2015-04-27 09:43:15]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2015-04-27 09:43:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/1099613003/#ps40001
(title: "simplify XPathResult dtor")

[commit-bot: I haz the power, 2015-04-27 09:43:25]

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

[commit-bot: I haz the power, 2015-04-27 10:20:38]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194504