Reland: "[Traffic Annotation] Remove function_context from extractor output"

This is a reland of 17ccf43e

This is basically a copy of the original CL, with merge conflicts resolved.

Now that t_a_auditor.exe defaults to using extractor.py instead of the
Clang tool (t_a_extractor.exe), this should be safer to land.

Original change's description:
> [Traffic Annotation] Remove function_context from extractor output
>
> The new extractor can't trivially determine the name of the enclosing
> function, and it didn't bring much value anyways.
>
> Bug: 966883
> Change-Id: I7b52050730186c6972754d3b9e577d4c4740d5dc
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1686621
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
> Commit-Queue: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#675223}

Bug: 966883
Change-Id: I93fa0385d44c9427f805248fac2af55fb128fe5c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1752065
Commit-Queue: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699395}

tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp

@@ -47,17 +47,6 @@ namespace {
 struct Location {
   std::string file_path;
   int line_number = -1;
-
-  // Name of the function including this line. E.g., in the following code,
-  // |function_name| will be 'foo' for all |line_number| values 101-103.
-  //
-  // 100 void foo() {
-  // 101   NetworkTrafficAnnotationTag baz =
-  // 102       net::DefineNetworkTrafficAnnotation(...); }
-  // 103   bar(baz);
-  // 104 }
-  // If no function is found, 'Global Namespace' will be returned.
-  std::string function_name;
 };
 
 // An instance of a call to either of the 4 network traffic annotation
@@ -165,13 +154,6 @@ class NetworkAnnotationTagCallback : public MatchFinder::MatchCallback {
     location->line_number =
         result.SourceManager->getSpellingLineNumber(source_location);
 
-    const clang::FunctionDecl* ancestor =
-        result.Nodes.getNodeAs<clang::FunctionDecl>("function_context");
-    if (ancestor)
-      location->function_name = ancestor->getQualifiedNameAsString();
-    else
-      location->function_name = "Global Namespace";
-
     std::replace(location->file_path.begin(), location->file_path.end(), '\\',
                  '/');
 
@@ -196,45 +178,20 @@ class NetworkAnnotationTagCallback : public MatchFinder::MatchCallback {
     collector_->calls.push_back(instance);
   }
 
-  // Tests if the given function name belongs to the network traffic annotation
-  // API. These functions are all defined in
-  // 'net/traffic_annotation/network_traffic_annotation.h'.
-  bool IsAPIFunction(const std::string& function_name) {
-    return function_name == "net::NetworkTrafficAnnotationTag::NotReached" ||
-           function_name == "net::DefineNetworkTrafficAnnotation" ||
-           function_name == "net::DefinePartialNetworkTrafficAnnotation" ||
-           function_name == "net::CompleteNetworkTrafficAnnotation" ||
-           function_name == "net::BranchedCompleteNetworkTrafficAnnotation" ||
-           function_name ==
-               "net::MutableNetworkTrafficAnnotationTag::operator "
-               "NetworkTrafficAnnotationTag" ||
-           function_name ==
-               "net::MutablePartialNetworkTrafficAnnotationTag::operator "
-               "PartialNetworkTrafficAnnotationTag";
-  }
-
   // Stores an annotation constructor called with list expression.
   void AddConstructor(const clang::CXXConstructExpr* constructor_expr,
                       const MatchFinder::MatchResult& result) {
     Location instance;
-
     GetInstanceLocation(result, constructor_expr, &instance);
-    // Only report if the constructor is not in one of the API functions for
-    // network traffic annotations.
-    if (!IsAPIFunction(instance.function_name))
-      collector_->assignments.push_back(instance);
+    collector_->assignments.push_back(instance);
   }
 
   // Stores a value assignment to |unique_id_hash_code| of a mutable annotaton.
   void AddAssignment(const clang::MemberExpr* member_expr,
                      const MatchFinder::MatchResult& result) {
     Location instance;
-
     GetInstanceLocation(result, member_expr, &instance);
-    // Only report if the assignment is not in one of the API functions for
-    // network traffic annotations.
-    if (!IsAPIFunction(instance.function_name))
-      collector_->assignments.push_back(instance);
+    collector_->assignments.push_back(instance);
   }
 
   // Stores an annotation.
@@ -414,7 +371,6 @@ int main(int argc, const char* argv[]) {
   for (const NetworkAnnotationInstance& instance : collector.annotations) {
     llvm::outs() << "==== NEW ANNOTATION ====\n";
     llvm::outs() << instance.location.file_path << "\n";
-    llvm::outs() << instance.location.function_name << "\n";
     llvm::outs() << instance.location.line_number << "\n";
     llvm::outs() << instance.GetTypeName() << "\n";
     llvm::outs() << instance.annotation.unique_id << "\n";
@@ -427,7 +383,6 @@ int main(int argc, const char* argv[]) {
   for (const CallInstance& instance : collector.calls) {
     llvm::outs() << "==== NEW CALL ====\n";
     llvm::outs() << instance.location.file_path << "\n";
-    llvm::outs() << instance.location.function_name << "\n";
     llvm::outs() << instance.location.line_number << "\n";
     llvm::outs() << instance.called_function_name << "\n";
     llvm::outs() << instance.has_annotation << "\n";
@@ -438,7 +393,6 @@ int main(int argc, const char* argv[]) {
   for (const Location& instance : collector.assignments) {
     llvm::outs() << "==== NEW ASSIGNMENT ====\n";
     llvm::outs() << instance.file_path << "\n";
-    llvm::outs() << instance.function_name << "\n";
     llvm::outs() << instance.line_number << "\n";
     llvm::outs() << "==== ASSIGNMENT ENDS ====\n";
   }

tools/traffic_annotation/auditor/BUILD.gn

@@ -97,8 +97,7 @@ test("traffic_annotation_auditor_unittests") {
     "tests/annotations_sample1.xml",
     "tests/annotations_sample2.xml",
     "tests/annotations_sample3.xml",
-    "tests/extractor_outputs/bad_assignment1.txt",
-    "tests/extractor_outputs/bad_assignment2.txt",
+    "tests/extractor_outputs/bad_assignment.txt",
     "tests/extractor_outputs/bad_call.txt",
     "tests/extractor_outputs/bad_syntax_annotation1.txt",
     "tests/extractor_outputs/bad_syntax_annotation2.txt",

tools/traffic_annotation/auditor/instance.cc

@@ -130,14 +130,13 @@ AuditorResult AnnotationInstance::Deserialize(
     const std::vector<std::string>& serialized_lines,
     int start_line,
     int end_line) {
-  if (end_line - start_line < 7) {
+  if (end_line - start_line < 6) {
     return AuditorResult(AuditorResult::Type::ERROR_FATAL,
                          "Not enough lines to deserialize annotation.");
   }
 
   // Extract header lines.
   const std::string& file_path = serialized_lines[start_line++];
-  const std::string& function_context = serialized_lines[start_line++];
   int line_number;
   base::StringToInt(serialized_lines[start_line++], &line_number);
   std::string function_type = serialized_lines[start_line++];
@@ -196,7 +195,6 @@ AuditorResult AnnotationInstance::Deserialize(
   traffic_annotation::NetworkTrafficAnnotation_TrafficSource* src =
       proto.mutable_source();
   src->set_file(file_path);
-  src->set_function(function_context);
   src->set_line(line_number);
   proto.set_unique_id(unique_id);
   second_id_hash_code = TrafficAnnotationAuditor::ComputeHashValue(second_id);
@@ -645,7 +643,6 @@ CallInstance::CallInstance() : line_number(0), is_annotated(false) {}
 CallInstance::CallInstance(const CallInstance& other)
     : file_path(other.file_path),
       line_number(other.line_number),
-      function_context(other.function_context),
       function_name(other.function_name),
       is_annotated(other.is_annotated) {}
 
@@ -653,13 +650,12 @@ AuditorResult CallInstance::Deserialize(
     const std::vector<std::string>& serialized_lines,
     int start_line,
     int end_line) {
-  if (end_line - start_line != 5) {
+  if (end_line - start_line != 4) {
     return AuditorResult(AuditorResult::Type::ERROR_FATAL,
                          "Not enough lines to deserialize call.");
   }
 
   file_path = serialized_lines[start_line++];
-  function_context = serialized_lines[start_line++];
   int line_number_int;
   base::StringToInt(serialized_lines[start_line++], &line_number_int);
   line_number = static_cast<uint32_t>(line_number_int);
@@ -673,20 +669,17 @@ AuditorResult CallInstance::Deserialize(
 AssignmentInstance::AssignmentInstance() : line_number(0) {}
 
 AssignmentInstance::AssignmentInstance(const AssignmentInstance& other)
-    : file_path(other.file_path),
-      line_number(other.line_number),
-      function_context(other.function_context) {}
+    : file_path(other.file_path), line_number(other.line_number) {}
 
 AuditorResult AssignmentInstance::Deserialize(
     const std::vector<std::string>& serialized_lines,
     int start_line,
     int end_line) {
-  if (end_line - start_line != 3) {
+  if (end_line - start_line != 2) {
     return AuditorResult(AuditorResult::Type::ERROR_FATAL,
                          "Not enough lines to deserialize assignment.");
   }
   file_path = serialized_lines[start_line++];
-  function_context = serialized_lines[start_line++];
   int line_number_int;
   base::StringToInt(serialized_lines[start_line++], &line_number_int);
   line_number = static_cast<uint32_t>(line_number_int);

tools/traffic_annotation/auditor/instance.h

@@ -156,9 +156,6 @@ class CallInstance : public InstanceBase {
   std::string file_path;
   uint32_t line_number;
 
-  // Name of the function in which the call happens.
-  std::string function_context;
-
   // Name of the function that may need annotation.
   std::string function_name;
 
@@ -187,9 +184,6 @@ class AssignmentInstance : public InstanceBase {
 
   std::string file_path;
   uint32_t line_number;
-
-  // Name of the function in which assignment happens.
-  std::string function_context;
 };
 
 #endif  // TOOLS_TRAFFIC_ANNOTATION_AUDITOR_INSTANCE_H_

tools/traffic_annotation/auditor/tests/extractor_outputs/bad_assignment.txt

+components/download/internal/proto_conversions.cc
\ No newline at end of file

tools/traffic_annotation/auditor/tests/extractor_outputs/bad_assignment1.txt

-components/download/internal/proto_conversions.cc
-267
\ No newline at end of file

tools/traffic_annotation/auditor/tests/extractor_outputs/bad_assignment2.txt

-components/download/internal/proto_conversions.cc
-download::ProtoConversions::EntryFromProto
\ No newline at end of file

tools/traffic_annotation/auditor/tests/extractor_outputs/bad_call.txt

 headless/public/util/http_url_fetcher.cc
-headless::HttpURLFetcher::Delegate::Delegate
 net::URLRequestContext::CreateRequest
 1
\ No newline at end of file

tools/traffic_annotation/auditor/tests/extractor_outputs/bad_syntax_annotation1.txt

 chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc
-OnGetTokenSuccess
 166
 Definition
 supervised_user_refresh_token_fetcher

tools/traffic_annotation/auditor/tests/extractor_outputs/bad_syntax_annotation2.txt

 chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc
-OnGetTokenSuccess
 166
 Definition
 supervised_user_refresh_token_fetcher

tools/traffic_annotation/auditor/tests/extractor_outputs/bad_syntax_annotation3.txt

 chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc
-OnGetTokenSuccess
 166
 Definition
 supervised_user_refresh_token_fetcher

tools/traffic_annotation/auditor/tests/extractor_outputs/bad_syntax_annotation4.txt

 chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc
-OnGetTokenSuccess
 166
 Definition
 supervised_user_refresh_token_fetcher

tools/traffic_annotation/auditor/tests/extractor_outputs/fatal_annotation1.txt

 chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc
-OnGetTokenSuccess
 Definition
 supervised_user_refresh_token_fetcher
 

tools/traffic_annotation/auditor/tests/extractor_outputs/fatal_annotation2.txt

 chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc
-OnGetTokenSuccess
 122
 definition
 supervised_user_refresh_token_fetcher

tools/traffic_annotation/auditor/tests/extractor_outputs/fatal_annotation3.txt

 chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc
-OnGetTokenSuccess
 122
 definition
 supervised_user_refresh_token_fetcher

tools/traffic_annotation/auditor/tests/extractor_outputs/good_assignment.txt

 components/download/internal/background_service/proto_conversions.cc
-download::ProtoConversions::EntryFromProto
 267
\ No newline at end of file

tools/traffic_annotation/auditor/tests/extractor_outputs/good_branched_completing_annotation.txt

 chrome/service/cloud_print/cloud_print_url_fetcher.cc
-cloud_print::CloudPrintURLFetcher::StartRequestHelper
 265
 BranchedCompleting
 cloud_print

tools/traffic_annotation/auditor/tests/extractor_outputs/good_call.txt

 headless/public/util/http_url_fetcher.cc
-headless::HttpURLFetcher::Delegate::Delegate
 100
 net::URLRequestContext::CreateRequest
 1
\ No newline at end of file

tools/traffic_annotation/auditor/tests/extractor_outputs/good_complete_annotation.txt

 chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc
-OnGetTokenSuccess
 166
 Definition
 supervised_user_refresh_token_fetcher

tools/traffic_annotation/auditor/tests/extractor_outputs/good_completing_annotation.txt

 chrome/service/cloud_print/cloud_print_url_fetcher.cc
-cloud_print::CloudPrintURLFetcher::StartRequestHelper
 265
 Completing
 cloud_print

tools/traffic_annotation/auditor/tests/extractor_outputs/good_partial_annotation.txt

 components/browsing_data/core/counters/history_counter.cc
-browsing_data::HistoryCounter::Count
 98
 Partial
 web_history_counter

tools/traffic_annotation/auditor/tests/extractor_outputs/good_test_annotation.txt

 google_apis/drive/request_sender_unittest.cc
-google_apis::(anonymous namespace)::RequestSenderTest::RequestSenderTest
 67
 Definition
 test

tools/traffic_annotation/auditor/tests/extractor_outputs/missing_annotation.txt

 net/url_request/url_request_context.cc
-net::URLRequestContext::CreateRequest
 120
 Definition
 missing

tools/traffic_annotation/auditor/traffic_annotation_auditor_unittest.cc

@@ -334,7 +334,6 @@ TEST_F(TrafficAnnotationAuditorTest, AnnotationDeserialization) {
     EXPECT_EQ(annotation.proto.source().file(),
               "chrome/browser/supervised_user/legacy/"
               "supervised_user_refresh_token_fetcher.cc");
-    EXPECT_EQ(annotation.proto.source().function(), "OnGetTokenSuccess");
     EXPECT_EQ(annotation.proto.source().line(), 166);
     EXPECT_EQ(annotation.proto.semantics().sender(), "Supervised Users");
     EXPECT_EQ(annotation.proto.policy().cookies_allowed(), 1);
@@ -365,8 +364,6 @@ TEST_F(TrafficAnnotationAuditorTest, CallDeserialization) {
 
     EXPECT_EQ(call.file_path, "headless/public/util/http_url_fetcher.cc");
     EXPECT_EQ(call.line_number, 100u);
-    EXPECT_EQ(call.function_context,
-              "headless::HttpURLFetcher::Delegate::Delegate");
     EXPECT_EQ(call.function_name, "net::URLRequestContext::CreateRequest");
     EXPECT_EQ(call.is_annotated, true);
   }
@@ -381,8 +378,7 @@ TEST_F(TrafficAnnotationAuditorTest, AssignmentDeserialization) {
 
   Assignmentample test_cases[] = {
       {"good_assignment.txt", AuditorResult::Type::RESULT_OK},
-      {"bad_assignment1.txt", AuditorResult::Type::ERROR_FATAL},
-      {"bad_assignment2.txt", AuditorResult::Type::ERROR_FATAL},
+      {"bad_assignment.txt", AuditorResult::Type::ERROR_FATAL},
   };
 
   for (const auto& test_case : test_cases) {

tools/traffic_annotation/bin/README.md

@@ -74,5 +74,5 @@ and land the resulting CL.
 The following two lines will be updated by the above script, and the modified
 README should be committed along with the updated .sha1 checksums.
 
-CLANG_REVISION = '8288453f6aac05080b751b680455349e09d49825'
-LASTCHANGE=1f619bc51ddddccc493776c0d2d3596d1c758a28-refs/heads/master@{#691147}
+CLANG_REVISION = 'b4160cb94c54f0b31d0ce14694950dac7b6cd83f'
+LASTCHANGE=af603aeed8f4a091c4f94605c00084c69c708bca-refs/heads/master@{#696886}
\ No newline at end of file

tools/traffic_annotation/bin/linux64/traffic_annotation_auditor.sha1

-eee0f2d9b6f83e03206912c748132495a21b5e37
\ No newline at end of file
+e7c47594d4aeafff709f574183586e7ebc99817a
\ No newline at end of file

tools/traffic_annotation/bin/linux64/traffic_annotation_extractor.sha1

-c4e2a772fbab2f1f7e47cf8f4f23425933f8cd0f
\ No newline at end of file
+8618305abe807cbeefaaa512e47796f89116bf98
\ No newline at end of file

tools/traffic_annotation/bin/win32/traffic_annotation_auditor.exe.sha1

-1709431c1a680a1d3880b4d9852c7d3d1a7e3661
\ No newline at end of file
+23b304a9748dc1cf023e4c35e34c2b0a90cc5e95
\ No newline at end of file

tools/traffic_annotation/bin/win32/traffic_annotation_extractor.exe.sha1

-eac43e1fb9f3fb1a300515061ad9daef6d51839d
\ No newline at end of file
+4f467153df6ea1dee2d83efd8b01dc78673e88c3
\ No newline at end of file

tools/traffic_annotation/scripts/extractor.py

@@ -54,13 +54,6 @@ class Annotation:
       file_path: Path to the file that contains this annotation.
     """
     self.file_path = file_path
-    # TODO(crbug/966883): Remove function_name from here and from the clang
-    # tool's output.
-    #
-    # |function_name| is not supported, but keep it in the output for
-    # compatibility with the clang tool. Use an obviously wrong function name
-    # in case this details ends up in auditor output.
-    self.function_name = "XXX_UNIMPLEMENTED_XXX"
     self.line_number = line_number
     self.type_name = type_name
     self.unique_id = unique_id
@@ -88,7 +81,6 @@ class Annotation:
     return "\n".join(map(str, [
         "==== NEW ANNOTATION ====",
         self.file_path,
-        self.function_name,
         self.line_number,
         self.type_name,
         self.unique_id,

tools/traffic_annotation/scripts/test_data/valid_file-stdout.txt

 ==== NEW ANNOTATION ====
 valid_file.cc
-XXX_UNIMPLEMENTED_XXX
 14
 Definition
 id1
@@ -26,7 +25,6 @@ id1
 ==== ANNOTATION ENDS ====
 ==== NEW ANNOTATION ====
 valid_file.cc
-XXX_UNIMPLEMENTED_XXX
 36
 Partial
 id2
@@ -42,7 +40,6 @@ completing_id2
 ==== ANNOTATION ENDS ====
 ==== NEW ANNOTATION ====
 valid_file.cc
-XXX_UNIMPLEMENTED_XXX
 46
 Completing
 id3
@@ -62,7 +59,6 @@ id3
 ==== ANNOTATION ENDS ====
 ==== NEW ANNOTATION ====
 valid_file.cc
-XXX_UNIMPLEMENTED_XXX
 61
 BranchedCompleting
 id4
@@ -77,7 +73,6 @@ branch4
 ==== ANNOTATION ENDS ====
 ==== NEW ANNOTATION ====
 valid_file.cc
-XXX_UNIMPLEMENTED_XXX
 126
 Mutable
 
@@ -86,7 +81,6 @@ Mutable
 ==== ANNOTATION ENDS ====
 ==== NEW ANNOTATION ====
 valid_file.cc
-XXX_UNIMPLEMENTED_XXX
 86
 Definition
 test
@@ -95,7 +89,6 @@ Traffic annotation for unit, browser and other tests
 ==== ANNOTATION ENDS ====
 ==== NEW ANNOTATION ====
 valid_file.cc
-XXX_UNIMPLEMENTED_XXX
 88
 Partial
 test_partial

tools/traffic_annotation/traffic_annotation.proto

@@ -38,12 +38,6 @@ message NetworkTrafficAnnotation {
     // specified.
     string file = 1;
 
-    // Function name where the network request is instantiated.
-    // This is typically filled by the extractor and does not need to be
-    // specified in the source code. For manual whitelisting this needs to be
-    // specified.
-    string function = 2;
-
     // __LINE__ in file, where the AuditPolicy object is instantiated.
     // This is typically filled by the extractor and does not need to be
     // specified in the source code.