Try FROM_HERE without function names

Looks like this saves ~140k on android-binary-size. Let's give it a go
on official builds?

Bug: 41342136
Change-Id: Ia565619b91fff9e8fdd3efdf376dbb0cdd236b78
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6099092
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Owners-Override: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1399770}

base/location.cc

@@ -106,9 +106,9 @@ Location::Location(const char* function_name,
 }
 
 std::string Location::ToString() const {
-  if (has_source_info()) {
-    return std::string(function_name_) + "@" + file_name_ + ":" +
-           NumberToString(line_number_);
+  if (function_name_ || file_name_) {
+    return std::string(function_name_ ? function_name_ : "(unknown)") + "@" +
+           file_name_ + ":" + NumberToString(line_number_);
   }
   return StringPrintf("pc:%p", program_counter_);
 }
@@ -140,7 +140,15 @@ NOINLINE Location Location::Current(const char* function_name,
 // static
 NOINLINE Location Location::CurrentWithoutFunctionName(const char* file_name,
                                                        int line_number) {
-  return Location(nullptr, file_name + kStrippedPrefixLength, line_number,
+  // TODO(pbos): Make sure that Location clients
+  // don't expect all fields of Location to be set. Doing so may require
+  // experiment rollout as existing code doesn't necessarily nullptr check
+  // function_name() etc. Right now (2024-12-19) TraceEventTestFixture tests
+  // crash inside std::string when tracing calls
+  // .set_function_name(location.function_name()) as std::string can't be
+  // constructed with nullptr. For now we initialize with "(unknown)" as we
+  // don't know whether there are untested call sites that expect non-nullptr.
+  return Location("(unknown)", file_name + kStrippedPrefixLength, line_number,
                   RETURN_ADDRESS());
 }
 

base/location.h

@@ -46,11 +46,6 @@ class BASE_EXPORT Location {
     return lhs.program_counter_ <=> rhs.program_counter_;
   }
 
-  // Returns true if there is source code location info. If this is false,
-  // the Location object only contains a program counter or is
-  // default-initialized (the program counter is also null).
-  bool has_source_info() const { return function_name_ && file_name_; }
-
   // Will be nullptr for default initialized Location objects and when source
   // names are disabled.
   const char* function_name() const { return function_name_; }
@@ -108,7 +103,11 @@ class BASE_EXPORT Location {
 
 BASE_EXPORT const void* GetProgramCounter();
 
+#if defined(OFFICIAL_BUILD)
+#define FROM_HERE ::base::Location::CurrentWithoutFunctionName()
+#else
 #define FROM_HERE ::base::Location::Current()
+#endif  // defined(OFFICIAL_BUILD)
 
 }  // namespace base
 

base/task/sequence_manager/sequence_manager_impl_unittest.cc

@@ -4998,9 +4998,14 @@ TEST_P(SequenceManagerTest, DescribeAllPendingTasks) {
   PostTaskC(queues[2]->task_runner());
 
   std::string description = sequence_manager()->DescribeAllPendingTasks();
+#if defined(OFFICIAL_BUILD)
+  // Function names are omitted from FROM_HERE in official builds.
+  EXPECT_THAT(description, HasSubstr("(unknown)@"));
+#else
   EXPECT_THAT(description, HasSubstr("PostTaskA@"));
   EXPECT_THAT(description, HasSubstr("PostTaskB@"));
   EXPECT_THAT(description, HasSubstr("PostTaskC@"));
+#endif  // defined(OFFICIAL_BUILD)
 }
 
 TEST_P(SequenceManagerTest, TaskPriortyInterleaving) {

base/test/scoped_run_loop_timeout_unittest.cc

@@ -125,7 +125,13 @@ constexpr char kErrorMessage[] = "I like kittens!";
 std::string GetExpectedTimeoutMessage(const Location& from,
                                       const char* expected_message) {
   std::ostringstream oss;
-  oss << "RunLoop::Run() timed out. Timeout set at " << from.function_name()
+  oss << "RunLoop::Run() timed out. Timeout set at " <<
+#if defined(OFFICIAL_BUILD)
+      // Function names are omitted from FROM_HERE in official builds.
+      "(unknown)"
+#else
+      from.function_name()
+#endif  // defined(OFFICIAL_BUILD)
       << "@" << from.file_name() << ":" << from.line_number() << ".\n"
       << expected_message;
   return oss.str();

base/test/task_environment_unittest.cc

@@ -951,20 +951,25 @@ TEST_F(TaskEnvironmentTest, SetsDefaultRunTimeout) {
       EXPECT_LT(run_timeout->timeout, TestTimeouts::test_launcher_timeout());
     }
     static auto& static_on_timeout_cb = run_timeout->on_timeout;
-#if defined(__clang__) && defined(_MSC_VER)
-    EXPECT_NONFATAL_FAILURE(
-        static_on_timeout_cb.Run(FROM_HERE),
-        "RunLoop::Run() timed out. Timeout set at "
-        // We don't test the line number but it would be present.
-        "TaskEnvironment@base\\test\\task_environment.cc:");
+#if defined(OFFICIAL_BUILD)
+    // Function names are omitted from FROM_HERE in official builds.
+#define EXPECTED_FUNCTION "(unknown)"
 #else
+#define EXPECTED_FUNCTION "TaskEnvironment"
+#endif  // defined(OFFICIAL_BUILD)
+#if defined(__clang__) && defined(_MSC_VER)
+#define EXPECTED_FILE "base\\test\\task_environment.cc"
+#else
+#define EXPECTED_FILE "base/test/task_environment.cc"
+#endif
     EXPECT_NONFATAL_FAILURE(
         static_on_timeout_cb.Run(FROM_HERE),
         "RunLoop::Run() timed out. Timeout set at "
         // We don't test the line number but it would be present.
-        "TaskEnvironment@base/test/task_environment.cc:");
-#endif
+        EXPECTED_FUNCTION "@" EXPECTED_FILE ":");
   }
+#undef EXPECTED_FUNCTION
+#undef EXPECTED_FILE
 
   EXPECT_EQ(ScopedRunLoopTimeout::GetTimeoutForCurrentThread(),
             old_run_timeout);

content/browser/web_contents/web_contents_impl.cc

@@ -1167,18 +1167,10 @@ class WebContentsOfBrowserContext : public base::SupportsUserData::Data {
 
     const std::optional<base::Location>& ownership_location =
         web_contents_with_dangling_ptr_to_browser_context->ownership_location();
-    std::string owner;
-    if (ownership_location) {
-      if (ownership_location->has_source_info()) {
-        owner = std::string(ownership_location->function_name()) + "@" +
-                ownership_location->file_name();
-      } else {
-        owner = "no_source_info";
-      }
-    } else {
-      owner = "unknown";
-    }
-    SCOPED_CRASH_KEY_STRING256("shutdown", "web_contents/owner", owner);
+    SCOPED_CRASH_KEY_STRING256("shutdown", "web_contents/owner",
+                               ownership_location
+                                   ? ownership_location->ToString()
+                                   : std::string("unknown"));
 
 #if BUILDFLAG(IS_ANDROID)
     // On Android, also report the Java stack trace from WebContents's