[Devtools] Moved DataGridNode's .parent property to proper getter/setter

[Devtools] Moved DataGridNode's .parent property to proper getter/setter

This patch is in prep for a future patch that will invalidate network
grid nodes if the parent of the node is changed it will need to
recalculate the depth of the node. As it stands now it caches the
._depth property, but we need to make sure it recalculates under any
case the parent changes.

R=caseq,luoe
BUG=None

[allada, 2017-02-25 00:26:18]

Description was changed from

==========
[Devtools] Moved DataGridNode's .parent property to proper getter/setter

This patch is in prep for a future patch that will invalidate network
grid nodes if the parent of the node is changed it will need to
recalculate the depth of the node. As it stands now it caches the
._depth property, but we need to make sure it recalculates under any
case the parent changes.

R=caseq,luoe
BUG=None
==========

to

==========
[Devtools] Moved DataGridNode's .parent property to proper getter/setter

This patch is in prep for a future patch that will invalidate network
grid nodes if the parent of the node is changed it will need to
recalculate the depth of the node. As it stands now it caches the
._depth property, but we need to make sure it recalculates under any
case the parent changes.

R=caseq,luoe
BUG=None
==========

[allada, 2017-02-25 00:26:18]

allada@chromium.org changed reviewers:
+ alph@chromium.org

[allada, 2017-02-25 00:26:27]

PTL

[allada, 2017-03-03 23:59:57]

ping

[luoe, 2017-03-06 16:56:40]

https://codereview.chromium.org/2716983002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/data_grid/DataGrid.js (right):

https://codereview.chromium.org/2716983002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/data_grid/DataGrid.js:1340:
this._parent = parentNode;
Do we need to recalculate depth of this node's grand/children as well?

https://codereview.chromium.org/2716983002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/data_grid/DataGrid.js:1640:
child._depth = undefined;
No longer need this since setParent deletes it?

[allada, 2017-03-06 18:18:32]

PTaL

https://codereview.chromium.org/2716983002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/data_grid/DataGrid.js (right):

https://codereview.chromium.org/2716983002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/data_grid/DataGrid.js:1340:
this._parent = parentNode;
On 2017/03/06 16:56:40, luoe wrote:
> Do we need to recalculate depth of this node's grand/children as well?

Done.

[allada, 2017-03-08 02:44:35]

friendly ping @ caseq.

[caseq, 2017-03-09 00:13:49]

So does this imply that current way of invalidating depth within insertChild()
does not work? Do we have any other ways to change parent that happen in bypass
of insert child?

[allada, 2017-03-09 00:38:13]

On 2017/03/09 00:13:49, caseq wrote:
> So does this imply that current way of invalidating depth within insertChild()
> does not work? Do we have any other ways to change parent that happen in
bypass
> of insert child?

The reason I did it this way is because _depth is a cache property and owned
privately
by DataGrid. However, ViewPortDataGrid overrides the insertChild() method
without calling
DataGrid's insertChild() and since it does not have access to the private
property it
cannot clear it's cache.

I can make a "parentChanged()" event that must be called, but I felt this is
very
straight-forward and cleaner. Although maybe making the method @protected might
be
worth it.

[caseq, 2017-03-09 00:49:12]

On 2017/03/09 00:38:13, allada wrote:
> 
> The reason I did it this way is because _depth is a cache property and owned
> privately
> by DataGrid. However, ViewPortDataGrid overrides the insertChild() method
> without calling
> DataGrid's insertChild() and since it does not have access to the private
> property it
> cannot clear it's cache.
> 
> I can make a "parentChanged()" event that must be called, but I felt this is
> very
> straight-forward and cleaner. Although maybe making the method @protected
might
> be
> worth it.

Ah, I see... Can we do it in a less invasive manner? For example, any parent
changes seem to be done through detaching/attaching the node, so perhaps we
could move _depth invalidation into DataGridNode._attach()?

[allada, 2017-03-09 01:41:20]

On 2017/03/09 00:49:12, caseq wrote:
> Ah, I see... Can we do it in a less invasive manner? For example, any parent
> changes seem to be done through detaching/attaching the node, so perhaps we
> could move _depth invalidation into DataGridNode._attach()?

Yes, I could and don't have any major issue doing so, I am simply trying to
slowly pay off technical debt. "parent" should not be changed outside one of
the extending classes (much like "_dataGrid") so normalizing this field while
I needed it changed anyway seemed like the more appropriate thing to do.

[caseq, 2017-03-13 20:31:02]

On 2017/03/09 01:41:20, allada wrote:
> On 2017/03/09 00:49:12, caseq wrote:
> > Ah, I see... Can we do it in a less invasive manner? For example, any parent
> > changes seem to be done through detaching/attaching the node, so perhaps we
> > could move _depth invalidation into DataGridNode._attach()?
> 
> Yes, I could and don't have any major issue doing so, I am simply trying to
> slowly pay off technical debt. "parent" should not be changed outside one of
> the extending classes (much like "_dataGrid") so normalizing this field while
> I needed it changed anyway seemed like the more appropriate thing to do.

As discussed offline, let's extract logical tree structure management (i.e.
next/prev/parent etc) into separate methods in DataGrid and make sure these are
re-used in derived classes.