Remove fullPath from util.isSameEntry().

Remove fullPath from util.isSameEntry().

Full paths will not work after migrating to the separate file systems per volume. Therefore, for comparing urls should be used instead.

TEST=Tested manually, partly by browser_tests.
BUG=320967
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=244102

[mtomasz, 2014-01-09 01:39:37]

@hirono: PTAL at this tiny CL. Thanks.

[hirono, 2014-01-09 03:33:34]

On 2014/01/09 01:39:37, mtomasz wrote:
> @hirono: PTAL at this tiny CL. Thanks.

I'm not sure if FakeEntry should have URL or not.
For comparison, we can just extend isSameEntry without URL.
Please check where the URL is needed else?

[mtomasz, 2014-01-09 03:40:23]

On 2014/01/09 03:33:34, hirono wrote:
> On 2014/01/09 01:39:37, mtomasz wrote:
> > @hirono: PTAL at this tiny CL. Thanks.
> 
> I'm not sure if FakeEntry should have URL or not.
> For comparison, we can just extend isSameEntry without URL.
> Please check where the URL is needed else?

In general I'm not a fan of the idea of FakeEntry, but if we want it to mimic
Entry, then I don't think it is so bad to add a toURL(). Already the fullPath is
fake, so why not adding a fake url?

Do you see a better solution? We will have to compare fake entries for different
profiles. We could use location information, but then we would have to pass the
volume manager to isSameEntry and it would become quite heavy.

WDYT?

[hirono, 2014-01-09 03:54:11]

On 2014/01/09 03:40:23, mtomasz wrote:
> On 2014/01/09 03:33:34, hirono wrote:
> > On 2014/01/09 01:39:37, mtomasz wrote:
> > > @hirono: PTAL at this tiny CL. Thanks.
> > 
> > I'm not sure if FakeEntry should have URL or not.
> > For comparison, we can just extend isSameEntry without URL.
> > Please check where the URL is needed else?
> 
> In general I'm not a fan of the idea of FakeEntry, but if we want it to mimic
> Entry, then I don't think it is so bad to add a toURL(). Already the fullPath
is
> fake, so why not adding a fake url?
> 
> Do you see a better solution? We will have to compare fake entries for
different
> profiles. We could use location information, but then we would have to pass
the
> volume manager to isSameEntry and it would become quite heavy.
> 
> WDYT?

For comparison, we can compare FakeEntry by adding profile information to the
FakeEntry and comparing the profile and the root type.
The FakeEntry now is created by VolumeInfo, so adding the profile information to
the fake entry is easy.
And after introducing multi-profile, the private API driveSearch, which is used
for obtaining the contents of FakeEntry, will take profile specifying, thus it
is natural FakeEntry has profile information.

[mtomasz, 2014-01-10 01:48:46]

On 2014/01/09 03:54:11, hirono wrote:
> On 2014/01/09 03:40:23, mtomasz wrote:
> > On 2014/01/09 03:33:34, hirono wrote:
> > > On 2014/01/09 01:39:37, mtomasz wrote:
> > > > @hirono: PTAL at this tiny CL. Thanks.
> > > 
> > > I'm not sure if FakeEntry should have URL or not.
> > > For comparison, we can just extend isSameEntry without URL.
> > > Please check where the URL is needed else?
> > 
> > In general I'm not a fan of the idea of FakeEntry, but if we want it to
mimic
> > Entry, then I don't think it is so bad to add a toURL(). Already the
fullPath
> is
> > fake, so why not adding a fake url?
> > 
> > Do you see a better solution? We will have to compare fake entries for
> different
> > profiles. We could use location information, but then we would have to pass
> the
> > volume manager to isSameEntry and it would become quite heavy.
> > 
> > WDYT?
> 
> For comparison, we can compare FakeEntry by adding profile information to the
> FakeEntry and comparing the profile and the root type.
> The FakeEntry now is created by VolumeInfo, so adding the profile information
to
> the fake entry is easy.
> And after introducing multi-profile, the private API driveSearch, which is
used
> for obtaining the contents of FakeEntry, will take profile specifying, thus it
> is natural FakeEntry has profile information.

My concern is that DirectoryModel::getCurrentDirEntry() may return a fake entry.
And since we are moving from fullPath, then callers may want to access toURL().
I don't remember well what was our conclusion last time we discussed the fake
entries, but I think we should remove them as soon as possible, since they cause
problems like this over and over again. If you are ok with it, I'd like to hold
on with this CL and remove fake entries first.

[mtomasz, 2014-01-10 01:53:18]

On 2014/01/10 01:48:46, mtomasz wrote:
> On 2014/01/09 03:54:11, hirono wrote:
> > On 2014/01/09 03:40:23, mtomasz wrote:
> > > On 2014/01/09 03:33:34, hirono wrote:
> > > > On 2014/01/09 01:39:37, mtomasz wrote:
> > > > > @hirono: PTAL at this tiny CL. Thanks.
> > > > 
> > > > I'm not sure if FakeEntry should have URL or not.
> > > > For comparison, we can just extend isSameEntry without URL.
> > > > Please check where the URL is needed else?
> > > 
> > > In general I'm not a fan of the idea of FakeEntry, but if we want it to
> mimic
> > > Entry, then I don't think it is so bad to add a toURL(). Already the
> fullPath
> > is
> > > fake, so why not adding a fake url?
> > > 
> > > Do you see a better solution? We will have to compare fake entries for
> > different
> > > profiles. We could use location information, but then we would have to
pass
> > the
> > > volume manager to isSameEntry and it would become quite heavy.
> > > 
> > > WDYT?
> > 
> > For comparison, we can compare FakeEntry by adding profile information to
the
> > FakeEntry and comparing the profile and the root type.
> > The FakeEntry now is created by VolumeInfo, so adding the profile
information
> to
> > the fake entry is easy.
> > And after introducing multi-profile, the private API driveSearch, which is
> used
> > for obtaining the contents of FakeEntry, will take profile specifying, thus
it
> > is natural FakeEntry has profile information.
> 
> My concern is that DirectoryModel::getCurrentDirEntry() may return a fake
entry.
> And since we are moving from fullPath, then callers may want to access
toURL().
> I don't remember well what was our conclusion last time we discussed the fake
> entries, but I think we should remove them as soon as possible, since they
cause
> problems like this over and over again. If you are ok with it, I'd like to
hold
> on with this CL and remove fake entries first.

To clarify, since we are moving away from fullPath, toURL() is used often
instead, as a substitute to fullPath. This happens eg. in the NavigationList
being refactored.

[mtomasz, 2014-01-10 01:58:16]

On 2014/01/10 01:53:18, mtomasz wrote:
> On 2014/01/10 01:48:46, mtomasz wrote:
> > On 2014/01/09 03:54:11, hirono wrote:
> > > On 2014/01/09 03:40:23, mtomasz wrote:
> > > > On 2014/01/09 03:33:34, hirono wrote:
> > > > > On 2014/01/09 01:39:37, mtomasz wrote:
> > > > > > @hirono: PTAL at this tiny CL. Thanks.
> > > > > 
> > > > > I'm not sure if FakeEntry should have URL or not.
> > > > > For comparison, we can just extend isSameEntry without URL.
> > > > > Please check where the URL is needed else?
> > > > 
> > > > In general I'm not a fan of the idea of FakeEntry, but if we want it to
> > mimic
> > > > Entry, then I don't think it is so bad to add a toURL(). Already the
> > fullPath
> > > is
> > > > fake, so why not adding a fake url?
> > > > 
> > > > Do you see a better solution? We will have to compare fake entries for
> > > different
> > > > profiles. We could use location information, but then we would have to
> pass
> > > the
> > > > volume manager to isSameEntry and it would become quite heavy.
> > > > 
> > > > WDYT?
> > > 
> > > For comparison, we can compare FakeEntry by adding profile information to
> the
> > > FakeEntry and comparing the profile and the root type.
> > > The FakeEntry now is created by VolumeInfo, so adding the profile
> information
> > to
> > > the fake entry is easy.
> > > And after introducing multi-profile, the private API driveSearch, which is
> > used
> > > for obtaining the contents of FakeEntry, will take profile specifying,
thus
> it
> > > is natural FakeEntry has profile information.
> > 
> > My concern is that DirectoryModel::getCurrentDirEntry() may return a fake
> entry.
> > And since we are moving from fullPath, then callers may want to access
> toURL().
> > I don't remember well what was our conclusion last time we discussed the
fake
> > entries, but I think we should remove them as soon as possible, since they
> cause
> > problems like this over and over again. If you are ok with it, I'd like to
> hold
> > on with this CL and remove fake entries first.
> 
> To clarify, since we are moving away from fullPath, toURL() is used often
> instead, as a substitute to fullPath. This happens eg. in the NavigationList
> being refactored.

One more problem is that soon URL will be used in appState to restore from
crash. Currently we can restore to eg. Shared with me if it was opened before,
because we use fullPath. However, once we move to URL, we won't be able to
restore to fakeEntry simply unless we have a toURL() method on fake entries.

[hirono, 2014-01-10 03:36:52]

On 2014/01/10 01:58:16, mtomasz wrote:
> On 2014/01/10 01:53:18, mtomasz wrote:
> > On 2014/01/10 01:48:46, mtomasz wrote:
> > > On 2014/01/09 03:54:11, hirono wrote:
> > > > On 2014/01/09 03:40:23, mtomasz wrote:
> > > > > On 2014/01/09 03:33:34, hirono wrote:
> > > > > > On 2014/01/09 01:39:37, mtomasz wrote:
> > > > > > > @hirono: PTAL at this tiny CL. Thanks.
> > > > > > 
> > > > > > I'm not sure if FakeEntry should have URL or not.
> > > > > > For comparison, we can just extend isSameEntry without URL.
> > > > > > Please check where the URL is needed else?
> > > > > 
> > > > > In general I'm not a fan of the idea of FakeEntry, but if we want it
to
> > > mimic
> > > > > Entry, then I don't think it is so bad to add a toURL(). Already the
> > > fullPath
> > > > is
> > > > > fake, so why not adding a fake url?
> > > > > 
> > > > > Do you see a better solution? We will have to compare fake entries for
> > > > different
> > > > > profiles. We could use location information, but then we would have to
> > pass
> > > > the
> > > > > volume manager to isSameEntry and it would become quite heavy.
> > > > > 
> > > > > WDYT?
> > > > 
> > > > For comparison, we can compare FakeEntry by adding profile information
to
> > the
> > > > FakeEntry and comparing the profile and the root type.
> > > > The FakeEntry now is created by VolumeInfo, so adding the profile
> > information
> > > to
> > > > the fake entry is easy.
> > > > And after introducing multi-profile, the private API driveSearch, which
is
> > > used
> > > > for obtaining the contents of FakeEntry, will take profile specifying,
> thus
> > it
> > > > is natural FakeEntry has profile information.
> > > 
> > > My concern is that DirectoryModel::getCurrentDirEntry() may return a fake
> > entry.
> > > And since we are moving from fullPath, then callers may want to access
> > toURL().
> > > I don't remember well what was our conclusion last time we discussed the
> fake
> > > entries, but I think we should remove them as soon as possible, since they
> > cause
> > > problems like this over and over again. If you are ok with it, I'd like to
> > hold
> > > on with this CL and remove fake entries first.
> > 
> > To clarify, since we are moving away from fullPath, toURL() is used often
> > instead, as a substitute to fullPath. This happens eg. in the NavigationList
> > being refactored.
> 
> One more problem is that soon URL will be used in appState to restore from
> crash. Currently we can restore to eg. Shared with me if it was opened before,
> because we use fullPath. However, once we move to URL, we won't be able to
> restore to fakeEntry simply unless we have a toURL() method on fake entries.

lgtm!

After offline discussing, we should take the following steps.

1. Add toURL to fake entries.
2. Complete refactoring.
3. Introudce DirectoryContentProvider based on DirectoryEntry and
SpecialSearchContentProvider replacing FakeEntry.
   SpecialSearchContentProvider may not have toURL, but it is still needed to
support serialization.

[mtomasz, 2014-01-10 03:42:03]

On 2014/01/10 03:36:52, hirono wrote:
> On 2014/01/10 01:58:16, mtomasz wrote:
> > On 2014/01/10 01:53:18, mtomasz wrote:
> > > On 2014/01/10 01:48:46, mtomasz wrote:
> > > > On 2014/01/09 03:54:11, hirono wrote:
> > > > > On 2014/01/09 03:40:23, mtomasz wrote:
> > > > > > On 2014/01/09 03:33:34, hirono wrote:
> > > > > > > On 2014/01/09 01:39:37, mtomasz wrote:
> > > > > > > > @hirono: PTAL at this tiny CL. Thanks.
> > > > > > > 
> > > > > > > I'm not sure if FakeEntry should have URL or not.
> > > > > > > For comparison, we can just extend isSameEntry without URL.
> > > > > > > Please check where the URL is needed else?
> > > > > > 
> > > > > > In general I'm not a fan of the idea of FakeEntry, but if we want it
> to
> > > > mimic
> > > > > > Entry, then I don't think it is so bad to add a toURL(). Already the
> > > > fullPath
> > > > > is
> > > > > > fake, so why not adding a fake url?
> > > > > > 
> > > > > > Do you see a better solution? We will have to compare fake entries
for
> > > > > different
> > > > > > profiles. We could use location information, but then we would have
to
> > > pass
> > > > > the
> > > > > > volume manager to isSameEntry and it would become quite heavy.
> > > > > > 
> > > > > > WDYT?
> > > > > 
> > > > > For comparison, we can compare FakeEntry by adding profile information
> to
> > > the
> > > > > FakeEntry and comparing the profile and the root type.
> > > > > The FakeEntry now is created by VolumeInfo, so adding the profile
> > > information
> > > > to
> > > > > the fake entry is easy.
> > > > > And after introducing multi-profile, the private API driveSearch,
which
> is
> > > > used
> > > > > for obtaining the contents of FakeEntry, will take profile specifying,
> > thus
> > > it
> > > > > is natural FakeEntry has profile information.
> > > > 
> > > > My concern is that DirectoryModel::getCurrentDirEntry() may return a
fake
> > > entry.
> > > > And since we are moving from fullPath, then callers may want to access
> > > toURL().
> > > > I don't remember well what was our conclusion last time we discussed the
> > fake
> > > > entries, but I think we should remove them as soon as possible, since
they
> > > cause
> > > > problems like this over and over again. If you are ok with it, I'd like
to
> > > hold
> > > > on with this CL and remove fake entries first.
> > > 
> > > To clarify, since we are moving away from fullPath, toURL() is used often
> > > instead, as a substitute to fullPath. This happens eg. in the
NavigationList
> > > being refactored.
> > 
> > One more problem is that soon URL will be used in appState to restore from
> > crash. Currently we can restore to eg. Shared with me if it was opened
before,
> > because we use fullPath. However, once we move to URL, we won't be able to
> > restore to fakeEntry simply unless we have a toURL() method on fake entries.
> 
> lgtm!
> 
> After offline discussing, we should take the following steps.
> 
> 1. Add toURL to fake entries.
> 2. Complete refactoring.
> 3. Introudce DirectoryContentProvider based on DirectoryEntry and
> SpecialSearchContentProvider replacing FakeEntry.
>    SpecialSearchContentProvider may not have toURL, but it is still needed to
> support serialization.

Thanks! To clarify, we will make some kind of serialize() restore() in
DirectoryContentProvider. SpecialSearchContentProvider will not rely on url, and
therefore we will not need toURL() anymore.

[commit-bot: I haz the power, 2014-01-10 03:43:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mtomasz@chromium.org/130263003/30001

[commit-bot: I haz the power, 2014-01-10 06:09:02]

Change committed as 244102