[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/+/1686621Reviewed-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}

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

@@ -95,8 +95,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 = '360094'
-LASTCHANGE=ecda211dff8f3722704938f87637e0df657d15f1-refs/heads/master@{#659930}
+CLANG_REVISION = 'd874c057bc2361da5157553e1e2178f43c3ade1a'
+LASTCHANGE=5a2618740eb68a63b73a40f08a13b1e337cb6399-refs/heads/master@{#674114}

tools/traffic_annotation/bin/linux64/traffic_annotation_auditor.sha1

-73004f4964def13577f9d060d77908d51c30d521
+24a6a97630bc133de57c4812af3abca8763b5a7f
\ No newline at end of file

tools/traffic_annotation/bin/linux64/traffic_annotation_extractor.sha1

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

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

-e3c00e2607fbe0d5758e60bc42c6802feacdea08
+fe1e664643c009ca4c16fbe3cabe34a206ca50ee
\ No newline at end of file

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

-eac43e1fb9f3fb1a300515061ad9daef6d51839d
+76bd7403ff65eeeb27c7c56dda2688e3d9319053
\ 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.