drive: Deal with the root directory in the same way as others in MoveOperation.

drive: Deal with the root directory same as others in MoveOperation.

Our current implementation of file moving is done in the following logic:
"if(source!=root) RemoveFrom(source); if(target!=root) AddTo(target)"
implicitly assuming "belonging to nowhere" means belonging to root.
But this is not the case.

This patch removes the "if !=root" checks and treats the root as same as other directories.

BUG=169420
TEST=Manually test the steps in the bug report.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=177997

[kinaba, 2013-01-21 08:21:34]

Satoru, could you take a look?

[kinaba, 2013-01-21 08:24:29]

https://codereview.chromium.org/12039005/diff/1/chrome/browser/chromeos/drive...
File chrome/browser/chromeos/drive/file_system/move_operation.cc (right):

https://codereview.chromium.org/12039005/diff/1/chrome/browser/chromeos/drive...
chrome/browser/chromeos/drive/file_system/move_operation.cc:91: //    which
effectively moves the file to the root directory.
A note:
Strictly speaking, this is false. Removing from its parent does not move the
file to the root; just make it orphaned (in drive.google.com and Drive clients
in Win/Mac). However, since all the rest of our code is treating orphaned files
as root files, this is still true in our code. In the future we should match it
up with other clients: crbug/171207.

[satorux1, 2013-01-22 06:51:50]

Thank you for working on this!

https://codereview.chromium.org/12039005/diff/1/chrome/browser/chromeos/drive...
File chrome/browser/chromeos/drive/file_system/move_operation.cc (right):

https://codereview.chromium.org/12039005/diff/1/chrome/browser/chromeos/drive...
chrome/browser/chromeos/drive/file_system/move_operation.cc:91: //    which
effectively moves the file to the root directory.
On 2013/01/21 08:24:29, kinaba wrote:
> A note:
> Strictly speaking, this is false. Removing from its parent does not move the
> file to the root; just make it orphaned (in http://drive.google.com and Drive
clients
> in Win/Mac). However, since all the rest of our code is treating orphaned
files
> as root files, this is still true in our code. In the future we should match
it
> up with other clients: crbug/171207.

Could you update the comment?

https://codereview.chromium.org/12039005/diff/1/chrome/browser/chromeos/drive...
File chrome/browser/chromeos/drive/file_system/move_operation.h (right):

https://codereview.chromium.org/12039005/diff/1/chrome/browser/chromeos/drive...
chrome/browser/chromeos/drive/file_system/move_operation.h:87: // it's not in
the root directory. This essentially moves an entry to the
"if it's not in the root directory. " => remove this part?

https://codereview.chromium.org/12039005/diff/1/chrome/browser/chromeos/drive...
chrome/browser/chromeos/drive/file_system/move_operation.h:102: // Moves a file
or directory at |file_path| in the root directory to
"in the root directory " => remove this part?

https://codereview.chromium.org/12039005/diff/1/chrome/browser/google_apis/gd...
File chrome/browser/google_apis/gdata_wapi_operations.cc (left):

https://codereview.chromium.org/12039005/diff/1/chrome/browser/google_apis/gd...
chrome/browser/google_apis/gdata_wapi_operations.cc:495: return
url_generator_.GenerateResourceListRootUrl();
Is GenerateResourceListRootUrl() still used somewhere else? Otherwise, please
remove this function.

[kinaba, 2013-01-22 07:09:10]

https://codereview.chromium.org/12039005/diff/1/chrome/browser/chromeos/drive...
File chrome/browser/chromeos/drive/file_system/move_operation.cc (right):

https://codereview.chromium.org/12039005/diff/1/chrome/browser/chromeos/drive...
chrome/browser/chromeos/drive/file_system/move_operation.cc:91: //    which
effectively moves the file to the root directory.
On 2013/01/22 06:51:50, satorux1 wrote:
> On 2013/01/21 08:24:29, kinaba wrote:
> > A note:
> > Strictly speaking, this is false. Removing from its parent does not move the
> > file to the root; just make it orphaned (in http://drive.google.com and
Drive
> clients
> > in Win/Mac). However, since all the rest of our code is treating orphaned
> files
> > as root files, this is still true in our code. In the future we should match
> it
> > up with other clients: crbug/171207.
> 
> Could you update the comment?

Done.

https://codereview.chromium.org/12039005/diff/1/chrome/browser/chromeos/drive...
File chrome/browser/chromeos/drive/file_system/move_operation.h (right):

https://codereview.chromium.org/12039005/diff/1/chrome/browser/chromeos/drive...
chrome/browser/chromeos/drive/file_system/move_operation.h:87: // it's not in
the root directory. This essentially moves an entry to the
On 2013/01/22 06:51:50, satorux1 wrote:
> "if it's not in the root directory. " => remove this part?

Done.

https://codereview.chromium.org/12039005/diff/1/chrome/browser/chromeos/drive...
chrome/browser/chromeos/drive/file_system/move_operation.h:102: // Moves a file
or directory at |file_path| in the root directory to
On 2013/01/22 06:51:50, satorux1 wrote:
> "in the root directory " => remove this part?

Done.

https://codereview.chromium.org/12039005/diff/1/chrome/browser/google_apis/gd...
File chrome/browser/google_apis/gdata_wapi_operations.cc (left):

https://codereview.chromium.org/12039005/diff/1/chrome/browser/google_apis/gd...
chrome/browser/google_apis/gdata_wapi_operations.cc:495: return
url_generator_.GenerateResourceListRootUrl();
On 2013/01/22 06:51:50, satorux1 wrote:
> Is GenerateResourceListRootUrl() still used somewhere else? Otherwise, please
> remove this function.

CreateDirectory and CopyHostedDocument still uses it.

[satorux1, 2013-01-22 07:13:26]

LGTM

[commit-bot: I haz the power, 2013-01-22 07:21:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinaba@chromium.org/12039005/2016

[commit-bot: I haz the power, 2013-01-22 10:42:01]

Change committed as 177997