Fix gn meta to handle non-string metadata

Before this patch, 'gn meta' will fail if the meta data is not a
string. This patch fixes this problem. This patch also fixes the
problem that rebase is invoked even if '-rebase-files ' flag is
not used.

Bug: crbug.com/gn/67
Change-Id: I0890b5dec5e7331c69c055fc8f13fe81dedbce83
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/4660
Reviewed-by: Julie Hockett <juliehockett@google.com>
Commit-Queue: Haowei Wu <haowei@google.com>
diff --git a/tools/gn/command_meta.cc b/tools/gn/command_meta.cc
index 0603731..bb689e9 100644
--- a/tools/gn/command_meta.cc
+++ b/tools/gn/command_meta.cc
@@ -114,8 +114,15 @@
       walk_keys_str, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
   Err err;
   std::set<const Target*> targets_walked;
+  SourceDir rebase_source_dir(rebase_dir);
+  // When SourceDir constructor is supplied with an empty string,
+  // a trailing slash will be added. This prevent SourceDir::is_null()
+  // from returning true. Explicitly remove this traling slash here.
+  if (rebase_dir.empty()) {
+    rebase_source_dir = SourceDir();
+  }
   std::vector<Value> result =
-      WalkMetadata(targets, data_keys, walk_keys, SourceDir(rebase_dir),
+      WalkMetadata(targets, data_keys, walk_keys, rebase_source_dir,
                    &targets_walked, &err);
   if (err.has_error()) {
     err.PrintToStdout();
diff --git a/tools/gn/metadata.cc b/tools/gn/metadata.cc
index 3250b03..5091645 100644
--- a/tools/gn/metadata.cc
+++ b/tools/gn/metadata.cc
@@ -28,16 +28,11 @@
 
     if (!rebase_dir.is_null()) {
       for (const auto& val : iter->second.list_value()) {
-        if (!val.VerifyTypeIs(Value::STRING, err))
+        std::pair<Value, bool> pair =
+            this->RebaseValue(settings, rebase_dir, val, err);
+        if (!pair.second)
           return false;
-        std::string filename = source_dir_.ResolveRelativeAs(
-            /*as_file = */ true, val, err, settings->root_path_utf8());
-        if (err->has_error())
-          return false;
-
-        result->emplace_back(
-            val.origin(),
-            RebasePath(filename, rebase_dir, settings->root_path_utf8()));
+        result->push_back(pair.first);
       }
     } else {
       result->insert(result->end(), iter->second.list_value().begin(),
@@ -67,3 +62,73 @@
 
   return true;
 }
+
+std::pair<Value, bool> Metadata::RebaseValue(const BuildSettings* settings,
+                                             const SourceDir& rebase_dir,
+                                             const Value& value,
+                                             Err* err) const {
+  switch (value.type()) {
+    case Value::STRING:
+      return this->RebaseStringValue(settings, rebase_dir, value, err);
+    case Value::LIST:
+      return this->RebaseListValue(settings, rebase_dir, value, err);
+    case Value::SCOPE:
+      return this->RebaseScopeValue(settings, rebase_dir, value, err);
+    default:
+      return std::make_pair(value, true);
+  }
+}
+
+std::pair<Value, bool> Metadata::RebaseStringValue(
+    const BuildSettings* settings,
+    const SourceDir& rebase_dir,
+    const Value& value,
+    Err* err) const {
+  if (!value.VerifyTypeIs(Value::STRING, err))
+    return std::make_pair(value, false);
+  std::string filename = source_dir_.ResolveRelativeAs(
+      /*as_file = */ true, value, err, settings->root_path_utf8());
+  if (err->has_error())
+    return std::make_pair(value, false);
+  Value rebased_value(value.origin(), RebasePath(filename, rebase_dir,
+                                                 settings->root_path_utf8()));
+  return std::make_pair(rebased_value, true);
+}
+
+std::pair<Value, bool> Metadata::RebaseListValue(const BuildSettings* settings,
+                                                 const SourceDir& rebase_dir,
+                                                 const Value& value,
+                                                 Err* err) const {
+  if (!value.VerifyTypeIs(Value::LIST, err))
+    return std::make_pair(value, false);
+
+  Value rebased_list_value(value.origin(), Value::LIST);
+  for (auto& val : value.list_value()) {
+    std::pair<Value, bool> pair = RebaseValue(settings, rebase_dir, val, err);
+    if (!pair.second)
+      return std::make_pair(value, false);
+    rebased_list_value.list_value().push_back(pair.first);
+  }
+  return std::make_pair(rebased_list_value, true);
+}
+
+std::pair<Value, bool> Metadata::RebaseScopeValue(const BuildSettings* settings,
+                                                  const SourceDir& rebase_dir,
+                                                  const Value& value,
+                                                  Err* err) const {
+  if (!value.VerifyTypeIs(Value::SCOPE, err))
+    return std::make_pair(value, false);
+  Value rebased_scope_value(value);
+  Scope::KeyValueMap scope_values;
+  value.scope_value()->GetCurrentScopeValues(&scope_values);
+  for (auto& value_pair : scope_values) {
+    std::pair<Value, bool> pair =
+        RebaseValue(settings, rebase_dir, value_pair.second, err);
+    if (!pair.second)
+      return std::make_pair(value, false);
+
+    rebased_scope_value.scope_value()->SetValue(value_pair.first, pair.first,
+                                                value.origin());
+  }
+  return std::make_pair(rebased_scope_value, true);
+}
\ No newline at end of file
diff --git a/tools/gn/metadata.h b/tools/gn/metadata.h
index da79fdf..3c16d27 100644
--- a/tools/gn/metadata.h
+++ b/tools/gn/metadata.h
@@ -63,6 +63,26 @@
   Contents contents_;
   SourceDir source_dir_;
 
+  std::pair<Value, bool> RebaseValue(const BuildSettings* settings,
+                                     const SourceDir& rebase_dir,
+                                     const Value& value,
+                                     Err* err) const;
+
+  std::pair<Value, bool> RebaseStringValue(const BuildSettings* settings,
+                                           const SourceDir& rebase_dir,
+                                           const Value& value,
+                                           Err* err) const;
+
+  std::pair<Value, bool> RebaseListValue(const BuildSettings* settings,
+                                         const SourceDir& rebase_dir,
+                                         const Value& value,
+                                         Err* err) const;
+
+  std::pair<Value, bool> RebaseScopeValue(const BuildSettings* settings,
+                                          const SourceDir& rebase_dir,
+                                          const Value& value,
+                                          Err* err) const;
+
   DISALLOW_COPY_AND_ASSIGN(Metadata);
 };
 
diff --git a/tools/gn/metadata_unittest.cc b/tools/gn/metadata_unittest.cc
index b159372..901e539 100644
--- a/tools/gn/metadata_unittest.cc
+++ b/tools/gn/metadata_unittest.cc
@@ -99,29 +99,58 @@
   EXPECT_EQ(results, expected);
 }
 
-TEST(MetadataTest, WalkWithRebaseError) {
+TEST(MetadataTest, WalkWithRebaseNonString) {
   TestWithScope setup;
   Metadata metadata;
   metadata.set_source_dir(SourceDir("/usr/home/files/"));
 
-  Value a_expected(nullptr, Value::LIST);
-  a_expected.list_value().push_back(Value(nullptr, "foo.cpp"));
-  a_expected.list_value().push_back(Value(nullptr, true));
+  Value a(nullptr, Value::LIST);
+  Value inner_list(nullptr, Value::LIST);
+  Value inner_scope(nullptr, Value::SCOPE);
+  inner_list.list_value().push_back(Value(nullptr, "foo.cpp"));
+  inner_list.list_value().push_back(Value(nullptr, "foo/bar.h"));
+  a.list_value().push_back(inner_list);
 
-  metadata.contents().insert(
-      std::pair<base::StringPiece, Value>("a", a_expected));
+  std::unique_ptr<Scope> scope(new Scope(setup.settings()));
+  scope->SetValue("a1", Value(nullptr, "foo2.cpp"), nullptr);
+  scope->SetValue("a2", Value(nullptr, "foo/bar2.h"), nullptr);
+  inner_scope.SetScopeValue(std::move(scope));
+  a.list_value().push_back(inner_scope);
 
+  metadata.contents().insert(std::pair<base::StringPiece, Value>("a", a));
   std::vector<std::string> data_keys;
   data_keys.emplace_back("a");
   std::vector<std::string> walk_keys;
   std::vector<Value> next_walk_keys;
   std::vector<Value> results;
 
+  std::vector<Value> expected;
+  Value inner_list_expected(nullptr, Value::LIST);
+  Value inner_scope_expected(nullptr, Value::SCOPE);
+  inner_list_expected.list_value().push_back(
+      Value(nullptr, "../home/files/foo.cpp"));
+  inner_list_expected.list_value().push_back(
+      Value(nullptr, "../home/files/foo/bar.h"));
+  expected.push_back(inner_list_expected);
+
+  std::unique_ptr<Scope> scope_expected(new Scope(setup.settings()));
+  scope_expected->SetValue("a1", Value(nullptr, "../home/files/foo2.cpp"),
+                           nullptr);
+  scope_expected->SetValue("a2", Value(nullptr, "../home/files/foo/bar2.h"),
+                           nullptr);
+  inner_scope_expected.SetScopeValue(std::move(scope_expected));
+  expected.push_back(inner_scope_expected);
+
+  std::vector<Value> expected_walk_keys;
+  expected_walk_keys.emplace_back(nullptr, "");
+
   Err err;
-  EXPECT_FALSE(metadata.WalkStep(setup.settings()->build_settings(), data_keys,
-                                 walk_keys, SourceDir("/foo_dir/"),
-                                 &next_walk_keys, &results, &err));
-  EXPECT_TRUE(err.has_error());
+  EXPECT_TRUE(metadata.WalkStep(setup.settings()->build_settings(), data_keys,
+                                walk_keys, SourceDir("/usr/foo_dir/"),
+                                &next_walk_keys, &results, &err));
+  EXPECT_FALSE(err.has_error());
+  EXPECT_EQ(next_walk_keys, expected_walk_keys);
+  EXPECT_EQ(results, expected);
 }
 
 TEST(MetadataTest, WalkKeysToWalk) {