[Clank SSM]: Inject module factory to ModuleCache.

This CL avoids expensive and repeating work of filling ModuleCache
with modules in NativeUnwinderAndroid::AddInitialModules.
Instead, ModuleCache::RegisterAuxiliaryModuleProvider is used
to register a provider that lazily creates non-elf modules when
needed.

To make that possible, this CL makes explicit new constraints on
Unwinder: a single ModuleCache is "associated" with a Unwinder for its
lifetime and ModuleCache must be outlive by any Unwinders it's
associated with.
A follow up CL will refactor the Unwinder interface to better fit these
constraints.

Change-Id: I05374ed8989061c81cde0ba09605b54e6b2309b9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2514743
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842185}

base/profiler/module_cache.cc

@@ -31,15 +31,21 @@ struct ModuleAddressCompare {
 }  // namespace
 
 ModuleCache::ModuleCache() = default;
-ModuleCache::~ModuleCache() = default;
+
+ModuleCache::~ModuleCache() {
+  DCHECK_EQ(auxiliary_module_provider_, nullptr);
+}
 
 const ModuleCache::Module* ModuleCache::GetModuleForAddress(uintptr_t address) {
   if (const ModuleCache::Module* module = GetExistingModuleForAddress(address))
     return module;
 
   std::unique_ptr<const Module> new_module = CreateModuleForAddress(address);
+  if (!new_module && auxiliary_module_provider_)
+    new_module = auxiliary_module_provider_->TryCreateModuleForAddress(address);
   if (!new_module)
     return nullptr;
+
   const auto result = native_modules_.insert(std::move(new_module));
   // TODO(https://crbug.com/1131769): Reintroduce DCHECK(result.second) after
   // fixing the issue that is causing it to fail.
@@ -126,6 +132,18 @@ const ModuleCache::Module* ModuleCache::GetExistingModuleForAddress(
   return nullptr;
 }
 
+void ModuleCache::RegisterAuxiliaryModuleProvider(
+    AuxiliaryModuleProvider* auxiliary_module_provider) {
+  DCHECK(!auxiliary_module_provider_);
+  auxiliary_module_provider_ = auxiliary_module_provider;
+}
+
+void ModuleCache::UnregisterAuxiliaryModuleProvider(
+    AuxiliaryModuleProvider* auxiliary_module_provider) {
+  DCHECK_EQ(auxiliary_module_provider_, auxiliary_module_provider);
+  auxiliary_module_provider_ = nullptr;
+}
+
 bool ModuleCache::ModuleAndAddressCompare::operator()(
     const std::unique_ptr<const Module>& m1,
     const std::unique_ptr<const Module>& m2) const {

base/profiler/module_cache.h

@@ -66,6 +66,21 @@ class BASE_EXPORT ModuleCache {
     virtual bool IsNative() const = 0;
   };
 
+  // Interface for lazily creating a native module for a given |address|. The
+  // provider is registered with RegisterAuxiliaryModuleProvider().
+  class AuxiliaryModuleProvider {
+   public:
+    AuxiliaryModuleProvider() = default;
+    AuxiliaryModuleProvider(const AuxiliaryModuleProvider&) = delete;
+    AuxiliaryModuleProvider& operator=(const AuxiliaryModuleProvider&) = delete;
+
+    virtual std::unique_ptr<const Module> TryCreateModuleForAddress(
+        uintptr_t address) = 0;
+
+   protected:
+    ~AuxiliaryModuleProvider() = default;
+  };
+
   ModuleCache();
   ~ModuleCache();
 
@@ -103,6 +118,19 @@ class BASE_EXPORT ModuleCache {
   // ModuleCache.
   void AddCustomNativeModule(std::unique_ptr<const Module> module);
 
+  // Registers a custom module provider for lazily creating native modules. At
+  // most one provider can be registered at any time, and the provider must be
+  // unregistered before being destroyed. This is intended to support native
+  // modules that require custom handling. In general, native modules will be
+  // found and added automatically when invoking GetModuleForAddress(). If no
+  // module is found, this provider will be used as fallback.
+  void RegisterAuxiliaryModuleProvider(
+      AuxiliaryModuleProvider* auxiliary_module_provider);
+
+  // Unregisters the custom module provider.
+  void UnregisterAuxiliaryModuleProvider(
+      AuxiliaryModuleProvider* auxiliary_module_provider);
+
   // Gets the module containing |address| if one already exists, or nullptr
   // otherwise. The returned module remains owned by and has the same lifetime
   // as the ModuleCache object.
@@ -156,6 +184,9 @@ class BASE_EXPORT ModuleCache {
   // because it can contain multiple modules that were loaded (then subsequently
   // unloaded) at the same base address.
   std::vector<std::unique_ptr<const Module>> inactive_non_native_modules_;
+
+  // Auxiliary module provider, for lazily creating native modules.
+  AuxiliaryModuleProvider* auxiliary_module_provider_ = nullptr;
 };
 
 }  // namespace base

base/profiler/module_cache_unittest.cc

@@ -411,5 +411,68 @@ TEST(ModuleCacheTest, CheckAgainstProcMaps) {
 }
 #endif
 
+// Module provider that always return a fake module of size 1 for a given
+// |address|.
+class MockModuleProvider : public ModuleCache::AuxiliaryModuleProvider {
+ public:
+  explicit MockModuleProvider(size_t module_size = 1)
+      : module_size_(module_size) {}
+
+  std::unique_ptr<const ModuleCache::Module> TryCreateModuleForAddress(
+      uintptr_t address) override {
+    return std::make_unique<FakeModule>(address, module_size_);
+  }
+
+ private:
+  size_t module_size_;
+};
+
+// Check that auxiliary provider can inject new modules when registered.
+TEST(ModuleCacheTest, RegisterAuxiliaryModuleProvider) {
+  ModuleCache cache;
+  EXPECT_EQ(nullptr, cache.GetModuleForAddress(1));
+
+  MockModuleProvider auxiliary_provider;
+  cache.RegisterAuxiliaryModuleProvider(&auxiliary_provider);
+  auto* module = cache.GetModuleForAddress(1);
+  EXPECT_NE(nullptr, module);
+  EXPECT_EQ(1U, module->GetBaseAddress());
+  cache.UnregisterAuxiliaryModuleProvider(&auxiliary_provider);
+
+  // Even when unregistered, the module remains in the cache.
+  EXPECT_EQ(module, cache.GetModuleForAddress(1));
+}
+
+// Check that ModuleCache's own module creator is used preferentially over
+// auxiliary provider if possible.
+MAYBE_TEST(ModuleCacheTest, NativeModuleOverAuxiliaryModuleProvider) {
+  ModuleCache cache;
+
+  MockModuleProvider auxiliary_provider(/*module_size=*/100);
+  cache.RegisterAuxiliaryModuleProvider(&auxiliary_provider);
+
+  const ModuleCache::Module* module =
+      cache.GetModuleForAddress(reinterpret_cast<uintptr_t>(&AFunctionForTest));
+  ASSERT_NE(nullptr, module);
+
+  // The module should be a native module, which will have size greater than 100
+  // bytes.
+  EXPECT_NE(100u, module->GetSize());
+  cache.UnregisterAuxiliaryModuleProvider(&auxiliary_provider);
+}
+
+// Check that auxiliary provider is no longer used after being unregistered.
+TEST(ModuleCacheTest, UnregisterAuxiliaryModuleProvider) {
+  ModuleCache cache;
+
+  EXPECT_EQ(nullptr, cache.GetModuleForAddress(1));
+
+  MockModuleProvider auxiliary_provider;
+  cache.RegisterAuxiliaryModuleProvider(&auxiliary_provider);
+  cache.UnregisterAuxiliaryModuleProvider(&auxiliary_provider);
+
+  EXPECT_EQ(nullptr, cache.GetModuleForAddress(1));
+}
+
 }  // namespace
 }  // namespace base

base/profiler/native_unwinder_android.cc

@@ -127,10 +127,14 @@ NativeUnwinderAndroid::NativeUnwinderAndroid(
       process_memory_(process_memory),
       exclude_module_with_base_address_(exclude_module_with_base_address) {}
 
-NativeUnwinderAndroid::~NativeUnwinderAndroid() = default;
+NativeUnwinderAndroid::~NativeUnwinderAndroid() {
+  if (module_cache_)
+    module_cache_->UnregisterAuxiliaryModuleProvider(this);
+}
 
-void NativeUnwinderAndroid::AddInitialModules(ModuleCache* module_cache) {
-  AddInitialModulesFromMaps(*memory_regions_map_, module_cache);
+void NativeUnwinderAndroid::InitializeModules(ModuleCache* module_cache) {
+  module_cache_ = module_cache;
+  module_cache_->RegisterAuxiliaryModuleProvider(this);
 }
 
 bool NativeUnwinderAndroid::CanUnwindFrom(const Frame& current_frame) const {
@@ -215,34 +219,14 @@ UnwindResult NativeUnwinderAndroid::TryUnwind(RegisterContext* thread_context,
   return UnwindResult::UNRECOGNIZED_FRAME;
 }
 
-// static
-void NativeUnwinderAndroid::AddInitialModulesFromMaps(
-    const unwindstack::Maps& memory_regions_map,
-    ModuleCache* module_cache) {
-  // The effect of this loop is to create modules for the executable regions in
-  // the memory map. Regions composing a mapped ELF file are handled specially
-  // however: for just one module extending from the ELF base address to the
-  // *last* executable region backed by the file is implicitly created by
-  // ModuleCache. This avoids duplicate module instances covering the same
-  // in-memory module in the case that a module has multiple mmapped executable
-  // regions.
-  for (const std::unique_ptr<unwindstack::MapInfo>& region :
-       memory_regions_map) {
-    if (!(region->flags & PROT_EXEC))
-      continue;
-
-    // Use the standard ModuleCache POSIX module representation for ELF files.
-    // This call returns the containing ELF module for the region, creating it
-    // if it doesn't exist.
-    if (module_cache->GetModuleForAddress(
-            static_cast<uintptr_t>(region->start))) {
-      continue;
-    }
-
-    // Non-ELF modules are represented with NonElfModule.
-    module_cache->AddCustomNativeModule(
-        std::make_unique<NonElfModule>(region.get()));
+std::unique_ptr<const ModuleCache::Module>
+NativeUnwinderAndroid::TryCreateModuleForAddress(uintptr_t address) {
+  unwindstack::MapInfo* map_info = memory_regions_map_->Find(address);
+  if (map_info == nullptr || !(map_info->flags & PROT_EXEC) ||
+      map_info->flags & unwindstack::MAPS_FLAGS_DEVICE_MAP) {
+    return nullptr;
   }
+  return std::make_unique<NonElfModule>(map_info);
 }
 
 void NativeUnwinderAndroid::EmitDexFrame(uintptr_t dex_pc,

base/profiler/native_unwinder_android.h

@@ -27,7 +27,8 @@ class UnwindStackMemoryAndroid : public unwindstack::Memory {
 };
 
 // Native unwinder implementation for Android, using libunwindstack.
-class NativeUnwinderAndroid : public Unwinder {
+class NativeUnwinderAndroid : public Unwinder,
+                              public ModuleCache::AuxiliaryModuleProvider {
  public:
   // Creates maps object from /proc/self/maps for use by NativeUnwinderAndroid.
   // Since this is an expensive call, the maps object should be re-used across
@@ -35,8 +36,9 @@ class NativeUnwinderAndroid : public Unwinder {
   static std::unique_ptr<unwindstack::Maps> CreateMaps();
   static std::unique_ptr<unwindstack::Memory> CreateProcessMemory();
 
-  // |exclude_module_with_base_address| is used to exclude a specific module
-  // and let another unwinder take control. TryUnwind() will exit with
+  // |memory_regions_map| and |process_memory| must outlive this unwinder.
+  // |exclude_module_with_base_address| is used to exclude a specific module and
+  // let another unwinder take control. TryUnwind() will exit with
   // UNRECOGNIZED_FRAME and CanUnwindFrom() will return false when a frame is
   // encountered in that module.
   NativeUnwinderAndroid(unwindstack::Maps* memory_regions_map,
@@ -48,24 +50,25 @@ class NativeUnwinderAndroid : public Unwinder {
   NativeUnwinderAndroid& operator=(const NativeUnwinderAndroid&) = delete;
 
   // Unwinder
-  void AddInitialModules(ModuleCache* module_cache) override;
+  void InitializeModules(ModuleCache* module_cache) override;
   bool CanUnwindFrom(const Frame& current_frame) const override;
   UnwindResult TryUnwind(RegisterContext* thread_context,
                          uintptr_t stack_top,
                          ModuleCache* module_cache,
                          std::vector<Frame>* stack) const override;
 
-  // Adds modules found from executable loaded memory regions to |module_cache|.
-  // Public for test access.
-  static void AddInitialModulesFromMaps(
-      const unwindstack::Maps& memory_regions_map,
-      ModuleCache* module_cache);
+  // ModuleCache::AuxiliaryModuleProvider
+  std::unique_ptr<const ModuleCache::Module> TryCreateModuleForAddress(
+      uintptr_t address) override;
 
  private:
   void EmitDexFrame(uintptr_t dex_pc,
                     ModuleCache* module_cache,
                     std::vector<Frame>* stack) const;
 
+  // InitializeModules() registers self as an AuxiliaryModuleProvider. A pointer
+  // to the ModuleCache is saved to unregister self in destructor.
+  ModuleCache* module_cache_ = nullptr;
   unwindstack::Maps* const memory_regions_map_;
   unwindstack::Memory* const process_memory_;
   const uintptr_t exclude_module_with_base_address_;

base/profiler/native_unwinder_android_unittest.cc

@@ -34,11 +34,6 @@ namespace base {
 
 namespace {
 
-bool CompareModulesByBaseAddress(const ModuleCache::Module* a,
-                                 const ModuleCache::Module* b) {
-  return a->GetBaseAddress() < b->GetBaseAddress();
-}
-
 // Add a MapInfo with the provided values to |maps|.
 void AddMapInfo(uint64_t start,
                 uint64_t end,
@@ -114,11 +109,12 @@ TEST(NativeUnwinderAndroidTest, MAYBE_PlainFunction) {
   std::unique_ptr<unwindstack::Maps> maps = NativeUnwinderAndroid::CreateMaps();
   std::unique_ptr<unwindstack::Memory> memory =
       NativeUnwinderAndroid::CreateProcessMemory();
+
+  ModuleCache module_cache;
   auto unwinder =
       std::make_unique<NativeUnwinderAndroid>(maps.get(), memory.get(), 0);
 
-  ModuleCache module_cache;
-  unwinder->AddInitialModules(&module_cache);
+  unwinder->InitializeModules(&module_cache);
   std::vector<Frame> sample =
       CaptureScenario(&scenario, &module_cache,
                       BindLambdaForTesting([&](RegisterContext* thread_context,
@@ -154,11 +150,12 @@ TEST(NativeUnwinderAndroidTest, MAYBE_Alloca) {
   std::unique_ptr<unwindstack::Maps> maps = NativeUnwinderAndroid::CreateMaps();
   std::unique_ptr<unwindstack::Memory> memory =
       NativeUnwinderAndroid::CreateProcessMemory();
+
+  ModuleCache module_cache;
   auto unwinder =
       std::make_unique<NativeUnwinderAndroid>(maps.get(), memory.get(), 0);
 
-  ModuleCache module_cache;
-  unwinder->AddInitialModules(&module_cache);
+  unwinder->InitializeModules(&module_cache);
   std::vector<Frame> sample =
       CaptureScenario(&scenario, &module_cache,
                       BindLambdaForTesting([&](RegisterContext* thread_context,
@@ -196,11 +193,12 @@ TEST(NativeUnwinderAndroidTest, MAYBE_OtherLibrary) {
   std::unique_ptr<unwindstack::Maps> maps = NativeUnwinderAndroid::CreateMaps();
   std::unique_ptr<unwindstack::Memory> memory =
       NativeUnwinderAndroid::CreateProcessMemory();
+
+  ModuleCache module_cache;
   auto unwinder =
       std::make_unique<NativeUnwinderAndroid>(maps.get(), memory.get(), 0);
 
-  ModuleCache module_cache;
-  unwinder->AddInitialModules(&module_cache);
+  unwinder->InitializeModules(&module_cache);
   std::vector<Frame> sample =
       CaptureScenario(&scenario, &module_cache,
                       BindLambdaForTesting([&](RegisterContext* thread_context,
@@ -228,12 +226,13 @@ TEST(NativeUnwinderAndroidTest, ExcludeOtherLibrary) {
   std::unique_ptr<unwindstack::Memory> memory =
       NativeUnwinderAndroid::CreateProcessMemory();
   ModuleCache module_cache;
-  NativeUnwinderAndroid::AddInitialModulesFromMaps(*maps, &module_cache);
-
+  unwindstack::MapInfo* other_library_map =
+      maps->Find(GetAddressInOtherLibrary(other_library));
+  ASSERT_NE(nullptr, other_library_map);
   auto unwinder = std::make_unique<NativeUnwinderAndroid>(
-      maps.get(), memory.get(),
-      module_cache.GetModuleForAddress(GetAddressInOtherLibrary(other_library))
-          ->GetBaseAddress());
+      maps.get(), memory.get(), other_library_map->start);
+  unwinder->InitializeModules(&module_cache);
+
   std::vector<Frame> sample =
       CaptureScenario(&scenario, &module_cache,
                       BindLambdaForTesting([&](RegisterContext* thread_context,
@@ -266,34 +265,44 @@ TEST(NativeUnwinderAndroidTest, MAYBE_ResumeUnwinding) {
   std::unique_ptr<unwindstack::Maps> maps = NativeUnwinderAndroid::CreateMaps();
   std::unique_ptr<unwindstack::Memory> memory =
       NativeUnwinderAndroid::CreateProcessMemory();
-  ModuleCache module_cache;
-  NativeUnwinderAndroid::AddInitialModulesFromMaps(*maps, &module_cache);
 
-  // Several unwinders are used to unwind different portion of the stack. This
-  // tests that NativeUnwinderAndroid can pick up from a state in the middle of
-  // the stack. This emulates having NativeUnwinderAndroid work with other
-  // unwinders, but doesn't reproduce what happens in production.
+  // Several unwinders are used to unwind different portion of the stack. Since
+  // only 1 unwinder can be registered as a module provider, each unwinder uses
+  // a distinct ModuleCache. This tests that NativeUnwinderAndroid can pick up
+  // from a state in the middle of the stack. This emulates having
+  // NativeUnwinderAndroid work with other unwinders, but doesn't reproduce what
+  // happens in production.
+  ModuleCache module_cache_for_all;
   auto unwinder_for_all =
       std::make_unique<NativeUnwinderAndroid>(maps.get(), memory.get(), 0);
+  unwinder_for_all->InitializeModules(&module_cache_for_all);
+
+  ModuleCache module_cache_for_native;
   auto unwinder_for_native = std::make_unique<NativeUnwinderAndroid>(
       maps.get(), memory.get(),
       reinterpret_cast<uintptr_t>(&__executable_start));
+  unwinder_for_native->InitializeModules(&module_cache_for_native);
+
+  ModuleCache module_cache_for_chrome;
+  unwindstack::MapInfo* other_library_map =
+      maps->Find(GetAddressInOtherLibrary(other_library));
+  ASSERT_NE(nullptr, other_library_map);
   auto unwinder_for_chrome = std::make_unique<NativeUnwinderAndroid>(
-      maps.get(), memory.get(),
-      module_cache.GetModuleForAddress(GetAddressInOtherLibrary(other_library))
-          ->GetBaseAddress());
+      maps.get(), memory.get(), other_library_map->start);
+  unwinder_for_chrome->InitializeModules(&module_cache_for_chrome);
 
   std::vector<Frame> sample = CaptureScenario(
-      &scenario, &module_cache,
+      &scenario, &module_cache_for_native,
       BindLambdaForTesting([&](RegisterContext* thread_context,
                                uintptr_t stack_top,
                                std::vector<Frame>* sample) {
         // |unwinder_for_native| unwinds through native frames, but stops at
         // chrome frames. It might not contain SampleAddressRange.
         ASSERT_TRUE(unwinder_for_native->CanUnwindFrom(sample->back()));
-        EXPECT_EQ(UnwindResult::UNRECOGNIZED_FRAME,
-                  unwinder_for_native->TryUnwind(thread_context, stack_top,
-                                                 &module_cache, sample));
+        EXPECT_EQ(
+            UnwindResult::UNRECOGNIZED_FRAME,
+            unwinder_for_native->TryUnwind(thread_context, stack_top,
+                                           &module_cache_for_native, sample));
         EXPECT_FALSE(unwinder_for_native->CanUnwindFrom(sample->back()));
 
         ExpectStackDoesNotContain(*sample,
@@ -304,9 +313,10 @@ TEST(NativeUnwinderAndroidTest, MAYBE_ResumeUnwinding) {
         // |unwinder_for_chrome| unwinds through Chrome frames, but stops at
         // |other_library|. It won't contain SetupFunctionAddressRange.
         ASSERT_TRUE(unwinder_for_chrome->CanUnwindFrom(sample->back()));
-        EXPECT_EQ(UnwindResult::UNRECOGNIZED_FRAME,
-                  unwinder_for_chrome->TryUnwind(thread_context, stack_top,
-                                                 &module_cache, sample));
+        EXPECT_EQ(
+            UnwindResult::UNRECOGNIZED_FRAME,
+            unwinder_for_chrome->TryUnwind(thread_context, stack_top,
+                                           &module_cache_for_chrome, sample));
         EXPECT_FALSE(unwinder_for_chrome->CanUnwindFrom(sample->back()));
         EXPECT_LT(prior_stack_size, sample->size());
         ExpectStackContains(*sample, {scenario.GetWaitForSampleAddressRange()});
@@ -318,7 +328,7 @@ TEST(NativeUnwinderAndroidTest, MAYBE_ResumeUnwinding) {
         ASSERT_TRUE(unwinder_for_all->CanUnwindFrom(sample->back()));
         EXPECT_EQ(UnwindResult::COMPLETED,
                   unwinder_for_all->TryUnwind(thread_context, stack_top,
-                                              &module_cache, sample));
+                                              &module_cache_for_all, sample));
       }));
 
   // The stack should contain a full unwind.
@@ -370,7 +380,7 @@ TEST(NativeUnwinderAndroidTest, DISABLED_JavaFunction) {
       std::make_unique<NativeUnwinderAndroid>(maps.get(), memory.get(), 0);
 
   ModuleCache module_cache;
-  unwinder->AddInitialModules(&module_cache);
+  unwinder->InitializeModules(&module_cache);
   std::vector<Frame> sample =
       CaptureScenario(&scenario, &module_cache,
                       BindLambdaForTesting([&](RegisterContext* thread_context,
@@ -431,12 +441,17 @@ TEST(NativeUnwinderAndroidTest, ModuleDebugBasenameForNonElf) {
              maps);
 
   ModuleCache module_cache;
-  NativeUnwinderAndroid::AddInitialModulesFromMaps(maps, &module_cache);
 
-  std::vector<const ModuleCache::Module*> modules = module_cache.GetModules();
+  std::unique_ptr<unwindstack::Memory> memory =
+      NativeUnwinderAndroid::CreateProcessMemory();
+  auto unwinder =
+      std::make_unique<NativeUnwinderAndroid>(&maps, memory.get(), 0);
+  unwinder->InitializeModules(&module_cache);
+
+  const ModuleCache::Module* module = module_cache.GetModuleForAddress(0x1000u);
 
-  ASSERT_EQ(1u, modules.size());
-  EXPECT_EQ("[foo / bar]", modules[0]->GetDebugBasename().value());
+  ASSERT_TRUE(module);
+  EXPECT_EQ("[foo / bar]", module->GetDebugBasename().value());
 }
 
 // Checks that modules are only created for executable memory regions.
@@ -446,113 +461,26 @@ TEST(NativeUnwinderAndroidTest, ModulesCreatedOnlyForExecutableRegions) {
   AddMapInfo(0x2000u, 0x3000u, 0u, PROT_READ, "[b]", {0xAB}, maps);
   AddMapInfo(0x3000u, 0x4000u, 0u, PROT_READ | PROT_EXEC, "[c]", {0xAC}, maps);
 
-  ModuleCache module_cache;
-  NativeUnwinderAndroid::AddInitialModulesFromMaps(maps, &module_cache);
-
-  std::vector<const ModuleCache::Module*> modules = module_cache.GetModules();
-  std::sort(modules.begin(), modules.end(), CompareModulesByBaseAddress);
-
-  ASSERT_EQ(2u, modules.size());
-  EXPECT_EQ(0x1000u, modules[0]->GetBaseAddress());
-  EXPECT_EQ(0x3000u, modules[1]->GetBaseAddress());
-}
+  std::unique_ptr<unwindstack::Memory> memory =
+      NativeUnwinderAndroid::CreateProcessMemory();
 
-// Checks that module address ranges don't overlap.
-TEST(NativeUnwinderAndroidTest, NonOverlappingModules) {
   ModuleCache module_cache;
-  std::unique_ptr<unwindstack::Maps> maps = NativeUnwinderAndroid::CreateMaps();
-  NativeUnwinderAndroid::AddInitialModulesFromMaps(*maps, &module_cache);
-
-  std::vector<const ModuleCache::Module*> modules = module_cache.GetModules();
-  std::sort(modules.begin(), modules.end(), CompareModulesByBaseAddress);
-  auto loc = std::adjacent_find(
-      modules.begin(), modules.end(),
-      [](const ModuleCache::Module* m1, const ModuleCache::Module* m2) {
-        return m2->GetBaseAddress() < m1->GetBaseAddress() + m1->GetSize();
-      });
-
-  const auto describe_module = [](const ModuleCache::Module* module) {
-    return StringPrintf(
-        "id \"%s\", debug basename \"%s\" at [0x%" PRIxPTR ", 0x%" PRIxPTR ")",
-        module->GetId().c_str(), module->GetDebugBasename().value().c_str(),
-        module->GetBaseAddress(), module->GetBaseAddress() + module->GetSize());
-  };
-
-  EXPECT_EQ(modules.end(), loc) << "module overlap found between\n"
-                                << "  " << describe_module(*loc) << " and \n"
-                                << "  " << describe_module(*std::next(loc));
-}
-
-// ModuleCache::GetModuleForAddress() is not implemented for 64-bit arm.
-#if defined(ARCH_CPU_ARM64)
-#define MAYBE_ModuleState_SystemLibrary DISABLED_ModuleState_SystemLibrary
-#else
-#define MAYBE_ModuleState_SystemLibrary ModuleState_SystemLibrary
-#endif
-// Checks that the module state created by the unwinder is consistent with the
-// state created by the ModuleCache. Checks the module for a system library.
-TEST(NativeUnwinderAndroidTest, MAYBE_ModuleState_SystemLibrary) {
-  ModuleCache unwinder_module_cache;
-  std::unique_ptr<unwindstack::Maps> maps = NativeUnwinderAndroid::CreateMaps();
-  NativeUnwinderAndroid::AddInitialModulesFromMaps(*maps,
-                                                   &unwinder_module_cache);
-
-  const uintptr_t c_library_function_address =
-      reinterpret_cast<uintptr_t>(&printf);
-
-  const ModuleCache::Module* unwinder_module =
-      unwinder_module_cache.GetExistingModuleForAddress(
-          c_library_function_address);
-  ASSERT_NE(nullptr, unwinder_module);
-
-  ModuleCache reference_module_cache;
-  const ModuleCache::Module* reference_module =
-      reference_module_cache.GetModuleForAddress(c_library_function_address);
-  ASSERT_NE(nullptr, reference_module);
-
-  EXPECT_EQ(reference_module->GetBaseAddress(),
-            unwinder_module->GetBaseAddress());
-  EXPECT_EQ(reference_module->GetId(), unwinder_module->GetId());
-  EXPECT_EQ(reference_module->GetDebugBasename(),
-            unwinder_module->GetDebugBasename());
-  EXPECT_EQ(unwinder_module->GetSize(), reference_module->GetSize());
-}
-
-// ModuleCache::GetModuleForAddress() is not implemented for 64-bit arm.
-#if defined(ARCH_CPU_ARM64)
-#define MAYBE_ModuleState_ChromeLibrary DISABLED_ModuleState_ChromeLibrary
-#else
-#define MAYBE_ModuleState_ChromeLibrary ModuleState_ChromeLibrary
-#endif
-// Checks that the module state created by the unwinder is consistent with the
-// state created by the ModuleCache. Checks the module for a Chrome-compiled
-// library.
-TEST(NativeUnwinderAndroidTest, MAYBE_ModuleState_ChromeLibrary) {
-  ModuleCache unwinder_module_cache;
-  std::unique_ptr<unwindstack::Maps> maps = NativeUnwinderAndroid::CreateMaps();
-  NativeUnwinderAndroid::AddInitialModulesFromMaps(*maps,
-                                                   &unwinder_module_cache);
-
-  const uintptr_t chrome_function_address =
-      reinterpret_cast<uintptr_t>(&CaptureScenario);
-
-  const ModuleCache::Module* unwinder_module =
-      unwinder_module_cache.GetExistingModuleForAddress(
-          chrome_function_address);
-  ASSERT_NE(nullptr, unwinder_module);
-
-  ModuleCache reference_module_cache;
-  const ModuleCache::Module* reference_module =
-      reference_module_cache.GetModuleForAddress(chrome_function_address);
-  ASSERT_NE(nullptr, reference_module);
-
-  EXPECT_EQ(reference_module->GetBaseAddress(),
-            unwinder_module->GetBaseAddress());
-  EXPECT_NE("", unwinder_module->GetId());
-  EXPECT_EQ(reference_module->GetId(), unwinder_module->GetId());
-  EXPECT_EQ(reference_module->GetDebugBasename(),
-            unwinder_module->GetDebugBasename());
-  EXPECT_EQ(unwinder_module->GetSize(), reference_module->GetSize());
+  auto unwinder =
+      std::make_unique<NativeUnwinderAndroid>(&maps, memory.get(), 0);
+  unwinder->InitializeModules(&module_cache);
+
+  const ModuleCache::Module* module1 =
+      module_cache.GetModuleForAddress(0x1000u);
+  const ModuleCache::Module* module2 =
+      module_cache.GetModuleForAddress(0x2000u);
+  const ModuleCache::Module* module3 =
+      module_cache.GetModuleForAddress(0x3000u);
+
+  ASSERT_TRUE(module1);
+  EXPECT_EQ(0x1000u, module1->GetBaseAddress());
+  EXPECT_EQ(nullptr, module2);
+  ASSERT_TRUE(module3);
+  EXPECT_EQ(0x3000u, module3->GetBaseAddress());
 }
 
 }  // namespace base

base/profiler/stack_sampler_impl.cc

@@ -92,17 +92,17 @@ void StackSamplerImpl::Initialize() {
                     std::make_move_iterator(unwinders.rend()));
 
   for (const auto& unwinder : unwinders_)
-    unwinder->AddInitialModules(module_cache_);
+    unwinder->InitializeModules(module_cache_);
 
   was_initialized_ = true;
 }
 
 void StackSamplerImpl::AddAuxUnwinder(std::unique_ptr<Unwinder> unwinder) {
-  // Initialize() invokes AddInitialModules() on the unwinders that are present
+  // Initialize() invokes InitializeModules() on the unwinders that are present
   // at the time. If it hasn't occurred yet, we allow it to add the initial
   // modules, otherwise we do it here.
   if (was_initialized_)
-    unwinder->AddInitialModules(module_cache_);
+    unwinder->InitializeModules(module_cache_);
   unwinders_.push_front(std::move(unwinder));
 }
 

base/profiler/stack_sampling_profiler.cc

@@ -126,8 +126,8 @@ class StackSamplingProfiler::SamplingThread : public Thread {
         : collection_id(next_collection_id.GetNext()),
           params(params),
           finished(finished),
-          sampler(std::move(sampler)),
-          profile_builder(std::move(profile_builder)) {}
+          profile_builder(std::move(profile_builder)),
+          sampler(std::move(sampler)) {}
     ~CollectionContext() = default;
 
     // An identifier for this collection, used to uniquely identify the
@@ -137,12 +137,12 @@ class StackSamplingProfiler::SamplingThread : public Thread {
     const SamplingParams params;    // Information about how to sample.
     WaitableEvent* const finished;  // Signaled when all sampling complete.
 
-    // Platform-specific module that does the actual sampling.
-    std::unique_ptr<StackSampler> sampler;
-
     // Receives the sampling data and builds a CallStackProfile.
     std::unique_ptr<ProfileBuilder> profile_builder;
 
+    // Platform-specific module that does the actual sampling.
+    std::unique_ptr<StackSampler> sampler;
+
     // The absolute time for the next sample.
     TimeTicks next_sample_time;
 
@@ -217,7 +217,7 @@ class StackSamplingProfiler::SamplingThread : public Thread {
   // signalled. The |collection| should already have been removed from
   // |active_collections_| by the caller, as this is needed to avoid flakiness
   // in unit tests.
-  void FinishCollection(CollectionContext* collection);
+  void FinishCollection(std::unique_ptr<CollectionContext> collection);
 
   // Check if the sampling thread is idle and begin a shutdown if it is.
   void ScheduleShutdownIfIdle();
@@ -506,7 +506,7 @@ StackSamplingProfiler::SamplingThread::GetTaskRunnerOnSamplingThread() {
 }
 
 void StackSamplingProfiler::SamplingThread::FinishCollection(
-    CollectionContext* collection) {
+    std::unique_ptr<CollectionContext> collection) {
   DCHECK_EQ(GetThreadId(), PlatformThread::CurrentId());
   DCHECK_EQ(0u, active_collections_.count(collection->collection_id));
 
@@ -518,7 +518,11 @@ void StackSamplingProfiler::SamplingThread::FinishCollection(
       profile_duration, collection->params.sampling_interval);
 
   // Signal that this collection is finished.
-  collection->finished->Signal();
+  WaitableEvent* collection_finished = collection->finished;
+  // Ensure the collection is destroyed before signaling, so that it may
+  // not outlive StackSamplingProfiler.
+  collection.reset();
+  collection_finished->Signal();
 
   ScheduleShutdownIfIdle();
 }
@@ -612,7 +616,7 @@ void StackSamplingProfiler::SamplingThread::RemoveCollectionTask(
   size_t count = active_collections_.erase(collection_id);
   DCHECK_EQ(1U, count);
 
-  FinishCollection(collection.get());
+  FinishCollection(std::move(collection));
 }
 
 void StackSamplingProfiler::SamplingThread::RecordSampleTask(
@@ -658,7 +662,7 @@ void StackSamplingProfiler::SamplingThread::RecordSampleTask(
   DCHECK_EQ(1U, count);
 
   // All capturing has completed so finish the collection.
-  FinishCollection(collection);
+  FinishCollection(std::move(owned_collection));
 }
 
 void StackSamplingProfiler::SamplingThread::ShutdownTask(int add_events) {

base/profiler/stack_sampling_profiler_unittest.cc

@@ -230,22 +230,6 @@ struct TestProfilerInfo {
   DISALLOW_COPY_AND_ASSIGN(TestProfilerInfo);
 };
 
-// Creates multiple profilers based on a vector of parameters.
-std::vector<std::unique_ptr<TestProfilerInfo>> CreateProfilers(
-    SamplingProfilerThreadToken target_thread_token,
-    const std::vector<SamplingParams>& params,
-    ModuleCache* module_cache) {
-  DCHECK(!params.empty());
-
-  std::vector<std::unique_ptr<TestProfilerInfo>> profilers;
-  for (const auto& i : params) {
-    profilers.push_back(std::make_unique<TestProfilerInfo>(target_thread_token,
-                                                           i, module_cache));
-  }
-
-  return profilers;
-}
-
 // Captures samples as specified by |params| on the TargetThread, and returns
 // them. Waits up to |profiler_wait_time| for the profiler to complete.
 std::vector<std::vector<Frame>> CaptureSamples(const SamplingParams& params,
@@ -483,7 +467,7 @@ class TestAuxUnwinder : public Unwinder {
   TestAuxUnwinder(const TestAuxUnwinder&) = delete;
   TestAuxUnwinder& operator=(const TestAuxUnwinder&) = delete;
 
-  void AddInitialModules(ModuleCache* module_cache) override {
+  void InitializeModules(ModuleCache* module_cache) override {
     if (add_initial_modules_callback_)
       add_initial_modules_callback_.Run();
   }
@@ -631,8 +615,8 @@ PROFILER_TEST_F(StackSamplingProfilerTest, StopSafely) {
     size_t count_ = 0;
   };
 
-  WithTargetThread(BindLambdaForTesting(
-      [this](SamplingProfilerThreadToken target_thread_token) {
+  WithTargetThread(
+      BindLambdaForTesting([](SamplingProfilerThreadToken target_thread_token) {
         SamplingParams params[2];
 
         // Providing an initial delay makes it more likely that both will be
@@ -648,11 +632,11 @@ PROFILER_TEST_F(StackSamplingProfilerTest, StopSafely) {
         params[1].samples_per_profile = 100000;
 
         SampleRecordedCounter samples_recorded[size(params)];
-
+        ModuleCache module_cache1, module_cache2;
         TestProfilerInfo profiler_info0(target_thread_token, params[0],
-                                        module_cache(), &samples_recorded[0]);
+                                        &module_cache1, &samples_recorded[0]);
         TestProfilerInfo profiler_info1(target_thread_token, params[1],
-                                        module_cache(), &samples_recorded[1]);
+                                        &module_cache2, &samples_recorded[1]);
 
         profiler_info0.profiler.Start();
         profiler_info1.profiler.Start();
@@ -848,23 +832,26 @@ PROFILER_TEST_F(StackSamplingProfilerTest, CanRunMultipleProfilers) {
 
 // Checks that a sampler can be started while another is running.
 PROFILER_TEST_F(StackSamplingProfilerTest, MultipleStart) {
-  WithTargetThread(BindLambdaForTesting(
-      [this](SamplingProfilerThreadToken target_thread_token) {
-        std::vector<SamplingParams> params(2);
-
-        params[0].initial_delay = AVeryLongTimeDelta();
-        params[0].samples_per_profile = 1;
-
-        params[1].sampling_interval = TimeDelta::FromMilliseconds(1);
-        params[1].samples_per_profile = 1;
+  WithTargetThread(
+      BindLambdaForTesting([](SamplingProfilerThreadToken target_thread_token) {
+        SamplingParams params1;
+        params1.initial_delay = AVeryLongTimeDelta();
+        params1.samples_per_profile = 1;
+        ModuleCache module_cache1;
+        TestProfilerInfo profiler_info1(target_thread_token, params1,
+                                        &module_cache1);
 
-        std::vector<std::unique_ptr<TestProfilerInfo>> profiler_infos =
-            CreateProfilers(target_thread_token, params, module_cache());
+        SamplingParams params2;
+        params2.sampling_interval = TimeDelta::FromMilliseconds(1);
+        params2.samples_per_profile = 1;
+        ModuleCache module_cache2;
+        TestProfilerInfo profiler_info2(target_thread_token, params2,
+                                        &module_cache2);
 
-        profiler_infos[0]->profiler.Start();
-        profiler_infos[1]->profiler.Start();
-        profiler_infos[1]->completed.Wait();
-        EXPECT_EQ(1u, profiler_infos[1]->profile.samples.size());
+        profiler_info1.profiler.Start();
+        profiler_info2.profiler.Start();
+        profiler_info2.completed.Wait();
+        EXPECT_EQ(1u, profiler_info2.profile.samples.size());
       }));
 }
 
@@ -981,20 +968,26 @@ PROFILER_TEST_F(StackSamplingProfilerTest, StopAfterIdleShutdown) {
 // started.
 PROFILER_TEST_F(StackSamplingProfilerTest,
                 ProfileBeforeAndAfterSamplingThreadRunning) {
-  WithTargetThread(BindLambdaForTesting(
-      [this](SamplingProfilerThreadToken target_thread_token) {
-        std::vector<SamplingParams> params(2);
-
-        params[0].initial_delay = AVeryLongTimeDelta();
-        params[0].sampling_interval = TimeDelta::FromMilliseconds(1);
-        params[0].samples_per_profile = 1;
-
-        params[1].initial_delay = TimeDelta::FromMilliseconds(0);
-        params[1].sampling_interval = TimeDelta::FromMilliseconds(1);
-        params[1].samples_per_profile = 1;
+  WithTargetThread(
+      BindLambdaForTesting([](SamplingProfilerThreadToken target_thread_token) {
+        ModuleCache module_cache1;
+        ModuleCache module_cache2;
 
-        std::vector<std::unique_ptr<TestProfilerInfo>> profiler_infos =
-            CreateProfilers(target_thread_token, params, module_cache());
+        std::vector<std::unique_ptr<TestProfilerInfo>> profiler_infos;
+        profiler_infos.push_back(std::make_unique<TestProfilerInfo>(
+            target_thread_token,
+            SamplingParams{
+                /*initial_delay=*/AVeryLongTimeDelta(),
+                /*samples_per_profile=*/1,
+                /*sampling_interval=*/TimeDelta::FromMilliseconds(1)},
+            &module_cache1));
+        profiler_infos.push_back(std::make_unique<TestProfilerInfo>(
+            target_thread_token,
+            SamplingParams{
+                /*initial_delay=*/TimeDelta::FromMilliseconds(0),
+                /*samples_per_profile=*/1,
+                /*sampling_interval=*/TimeDelta::FromMilliseconds(1)},
+            &module_cache2));
 
         // First profiler is started when there has never been a sampling
         // thread.
@@ -1053,9 +1046,9 @@ PROFILER_TEST_F(StackSamplingProfilerTest, IdleShutdownAbort) {
 
 // Checks that synchronized multiple sampling requests execute in parallel.
 PROFILER_TEST_F(StackSamplingProfilerTest, ConcurrentProfiling_InSync) {
-  WithTargetThread(BindLambdaForTesting(
-      [this](SamplingProfilerThreadToken target_thread_token) {
-        std::vector<SamplingParams> params(2);
+  WithTargetThread(
+      BindLambdaForTesting([](SamplingProfilerThreadToken target_thread_token) {
+        std::vector<ModuleCache> module_caches(2);
 
         // Providing an initial delay makes it more likely that both will be
         // scheduled before either starts to run. Once started, samples will
@@ -1063,16 +1056,21 @@ PROFILER_TEST_F(StackSamplingProfilerTest, ConcurrentProfiling_InSync) {
         // whatever interval the thread wakes up. Thus, total execution time
         // will be 10ms (delay) + 10x1ms (sampling) + 1/2 timer minimum
         // interval.
-        params[0].initial_delay = TimeDelta::FromMilliseconds(10);
-        params[0].sampling_interval = TimeDelta::FromMilliseconds(1);
-        params[0].samples_per_profile = 9;
-
-        params[1].initial_delay = TimeDelta::FromMilliseconds(11);
-        params[1].sampling_interval = TimeDelta::FromMilliseconds(1);
-        params[1].samples_per_profile = 8;
-
-        std::vector<std::unique_ptr<TestProfilerInfo>> profiler_infos =
-            CreateProfilers(target_thread_token, params, module_cache());
+        std::vector<std::unique_ptr<TestProfilerInfo>> profiler_infos;
+        profiler_infos.push_back(std::make_unique<TestProfilerInfo>(
+            target_thread_token,
+            SamplingParams{
+                /*initial_delay=*/TimeDelta::FromMilliseconds(10),
+                /*samples_per_profile=*/9,
+                /*sampling_interval=*/TimeDelta::FromMilliseconds(1)},
+            &module_caches[0]));
+        profiler_infos.push_back(std::make_unique<TestProfilerInfo>(
+            target_thread_token,
+            SamplingParams{
+                /*initial_delay=*/TimeDelta::FromMilliseconds(11),
+                /*samples_per_profile=*/8,
+                /*sampling_interval=*/TimeDelta::FromMilliseconds(1)},
+            &module_caches[1]));
 
         profiler_infos[0]->profiler.Start();
         profiler_infos[1]->profiler.Start();
@@ -1092,39 +1090,43 @@ PROFILER_TEST_F(StackSamplingProfilerTest, ConcurrentProfiling_InSync) {
 
 // Checks that several mixed sampling requests execute in parallel.
 PROFILER_TEST_F(StackSamplingProfilerTest, ConcurrentProfiling_Mixed) {
-  WithTargetThread(BindLambdaForTesting(
-      [this](SamplingProfilerThreadToken target_thread_token) {
-        std::vector<SamplingParams> params(3);
-
-        params[0].initial_delay = TimeDelta::FromMilliseconds(8);
-        params[0].sampling_interval = TimeDelta::FromMilliseconds(4);
-        params[0].samples_per_profile = 10;
-
-        params[1].initial_delay = TimeDelta::FromMilliseconds(9);
-        params[1].sampling_interval = TimeDelta::FromMilliseconds(3);
-        params[1].samples_per_profile = 10;
-
-        params[2].initial_delay = TimeDelta::FromMilliseconds(10);
-        params[2].sampling_interval = TimeDelta::FromMilliseconds(2);
-        params[2].samples_per_profile = 10;
-
-        std::vector<std::unique_ptr<TestProfilerInfo>> profiler_infos =
-            CreateProfilers(target_thread_token, params, module_cache());
-
-        for (auto& i : profiler_infos)
-          i->profiler.Start();
-
-        // Wait for one profiler to finish.
-        size_t completed_profiler = WaitForSamplingComplete(profiler_infos);
-        EXPECT_EQ(10u,
-                  profiler_infos[completed_profiler]->profile.samples.size());
-        // Stop and destroy all profilers, always in the same order. Don't
-        // crash.
-        for (auto& i : profiler_infos)
-          i->profiler.Stop();
-        for (auto& i : profiler_infos)
-          i.reset();
-      }));
+  WithTargetThread(BindLambdaForTesting([](SamplingProfilerThreadToken
+                                               target_thread_token) {
+    std::vector<ModuleCache> module_caches(3);
+
+    std::vector<std::unique_ptr<TestProfilerInfo>> profiler_infos;
+    profiler_infos.push_back(std::make_unique<TestProfilerInfo>(
+        target_thread_token,
+        SamplingParams{/*initial_delay=*/TimeDelta::FromMilliseconds(8),
+                       /*samples_per_profile=*/10,
+                       /*sampling_interval=*/TimeDelta::FromMilliseconds(4)},
+        &module_caches[0]));
+    profiler_infos.push_back(std::make_unique<TestProfilerInfo>(
+        target_thread_token,
+        SamplingParams{/*initial_delay=*/TimeDelta::FromMilliseconds(9),
+                       /*samples_per_profile=*/10,
+                       /*sampling_interval=*/TimeDelta::FromMilliseconds(3)},
+        &module_caches[1]));
+    profiler_infos.push_back(std::make_unique<TestProfilerInfo>(
+        target_thread_token,
+        SamplingParams{/*initial_delay=*/TimeDelta::FromMilliseconds(10),
+                       /*samples_per_profile=*/10,
+                       /*sampling_interval=*/TimeDelta::FromMilliseconds(2)},
+        &module_caches[2]));
+
+    for (auto& i : profiler_infos)
+      i->profiler.Start();
+
+    // Wait for one profiler to finish.
+    size_t completed_profiler = WaitForSamplingComplete(profiler_infos);
+    EXPECT_EQ(10u, profiler_infos[completed_profiler]->profile.samples.size());
+    // Stop and destroy all profilers, always in the same order. Don't
+    // crash.
+    for (auto& i : profiler_infos)
+      i->profiler.Stop();
+    for (auto& i : profiler_infos)
+      i.reset();
+  }));
 }
 
 // Checks that different threads can be sampled in parallel.
@@ -1161,6 +1163,7 @@ PROFILER_TEST_F(StackSamplingProfilerTest, MultipleSampledThreads) {
   params2.samples_per_profile = 8;
 
   Profile profile1, profile2;
+  ModuleCache module_cache1, module_cache2;
 
   WaitableEvent sampling_thread_completed1(
       WaitableEvent::ResetPolicy::MANUAL,
@@ -1168,13 +1171,13 @@ PROFILER_TEST_F(StackSamplingProfilerTest, MultipleSampledThreads) {
   StackSamplingProfiler profiler1(
       target_thread1.thread_token(), params1,
       std::make_unique<TestProfileBuilder>(
-          module_cache(),
+          &module_cache1,
           BindLambdaForTesting(
               [&profile1, &sampling_thread_completed1](Profile result_profile) {
                 profile1 = std::move(result_profile);
                 sampling_thread_completed1.Signal();
               })),
-      CreateCoreUnwindersFactoryForTesting(module_cache()));
+      CreateCoreUnwindersFactoryForTesting(&module_cache1));
 
   WaitableEvent sampling_thread_completed2(
       WaitableEvent::ResetPolicy::MANUAL,
@@ -1182,13 +1185,13 @@ PROFILER_TEST_F(StackSamplingProfilerTest, MultipleSampledThreads) {
   StackSamplingProfiler profiler2(
       target_thread2.thread_token(), params2,
       std::make_unique<TestProfileBuilder>(
-          module_cache(),
+          &module_cache2,
           BindLambdaForTesting(
               [&profile2, &sampling_thread_completed2](Profile result_profile) {
                 profile2 = std::move(result_profile);
                 sampling_thread_completed2.Signal();
               })),
-      CreateCoreUnwindersFactoryForTesting(module_cache()));
+      CreateCoreUnwindersFactoryForTesting(&module_cache2));
 
   // Finally the real work.
   profiler1.Start();

base/profiler/unwinder.h

@@ -36,9 +36,12 @@ class Unwinder {
  public:
   virtual ~Unwinder() = default;
 
-  // Invoked to allow the unwinder to add any modules it recognizes to the
-  // ModuleCache.
-  virtual void AddInitialModules(ModuleCache* module_cache) {}
+  // Invoked to allow the unwinder to add any modules it recognizes or register
+  // a module factory to the ModuleCache. This associates this Unwinder with
+  // |module_cache| for the remaining of its lifetime, which is reused in
+  // subsequent methods UpdateModules() and TryUnwinder(). Thus, |module_cache|
+  // must outlive this Unwinder.
+  virtual void InitializeModules(ModuleCache* module_cache) {}
 
   // Invoked at the time the stack is captured. IMPORTANT NOTE: this function is
   // invoked while the target thread is suspended. To avoid deadlock it must not

chrome/renderer/v8_unwinder.cc

@@ -85,7 +85,7 @@ V8Unwinder::V8Unwinder(v8::Isolate* isolate)
 
 V8Unwinder::~V8Unwinder() = default;
 
-void V8Unwinder::AddInitialModules(base::ModuleCache* module_cache) {
+void V8Unwinder::InitializeModules(base::ModuleCache* module_cache) {
   // This function must be called only once.
   DCHECK(modules_.empty());
 
@@ -109,7 +109,7 @@ void V8Unwinder::OnStackCapture() {
 }
 
 // Update the modules based on what was recorded in |code_ranges_|. The singular
-// embedded code range was already added in in AddInitialModules(). It is
+// embedded code range was already added in in InitializeModules(). It is
 // preserved by the algorithm below, which is why kNonEmbedded is
 // unconditionally passed when creating new modules.
 void V8Unwinder::UpdateModules(base::ModuleCache* module_cache) {

chrome/renderer/v8_unwinder.h

@@ -22,7 +22,7 @@ class V8Unwinder : public base::Unwinder {
   V8Unwinder& operator=(const V8Unwinder&) = delete;
 
   // Unwinder:
-  void AddInitialModules(base::ModuleCache* module_cache) override;
+  void InitializeModules(base::ModuleCache* module_cache) override;
   void OnStackCapture() override;
   void UpdateModules(base::ModuleCache* module_cache) override;
   bool CanUnwindFrom(const base::Frame& current_frame) const override;

chrome/renderer/v8_unwinder_unittest.cc

@@ -222,7 +222,7 @@ TEST(V8UnwinderTest, EmbeddedCodeRangeModule) {
   V8Unwinder unwinder(v8_environment.isolate());
   base::ModuleCache module_cache;
 
-  unwinder.AddInitialModules(&module_cache);
+  unwinder.InitializeModules(&module_cache);
 
   v8::MemoryRange embedded_code_range;
   v8_environment.isolate()->GetEmbeddedCodeRange(
@@ -239,7 +239,7 @@ TEST(V8UnwinderTest, EmbeddedCodeRangeModulePreservedOnUpdate) {
   UpdateModulesTestUnwinder unwinder(v8_environment.isolate());
   base::ModuleCache module_cache;
 
-  unwinder.AddInitialModules(&module_cache);
+  unwinder.InitializeModules(&module_cache);
 
   unwinder.SetCodePages({{reinterpret_cast<void*>(1), 10},
                          GetEmbeddedCodeRange(v8_environment.isolate())});
@@ -264,7 +264,7 @@ TEST(V8UnwinderTest, EmbeddedCodeRangeModulePreservedOnOverCapacityUpdate) {
   UpdateModulesTestUnwinder unwinder(v8_environment.isolate());
   base::ModuleCache module_cache;
 
-  unwinder.AddInitialModules(&module_cache);
+  unwinder.InitializeModules(&module_cache);
 
   const int kDefaultCapacity = v8::Isolate::kMinCodePagesBufferSize;
   std::vector<v8::MemoryRange> code_pages;
@@ -291,7 +291,7 @@ TEST(V8UnwinderTest, UpdateModules_ModuleAdded) {
   UpdateModulesTestUnwinder unwinder(v8_environment.isolate());
   base::ModuleCache module_cache;
 
-  unwinder.AddInitialModules(&module_cache);
+  unwinder.InitializeModules(&module_cache);
   unwinder.SetCodePages({{reinterpret_cast<void*>(1), 10},
                          GetEmbeddedCodeRange(v8_environment.isolate())});
   unwinder.OnStackCapture();
@@ -312,7 +312,7 @@ TEST(V8UnwinderTest, UpdateModules_ModuleAddedBeforeLast) {
   UpdateModulesTestUnwinder unwinder(v8_environment.isolate());
   base::ModuleCache module_cache;
 
-  unwinder.AddInitialModules(&module_cache);
+  unwinder.InitializeModules(&module_cache);
 
   unwinder.SetCodePages({{reinterpret_cast<void*>(100), 10},
                          GetEmbeddedCodeRange(v8_environment.isolate())});
@@ -338,7 +338,7 @@ TEST(V8UnwinderTest, UpdateModules_ModuleRetained) {
   UpdateModulesTestUnwinder unwinder(v8_environment.isolate());
   base::ModuleCache module_cache;
 
-  unwinder.AddInitialModules(&module_cache);
+  unwinder.InitializeModules(&module_cache);
 
   unwinder.SetCodePages({{reinterpret_cast<void*>(1), 10},
                          GetEmbeddedCodeRange(v8_environment.isolate())});
@@ -362,7 +362,7 @@ TEST(V8UnwinderTest, UpdateModules_ModuleRetainedWithDifferentSize) {
   UpdateModulesTestUnwinder unwinder(v8_environment.isolate());
   base::ModuleCache module_cache;
 
-  unwinder.AddInitialModules(&module_cache);
+  unwinder.InitializeModules(&module_cache);
 
   unwinder.SetCodePages({{reinterpret_cast<void*>(1), 10},
                          GetEmbeddedCodeRange(v8_environment.isolate())});
@@ -387,7 +387,7 @@ TEST(V8UnwinderTest, UpdateModules_ModuleRemoved) {
   UpdateModulesTestUnwinder unwinder(v8_environment.isolate());
   base::ModuleCache module_cache;
 
-  unwinder.AddInitialModules(&module_cache);
+  unwinder.InitializeModules(&module_cache);
 
   unwinder.SetCodePages({{{reinterpret_cast<void*>(1), 10},
                           GetEmbeddedCodeRange(v8_environment.isolate())}});
@@ -408,7 +408,7 @@ TEST(V8UnwinderTest, UpdateModules_ModuleRemovedBeforeLast) {
   UpdateModulesTestUnwinder unwinder(v8_environment.isolate());
   base::ModuleCache module_cache;
 
-  unwinder.AddInitialModules(&module_cache);
+  unwinder.InitializeModules(&module_cache);
 
   unwinder.SetCodePages({{{reinterpret_cast<void*>(1), 10},
                           {reinterpret_cast<void*>(100), 10},
@@ -429,7 +429,7 @@ TEST(V8UnwinderTest, UpdateModules_CapacityExceeded) {
   UpdateModulesTestUnwinder unwinder(v8_environment.isolate());
   base::ModuleCache module_cache;
 
-  unwinder.AddInitialModules(&module_cache);
+  unwinder.InitializeModules(&module_cache);
 
   const int kDefaultCapacity = v8::Isolate::kMinCodePagesBufferSize;
 
@@ -466,7 +466,7 @@ TEST(V8UnwinderTest, UpdateModules_CapacitySubstantiallyExceeded) {
   UpdateModulesTestUnwinder unwinder(v8_environment.isolate());
   base::ModuleCache module_cache;
 
-  unwinder.AddInitialModules(&module_cache);
+  unwinder.InitializeModules(&module_cache);
 
   const int kDefaultCapacity = v8::Isolate::kMinCodePagesBufferSize;
   const int kCodePages = kDefaultCapacity * 3;
@@ -500,7 +500,7 @@ TEST(V8UnwinderTest, CanUnwindFrom_V8Module) {
   UpdateModulesTestUnwinder unwinder(v8_environment.isolate());
   base::ModuleCache module_cache;
 
-  unwinder.AddInitialModules(&module_cache);
+  unwinder.InitializeModules(&module_cache);
 
   unwinder.SetCodePages({{reinterpret_cast<void*>(1), 10},
                          GetEmbeddedCodeRange(v8_environment.isolate())});
@@ -518,7 +518,7 @@ TEST(V8UnwinderTest, CanUnwindFrom_OtherModule) {
   UpdateModulesTestUnwinder unwinder(v8_environment.isolate());
   base::ModuleCache module_cache;
 
-  unwinder.AddInitialModules(&module_cache);
+  unwinder.InitializeModules(&module_cache);
 
   unwinder.SetCodePages({GetEmbeddedCodeRange(v8_environment.isolate())});
   unwinder.OnStackCapture();
@@ -536,7 +536,7 @@ TEST(V8UnwinderTest, CanUnwindFrom_NullModule) {
   UpdateModulesTestUnwinder unwinder(v8_environment.isolate());
   base::ModuleCache module_cache;
 
-  unwinder.AddInitialModules(&module_cache);
+  unwinder.InitializeModules(&module_cache);
 
   // Insert a non-native module to potentially exercise the Module comparator.
   unwinder.SetCodePages({{reinterpret_cast<void*>(1), 10},