refactor: omit redundant map searches (#19929)

* refactor: don't walk maps twice to remove elements

* refactor: don't walk maps twice to read elements

* refactor: don't walk maps twice to insert elements

* refactor: don't walk map 3x on UvTaskRunner timeout

* refactor: more don't-walk-maps-twice cleanup

* fixup! refactor: don't walk maps twice to insert elements

* refactor: don't walk containers twice when erasing

* refactor: omit excess lookups in RemoteObjectFreer

shell/app/uv_task_runner.cc

@@ -42,12 +42,13 @@ bool UvTaskRunner::PostNonNestableDelayedTask(const base::Location& from_here,
 
 // static
 void UvTaskRunner::OnTimeout(uv_timer_t* timer) {
-  UvTaskRunner* self = static_cast<UvTaskRunner*>(timer->data);
+  auto& tasks = static_cast<UvTaskRunner*>(timer->data)->tasks_;
-  if (!base::Contains(self->tasks_, timer))
+  const auto iter = tasks.find(timer);
+  if (iter == std::end(tasks))
     return;
 
-  std::move(self->tasks_[timer]).Run();
+  std::move(iter->second).Run();
-  self->tasks_.erase(timer);
+  tasks.erase(iter);
   uv_timer_stop(timer);
   uv_close(reinterpret_cast<uv_handle_t*>(timer), UvTaskRunner::OnClose);
 }

shell/browser/api/atom_api_global_shortcut.cc

@@ -107,10 +107,9 @@ bool GlobalShortcut::Register(const ui::Accelerator& accelerator,
 }
 
 void GlobalShortcut::Unregister(const ui::Accelerator& accelerator) {
-  if (!base::Contains(accelerator_callback_map_, accelerator))
+  if (accelerator_callback_map_.erase(accelerator) == 0)
     return;
 
-  accelerator_callback_map_.erase(accelerator);
   GlobalShortcutListener::GetInstance()->UnregisterAccelerator(accelerator,
                                                                this);
 }

shell/browser/api/atom_api_protocol_ns.cc

@@ -176,21 +176,15 @@ void ProtocolNS::RegisterURLLoaderFactories(
 ProtocolError ProtocolNS::RegisterProtocol(ProtocolType type,
                                            const std::string& scheme,
                                            const ProtocolHandler& handler) {
-  ProtocolError error = ProtocolError::OK;
+  const bool added = base::TryEmplace(handlers_, scheme, type, handler).second;
-  if (!base::Contains(handlers_, scheme))
+  return added ? ProtocolError::OK : ProtocolError::REGISTERED;
-    handlers_[scheme] = std::make_pair(type, handler);
-  else
-    error = ProtocolError::REGISTERED;
-  return error;
 }
 
 void ProtocolNS::UnregisterProtocol(const std::string& scheme,
                                     mate::Arguments* args) {
-  ProtocolError error = ProtocolError::OK;
+  const bool removed = handlers_.erase(scheme) != 0;
-  if (base::Contains(handlers_, scheme))
+  const auto error =
-    handlers_.erase(scheme);
+      removed ? ProtocolError::OK : ProtocolError::NOT_REGISTERED;
-  else
-    error = ProtocolError::NOT_REGISTERED;
   HandleOptionalCallback(args, error);
 }
 
@@ -201,21 +195,16 @@ bool ProtocolNS::IsProtocolRegistered(const std::string& scheme) {
 ProtocolError ProtocolNS::InterceptProtocol(ProtocolType type,
                                             const std::string& scheme,
                                             const ProtocolHandler& handler) {
-  ProtocolError error = ProtocolError::OK;
+  const bool added =
-  if (!base::Contains(intercept_handlers_, scheme))
+      base::TryEmplace(intercept_handlers_, scheme, type, handler).second;
-    intercept_handlers_[scheme] = std::make_pair(type, handler);
+  return added ? ProtocolError::OK : ProtocolError::INTERCEPTED;
-  else
-    error = ProtocolError::INTERCEPTED;
-  return error;
 }
 
 void ProtocolNS::UninterceptProtocol(const std::string& scheme,
                                      mate::Arguments* args) {
-  ProtocolError error = ProtocolError::OK;
+  const bool removed = intercept_handlers_.erase(scheme) != 0;
-  if (base::Contains(intercept_handlers_, scheme))
+  const auto error =
-    intercept_handlers_.erase(scheme);
+      removed ? ProtocolError::OK : ProtocolError::NOT_INTERCEPTED;
-  else
-    error = ProtocolError::NOT_INTERCEPTED;
   HandleOptionalCallback(args, error);
 }
 

shell/browser/api/atom_api_top_level_window.cc

@@ -972,9 +972,6 @@ bool TopLevelWindow::HookWindowMessage(UINT message,
 }
 
 void TopLevelWindow::UnhookWindowMessage(UINT message) {
-  if (!base::Contains(messages_callback_map_, message))
-    return;
-
   messages_callback_map_.erase(message);
 }
 

shell/browser/api/atom_api_web_request_ns.cc

@@ -396,10 +396,11 @@ template <typename... Args>
 void WebRequestNS::HandleSimpleEvent(SimpleEvent event,
                                      extensions::WebRequestInfo* request_info,
                                      Args... args) {
-  if (!base::Contains(simple_listeners_, event))
+  const auto iter = simple_listeners_.find(event);
+  if (iter == std::end(simple_listeners_))
     return;
 
-  const auto& info = simple_listeners_[event];
+  const auto& info = iter->second;
   if (!MatchesFilterCondition(request_info, info.url_patterns))
     return;
 
@@ -416,10 +417,11 @@ int WebRequestNS::HandleResponseEvent(ResponseEvent event,
                                       net::CompletionOnceCallback callback,
                                       Out out,
                                       Args... args) {
-  if (!base::Contains(response_listeners_, event))
+  const auto iter = response_listeners_.find(event);
+  if (iter == std::end(response_listeners_))
     return net::OK;
 
-  const auto& info = response_listeners_[event];
+  const auto& info = iter->second;
   if (!MatchesFilterCondition(request_info, info.url_patterns))
     return net::OK;
 
@@ -441,7 +443,8 @@ template <typename T>
 void WebRequestNS::OnListenerResult(uint64_t id,
                                     T out,
                                     v8::Local<v8::Value> response) {
-  if (!base::Contains(callbacks_, id))
+  const auto iter = callbacks_.find(id);
+  if (iter == std::end(callbacks_))
     return;
 
   int result = net::OK;
@@ -461,7 +464,7 @@ void WebRequestNS::OnListenerResult(uint64_t id,
   // asynchronously, because it used to work on IO thread before NetworkService.
   base::SequencedTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::BindOnce(std::move(callbacks_[id]), result));
-  callbacks_.erase(id);
+  callbacks_.erase(iter);
 }
 
 // static

shell/browser/atom_browser_client.cc

@@ -197,8 +197,9 @@ content::WebContents* AtomBrowserClient::GetWebContentsFromProcessID(
     int process_id) {
   // If the process is a pending process, we should use the web contents
   // for the frame host passed into RegisterPendingProcess.
-  if (base::Contains(pending_processes_, process_id))
+  const auto iter = pending_processes_.find(process_id);
-    return pending_processes_[process_id];
+  if (iter != std::end(pending_processes_))
+    return iter->second;
 
   // Certain render process will be created with no associated render view,
   // for example: ServiceWorker.

shell/browser/ui/accelerator_util.cc

@@ -89,8 +89,9 @@ void GenerateAcceleratorTable(AcceleratorTable* table,
 
 bool TriggerAcceleratorTableCommand(AcceleratorTable* table,
                                     const ui::Accelerator& accelerator) {
-  if (base::Contains(*table, accelerator)) {
+  const auto iter = table->find(accelerator);
-    const accelerator_util::MenuItem& item = (*table)[accelerator];
+  if (iter != std::end(*table)) {
+    const accelerator_util::MenuItem& item = iter->second;
     if (item.model->IsEnabledAt(item.position)) {
       const auto event_flags =
           accelerator.MaskOutKeyEventFlags(accelerator.modifiers());

shell/browser/ui/atom_menu_model.cc

@@ -25,11 +25,9 @@ void AtomMenuModel::SetToolTip(int index, const base::string16& toolTip) {
 }
 
 base::string16 AtomMenuModel::GetToolTipAt(int index) {
-  int command_id = GetCommandIdAt(index);
+  const int command_id = GetCommandIdAt(index);
-  if (base::Contains(toolTips_, command_id))
+  const auto iter = toolTips_.find(command_id);
-    return toolTips_[command_id];
+  return iter == std::end(toolTips_) ? base::string16() : iter->second;
-  else
-    return base::string16();
 }
 
 void AtomMenuModel::SetRole(int index, const base::string16& role) {
@@ -38,11 +36,9 @@ void AtomMenuModel::SetRole(int index, const base::string16& role) {
 }
 
 base::string16 AtomMenuModel::GetRoleAt(int index) {
-  int command_id = GetCommandIdAt(index);
+  const int command_id = GetCommandIdAt(index);
-  if (base::Contains(roles_, command_id))
+  const auto iter = roles_.find(command_id);
-    return roles_[command_id];
+  return iter == std::end(roles_) ? base::string16() : iter->second;
-  else
-    return base::string16();
 }
 
 bool AtomMenuModel::GetAcceleratorAtWithParams(

shell/browser/ui/win/taskbar_host.cc

@@ -199,8 +199,9 @@ bool TaskbarHost::SetThumbnailToolTip(HWND window, const std::string& tooltip) {
 }
 
 bool TaskbarHost::HandleThumbarButtonEvent(int button_id) {
-  if (base::Contains(callback_map_, button_id)) {
+  const auto iter = callback_map_.find(button_id);
-    auto callback = callback_map_[button_id];
+  if (iter != std::end(callback_map_)) {
+    auto callback = iter->second;
     if (!callback.is_null())
       callback.Run();
     return true;

shell/browser/web_view_manager.cc

@@ -28,11 +28,9 @@ void WebViewManager::AddGuest(int guest_instance_id,
 }
 
 void WebViewManager::RemoveGuest(int guest_instance_id) {
-  if (!base::Contains(web_contents_embedder_map_, guest_instance_id))
+  if (web_contents_embedder_map_.erase(guest_instance_id) == 0)
     return;
 
-  web_contents_embedder_map_.erase(guest_instance_id);
-
   // Remove the record of element in embedder too.
   for (const auto& element : element_instance_id_to_guest_map_)
     if (element.second == guest_instance_id) {
@@ -42,24 +40,24 @@ void WebViewManager::RemoveGuest(int guest_instance_id) {
 }
 
 content::WebContents* WebViewManager::GetEmbedder(int guest_instance_id) {
-  if (base::Contains(web_contents_embedder_map_, guest_instance_id))
+  const auto iter = web_contents_embedder_map_.find(guest_instance_id);
-    return web_contents_embedder_map_[guest_instance_id].embedder;
+  return iter == std::end(web_contents_embedder_map_) ? nullptr
-  else
+                                                      : iter->second.embedder;
-    return nullptr;
 }
 
 content::WebContents* WebViewManager::GetGuestByInstanceID(
     int owner_process_id,
     int element_instance_id) {
-  ElementInstanceKey key(owner_process_id, element_instance_id);
+  const ElementInstanceKey key(owner_process_id, element_instance_id);
-  if (!base::Contains(element_instance_id_to_guest_map_, key))
+  const auto guest_iter = element_instance_id_to_guest_map_.find(key);
+  if (guest_iter == std::end(element_instance_id_to_guest_map_))
     return nullptr;
 
-  int guest_instance_id = element_instance_id_to_guest_map_[key];
+  const int guest_instance_id = guest_iter->second;
-  if (base::Contains(web_contents_embedder_map_, guest_instance_id))
+  const auto iter = web_contents_embedder_map_.find(guest_instance_id);
-    return web_contents_embedder_map_[guest_instance_id].web_contents;
+  return iter == std::end(web_contents_embedder_map_)
-  else
+             ? nullptr
-    return nullptr;
+             : iter->second.web_contents;
 }
 
 bool WebViewManager::ForEachGuest(content::WebContents* embedder_web_contents,

shell/common/api/remote_object_freer.cc

@@ -65,16 +65,27 @@ void RemoteObjectFreer::RunDestructor() {
   if (!render_frame)
     return;
 
+  // Reset our local ref count in case we are in a GC race condition
+  // and will get more references in an inbound IPC message
+  int ref_count = 0;
+  const auto objects_it = ref_mapper_.find(context_id_);
+  if (objects_it != std::end(ref_mapper_)) {
+    auto& objects = objects_it->second;
+    const auto ref_it = objects.find(object_id_);
+    if (ref_it != std::end(objects)) {
+      ref_count = ref_it->second;
+      objects.erase(ref_it);
+    }
+    if (objects.empty())
+      ref_mapper_.erase(objects_it);
+  }
+
   auto* channel = "ELECTRON_BROWSER_DEREFERENCE";
+
   base::ListValue args;
   args.AppendString(context_id_);
   args.AppendInteger(object_id_);
-  args.AppendInteger(ref_mapper_[context_id_][object_id_]);
+  args.AppendInteger(ref_count);
-  // Reset our local ref count in case we are in a GC race condition and will
-  // get more references in an inbound IPC message
-  ref_mapper_[context_id_].erase(object_id_);
-  if (ref_mapper_[context_id_].empty())
-    ref_mapper_.erase(context_id_);
 
   mojom::ElectronBrowserAssociatedPtr electron_ptr;
   render_frame->GetRemoteAssociatedInterfaces()->GetInterface(

shell/common/asar/asar_util.cc

@@ -30,14 +30,22 @@ const base::FilePath::CharType kAsarExtension[] = FILE_PATH_LITERAL(".asar");
 std::shared_ptr<Archive> GetOrCreateAsarArchive(const base::FilePath& path) {
   if (!g_archive_map_tls.Pointer()->Get())
     g_archive_map_tls.Pointer()->Set(new ArchiveMap);
-  ArchiveMap& archive_map = *g_archive_map_tls.Pointer()->Get();
+  ArchiveMap& map = *g_archive_map_tls.Pointer()->Get();
-  if (!base::Contains(archive_map, path)) {
+
-    std::shared_ptr<Archive> archive(new Archive(path));
+  // if we have it, return it
-    if (!archive->Init())
+  const auto lower = map.lower_bound(path);
-      return nullptr;
+  if (lower != std::end(map) && !map.key_comp()(path, lower->first))
-    archive_map[path] = archive;
+    return lower->second;
+
+  // if we can create it, return it
+  auto archive = std::make_shared<Archive>(path);
+  if (archive->Init()) {
+    base::TryEmplace(map, lower, path, archive);
+    return archive;
   }
-  return archive_map[path];
+
+  // didn't have it, couldn't create it
+  return nullptr;
 }
 
 void ClearArchives() {

shell/renderer/atom_renderer_client.cc

@@ -153,14 +153,12 @@ void AtomRendererClient::DidCreateScriptContext(
 void AtomRendererClient::WillReleaseScriptContext(
     v8::Handle<v8::Context> context,
     content::RenderFrame* render_frame) {
-  if (injected_frames_.find(render_frame) == injected_frames_.end())
+  if (injected_frames_.erase(render_frame) == 0)
     return;
-  injected_frames_.erase(render_frame);
 
   node::Environment* env = node::Environment::GetCurrent(context);
-  if (environments_.find(env) == environments_.end())
+  if (environments_.erase(env) == 0)
     return;
-  environments_.erase(env);
 
   mate::EmitEvent(env->isolate(), env->process_object(), "exit");
 

shell/renderer/atom_sandboxed_renderer_client.cc

@@ -294,9 +294,8 @@ void AtomSandboxedRendererClient::SetupExtensionWorldOverrides(
 void AtomSandboxedRendererClient::WillReleaseScriptContext(
     v8::Handle<v8::Context> context,
     content::RenderFrame* render_frame) {
-  if (injected_frames_.find(render_frame) == injected_frames_.end())
+  if (injected_frames_.erase(render_frame) == 0)
     return;
-  injected_frames_.erase(render_frame);
 
   auto* isolate = context->GetIsolate();
   v8::HandleScope handle_scope(isolate);