drive: Add showroot=true to WAPI feed URLs and ignore "no parent" entries.

chrome/browser/chromeos/drive/drive_resource_metadata.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/drive/drive_resource_metadata.h"
 
 #include <leveldb/db.h>
 #include <stack>
 #include <utility>
 
@@ skipping 497 matching lines @@
 
   scoped_ptr<DriveEntry> drive_entry = CreateDriveEntryFromProto(entry_proto);
   if (!drive_entry.get()) {
     PostGetEntryInfoWithFilePathCallbackError(
         callback, DRIVE_FILE_ERROR_FAILED);
     return;
   }
 
   DriveEntry* old_entry = GetEntryByResourceId(drive_entry->resource_id());
   DriveDirectory* old_parent = NULL;
+  DriveDirectory* new_parent = NULL;
   if (old_entry && !old_entry->parent_resource_id().empty()) {

[hashimoto, 2013/03/11 09:32:17]

Can't this parent_resource_id().empty() check be replaced with old_entry !=
root_.get()?

[Haruki Sato, 2013/03/11 14:15:35]

Yes, assuming all non-root entry has parent_resource_id.
Revised.

     old_parent = GetEntryByResourceId(
         old_entry->parent_resource_id())->AsDriveDirectory();
+    new_parent = entry_proto.parent_resource_id().empty() ? NULL :
+        GetParent(entry_proto.parent_resource_id());

[hashimoto, 2013/03/11 09:32:17]

Why do you allow entry_proto.parent_resource_id() to be empty here?
How about checking it in the if-statement above?

[Haruki Sato, 2013/03/11 14:15:35]

Done. And we can simplify it. Thanks.

   }
-  DriveDirectory* new_parent = GetParent(entry_proto.parent_resource_id());
 
   scoped_ptr<DriveEntryProto> result_entry_proto(new DriveEntryProto);
   // We special case root here because old_parent of root is null.
+  // Note that this also catches "old_entry == NULL" case.
   if ((!old_parent || !new_parent) && old_entry != root_.get()) {
     drive_entry->ToProtoFull(result_entry_proto.get());
     base::MessageLoopProxy::current()->PostTask(
         FROM_HERE,
         base::Bind(callback,
                    DRIVE_FILE_ERROR_NOT_FOUND,
                    base::FilePath(),
                    base::Passed(&result_entry_proto)));
     return;
   }
@@ skipping 78 matching lines @@
     return;
   }
 
   DriveEntry* added_entry = new_entry.release();
   parent->AddEntry(added_entry);  // Transfers ownership.
   DVLOG(1) << "AddEntry "  << added_entry->GetFilePath().value();
   base::MessageLoopProxy::current()->PostTask(FROM_HERE,
       base::Bind(callback, DRIVE_FILE_OK, added_entry->GetFilePath()));
 }
 
 DriveDirectory* DriveResourceMetadata::GetParent(

[hashimoto, 2013/03/11 09:32:17]

Could you rename this method to GetDirectory() while you are here?
I think the name "GetParent" is quite confusing.

[Haruki Sato, 2013/03/11 14:15:35]

Done.

     const std::string& parent_resource_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  if (parent_resource_id.empty())
-    return root_.get();
+  DCHECK(!parent_resource_id.empty());
 
   DriveEntry* entry = GetEntryByResourceId(parent_resource_id);
   return entry ? entry->AsDriveDirectory() : NULL;
 }
 
 void DriveResourceMetadata::GetChildDirectories(
     const std::string& resource_id,
     const GetChildDirectoriesCallback& changed_dirs_callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!changed_dirs_callback.is_null());
@@ skipping 292 matching lines @@
   DCHECK(result.get());
 
   result->second.path = second_path;
   result->second.error = error;
   result->second.proto = entry_proto.Pass();
 
   callback.Run(result.Pass());
 }
 
 }  // namespace drive