Use FrameTreeNode ID as frameId in extension APIs

extensions/browser/extension_api_frame_id_map.cc

Index: extensions/browser/extension_api_frame_id_map.cc
diff --git a/extensions/browser/extension_api_frame_id_map.cc b/extensions/browser/extension_api_frame_id_map.cc
new file mode 100644
index 0000000000000000000000000000000000000000..5d947b70057fcf512b4318c89749148ad0302b7d
--- /dev/null
+++ b/extensions/browser/extension_api_frame_id_map.cc
@@ -0,0 +1,269 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "extensions/browser/extension_api_frame_id_map.h"
+
+#include <tuple>
+
+#include "base/time/time.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/web_contents.h"
+
+namespace extensions {
+
+namespace {
+
+// The map is accessed on the IO and UI thread, so construct it once and never
+// delete it.
+base::LazyInstance<ExtensionApiFrameIdMap>::Leaky g_map_instance =
+    LAZY_INSTANCE_INITIALIZER;
+
+int GetFrameIdFromFrame(content::RenderFrameHost* rfh) {
+  if (!rfh)
+    return ExtensionApiFrameId::kInvalidFrameId;
+  if (rfh->GetParent())
+    return rfh->GetFrameTreeNodeId();
+  return 0;  // Main frame.
+}
+
+}  // namespace
+
+ExtensionApiFrameId::ExtensionApiFrameId()
+    : frame_id(kInvalidFrameId), parent_frame_id(kInvalidFrameId) {}
+
+ExtensionApiFrameId::ExtensionApiFrameId(int frame_id, int parent_frame_id)
+    : frame_id(frame_id), parent_frame_id(parent_frame_id) {}
+
+ExtensionApiFrameIdMap::RenderFrameIdKey::RenderFrameIdKey()
+    : render_process_id(-1), frame_routing_id(-1) {}
+
+ExtensionApiFrameIdMap::RenderFrameIdKey::RenderFrameIdKey(
+    int render_process_id,
+    int frame_routing_id)
+    : render_process_id(render_process_id),
+      frame_routing_id(frame_routing_id) {}
+
+ExtensionApiFrameIdMap::FrameIdCallbackInfo::FrameIdCallbackInfo(
+    const RenderFrameIdKey& key,
+    const FrameIdCallback& callback)
+    : key(key), callback(callback), has_value(false) {}
+
+ExtensionApiFrameIdMap::FrameIdCallbackInfo::~FrameIdCallbackInfo() {}
+
+bool ExtensionApiFrameIdMap::RenderFrameIdKey::operator<(
+    const RenderFrameIdKey& other) const {
+  return std::tie(render_process_id, frame_routing_id) <
+         std::tie(other.render_process_id, other.frame_routing_id);
+}
+
+bool ExtensionApiFrameIdMap::RenderFrameIdKey::operator==(
+    const RenderFrameIdKey& other) const {
+  return render_process_id == other.render_process_id &&
+         frame_routing_id == other.frame_routing_id;
+}
+
+ExtensionApiFrameIdMap::FrameIdRemovalTask::FrameIdRemovalTask(
+    const RenderFrameIdKey& key,
+    const base::TimeTicks& creation_time)
+    : key(key), creation_time(creation_time) {}
+
+ExtensionApiFrameIdMap::ExtensionApiFrameIdMap() {}
+
+ExtensionApiFrameIdMap::~ExtensionApiFrameIdMap() {}
+
+ExtensionApiFrameIdMap* ExtensionApiFrameIdMap::Get() {
+  return g_map_instance.Pointer();
+}
+
+ExtensionApiFrameId ExtensionApiFrameIdMap::KeyToValue(
+    const RenderFrameIdKey& key) const {
+  content::RenderFrameHost* rfh = content::RenderFrameHost::FromID(
+      key.render_process_id, key.frame_routing_id);
+  return ExtensionApiFrameId(
+      GetFrameIdFromFrame(rfh),
+      GetFrameIdFromFrame(rfh ? rfh->GetParent() : nullptr));
+}
+
+ExtensionApiFrameId ExtensionApiFrameIdMap::LookupFrameIdOnUI(
+    const RenderFrameIdKey& key) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  FrameIdMap::const_iterator frame_id_iter = frame_id_map_.find(key);
+  if (frame_id_iter != frame_id_map_.end())
+    return frame_id_iter->second;
+
+  const ExtensionApiFrameId& extension_api_frame_id = KeyToValue(key);
+  // Don't save invalid values in the map.

[nasko, 2015/12/22 22:12:09]

nit: Comment above the variable or empty line between the two.

+  if (extension_api_frame_id.frame_id == ExtensionApiFrameId::kInvalidFrameId)
+    return extension_api_frame_id;
+
+  auto kvpair = FrameIdMap::value_type(key, extension_api_frame_id);
+  base::AutoLock lock(frame_id_map_lock_);
+  return frame_id_map_.insert(kvpair).first->second;
+}
+
+void ExtensionApiFrameIdMap::GotFrameIdOnIO(

[nasko, 2015/12/22 22:12:09]

nit: GetFrameIdOnIO and GotFrameIdOnIO differ by only one letter that is close
enough that makes reading the code a bit hard. Why not name it something else,
for example "ReceivedFrameIdOnIO"?

+    const RenderFrameIdKey& key,
+    const ExtensionApiFrameId& extension_api_frame_id) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  // BrowserThread::PostTaskAndReplyWithResult runs the tasks in order, so the

[nasko, 2015/12/22 22:12:10]

nit: Empty line before the comment.

+  // order of calling GotFrameIdOnIO() should match the order of the callbacks.
+  // This implies that whenever GotFrameIdOnIO() is called, the key should be
+  // identical to the key at the front of the queue.
+  DCHECK(!callbacks_.empty());
+  DCHECK(callbacks_.front().key == key);
+
+  // Note: Extra items can be appended to |callbacks_| during this loop if the
+  // callback calls GetFrameIdOnIO().
+  while (!callbacks_.empty() &&
+         (callbacks_.front().has_value || callbacks_.front().key == key)) {
+    FrameIdCallbackInfo callback_info = callbacks_.front();
+    callbacks_.pop_front();
+    if (callback_info.has_value)
+      callback_info.callback.Run(callback_info.value);
+    else  // callback_info.key == key
+      callback_info.callback.Run(extension_api_frame_id);
+  }
+
+  // Not all callbacks of |key| are immediately called, because it is possible
+  // that GetFrameIdOnIO() is called with a different argument. E.g. in the
+  // following scenario:
+  //   GetFrameIdOnIO(key1, cb1);  // cb1 run by GotFrameIdOnIO for key1.
+  //   GetFrameIdOnIO(key2, cb2);  // waiting for GotFrameIdOnIO for key2.
+  //   GetFrameIdOnIO(key1, cb3);  // waiting for GetFrameIdOnIO for key2.
+  // In the above example, |cb3| will be called as soon as |cb2| is removed from
+  // the callbacks queue.

[nasko, 2015/12/22 22:12:10]

Why do we need all of this complexity? Why can't we just run the callback if the
frame has already been added to the map?

+  for (auto& callback_info : callbacks_) {
+    if (callback_info.key == key) {
+      callback_info.has_value = true;
+      callback_info.value = extension_api_frame_id;
+    }
+  }
+}
+
+void ExtensionApiFrameIdMap::GetFrameIdOnIO(int render_process_id,
+                                            int frame_routing_id,
+                                            const FrameIdCallback& callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  const RenderFrameIdKey key(render_process_id, frame_routing_id);
+  FrameIdCallbackInfo callback_info(key, callback);
+
+  {
+    base::AutoLock lock(frame_id_map_lock_);
+    FrameIdMap::const_iterator frame_id_iter = frame_id_map_.find(key);
+    if (frame_id_iter != frame_id_map_.end()) {
+      callback_info.value = frame_id_iter->second;
+      callback_info.has_value = true;
+    }
+  }
+
+  if (callback_info.has_value) {
+    // Value already cached, thread hopping is not needed. Run the callback if
+    // there are no pending callbacks, otherwise queue it.
+    if (callbacks_.empty())
+      callback_info.callback.Run(callback_info.value);
+    else
+      callbacks_.push_back(callback_info);
+    return;
+  }
+
+  // Check whether the frame ID lookup was requested before. If yes, then there
+  // is no need for posting a task to the UI thread.
+  for (const auto& other : callbacks_) {
+    if (other.key == key) {
+      callbacks_.push_back(callback_info);
+      return;
+    }
+  }
+
+  // The key was seen for the first time, hop to the UI thread to look up the
+  // extension frame ID.
+  callbacks_.push_back(callback_info);
+  content::BrowserThread::PostTaskAndReplyWithResult(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&ExtensionApiFrameIdMap::LookupFrameIdOnUI,
+                 base::Unretained(this), key),
+      base::Bind(&ExtensionApiFrameIdMap::GotFrameIdOnIO,
+                 base::Unretained(this), key));
+}
+
+const ExtensionApiFrameId ExtensionApiFrameIdMap::GetFrameId(
+    int render_process_id,
+    int frame_routing_id) {
+  return LookupFrameIdOnUI(
+      RenderFrameIdKey(render_process_id, frame_routing_id));
+}
+
+const ExtensionApiFrameId ExtensionApiFrameIdMap::GetFrameId(
+    content::RenderFrameHost* rfh) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (!rfh)
+    return ExtensionApiFrameId();
+
+  return LookupFrameIdOnUI(
+      RenderFrameIdKey(rfh->GetProcess()->GetID(), rfh->GetRoutingID()));
+}
+
+void ExtensionApiFrameIdMap::RemoveFrameId(int render_process_id,
+                                           int frame_routing_id) {
+  // Defer removal of the key, so that RemoveFrameId() can be called when the
+  // RenderFrameHost is destroyed, without causing frame mappings to fail for
+  // other in-flight events / notifications.
+  pending_deletions_.push_back(
+      FrameIdRemovalTask(RenderFrameIdKey(render_process_id, frame_routing_id),
+                         base::TimeTicks::Now()));
+
+  // Minimum number of seconds to wait before removing the frame ID.

[nasko, 2015/12/22 22:12:10]

I found this comment to be confusing. I was expecting to see a wait and there
isn't one. I'd rephrase it to say that it is the minimum amount of time that has
elapsed since the entry was added to the list.

+  const int kSecondsUntilRemoval = 1;

[nasko, 2015/12/22 22:12:09]

This should be declared at the top of the file. Also, was there any rationale
behind picking the 1 second value? If yes, please add a comment to describe it,
otherwise it is pretty arbitrary and might as well be 5 seconds.

+  base::TimeTicks expired_creation_time =
+      base::TimeTicks::Now() -
+      base::TimeDelta::FromSeconds(kSecondsUntilRemoval);
+
+  // Remove previously queued removal tasks. RenderFrameId() is usually called
+  // whenever a frame is removed, so this clean up eventually happens. In the
+  // worst case scenario, lots of frames are created and immediately removed
+  // before |kSecondsUntilRemoval| seconds have passed. This is extremely

[nasko, 2015/12/22 22:12:09]

nit: I'd remove the "extremely" qualifier, as you'd be surprised what is likely
to happen on the web platform :).

+  // unlikely to happen. Even if it were to occur, the impact is just a slight
+  // waste of memory due to the retention of useless map entries.
+  base::AutoLock lock(frame_id_map_lock_);
+  while (!pending_deletions_.empty() &&
+         pending_deletions_.front().creation_time < expired_creation_time) {
+    frame_id_map_.erase(pending_deletions_.front().key);
+    pending_deletions_.pop_front();
+  }
+}
+
+void ExtensionApiFrameIdMap::RemoveFrameId(content::RenderFrameHost* rfh) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (!rfh)
+    return;
+
+  RemoveFrameId(rfh->GetProcess()->GetID(), rfh->GetRoutingID());
+}
+
+content::RenderFrameHost* ExtensionApiFrameIdMap::GetRenderFrameHostById(
+    content::WebContents* web_contents,
+    int frame_id) {
+  // Although it is technically possible to map |frame_id| to a RenderFrameHost
+  // without WebContents, we choose to not do that because in the extension API
+  // frameIds are only guaranteed to be meaningful in combination with a tabId.
+  if (!web_contents)
+    return nullptr;
+
+  if (frame_id == -1)
+    return nullptr;
+
+  if (frame_id == 0)
+    return web_contents->GetMainFrame();
+
+  DCHECK_GE(frame_id, 1);
+  return web_contents->FindFrameByFrameTreeNodeId(frame_id);
+}
+
+}  // namespace extensions