Remove unused functionality from ExtensionApiFrameIdMap

We will now get the frame data from the RenderFrameHost when needed. We
still need the map singleton to store the deleted frame info for beacon
requests.

Bug: 980774
Change-Id: I3bee8834b00897119f7493981a5080df26794bf1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1721933Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682133}

chrome/browser/extensions/BUILD.gn

@@ -445,8 +445,6 @@ jumbo_static_library("extensions") {
     "chrome_content_browser_client_extensions_part.h",
     "chrome_content_verifier_delegate.cc",
     "chrome_content_verifier_delegate.h",
-    "chrome_extension_api_frame_id_map_helper.cc",
-    "chrome_extension_api_frame_id_map_helper.h",
     "chrome_extension_browser_constants.cc",
     "chrome_extension_browser_constants.h",
     "chrome_extension_chooser_dialog.h",

chrome/browser/extensions/api/declarative_net_request/ruleset_manager_unittest.cc

@@ -460,7 +460,7 @@ TEST_P(RulesetManagerTest, PageAllowingAPI) {
     int frame_id;
     int parent_frame_id;
     std::string last_committed_main_frame_url;
-    base::Optional<std::string> pending_main_frame_url;
+    base::Optional<GURL> pending_main_frame_url;
   };
   struct TestCase {
     std::string url;
@@ -511,7 +511,7 @@ TEST_P(RulesetManagerTest, PageAllowingAPI) {
        "http://example.com", kDummyFrameRoutingId,
        FrameDataParams({ExtensionApiFrameIdMap::kTopFrameId,
                         ExtensionApiFrameIdMap::kInvalidFrameId,
-                        kAllowedPageURL, "http://example.com/xyz"}),
+                        kAllowedPageURL, GURL("http://example.com/xyz")}),
        true},
 
       // Here we'll determine |kAllowedPageURL| to be the main
@@ -520,7 +520,7 @@ TEST_P(RulesetManagerTest, PageAllowingAPI) {
        "http://google.com", kDummyFrameRoutingId,
        FrameDataParams({ExtensionApiFrameIdMap::kTopFrameId,
                         ExtensionApiFrameIdMap::kInvalidFrameId,
-                        kAllowedPageURL, "http://yahoo.com/xyz"}),
+                        kAllowedPageURL, GURL("http://yahoo.com/xyz")}),
        false},
 
       // In these cases both |pending_main_frame_url| and
@@ -530,19 +530,20 @@ TEST_P(RulesetManagerTest, PageAllowingAPI) {
        "http://google.com", kDummyFrameRoutingId,
        FrameDataParams({ExtensionApiFrameIdMap::kTopFrameId,
                         ExtensionApiFrameIdMap::kInvalidFrameId,
-                        "http://google.com/abc", kAllowedPageURL}),
+                        "http://google.com/abc", GURL(kAllowedPageURL)}),
        false},
       {"http://example.com/script.js", content::ResourceType::kScript,
        base::nullopt, kDummyFrameRoutingId,
        FrameDataParams({ExtensionApiFrameIdMap::kTopFrameId,
                         ExtensionApiFrameIdMap::kInvalidFrameId,
-                        kAllowedPageURL, "http://google.com/abc"}),
+                        kAllowedPageURL, GURL("http://google.com/abc")}),
        false},
       {"http://example.com/script.js", content::ResourceType::kScript,
        base::nullopt, kDummyFrameRoutingId,
        FrameDataParams({ExtensionApiFrameIdMap::kTopFrameId,
                         ExtensionApiFrameIdMap::kInvalidFrameId,
-                        "http://yahoo.com/abc", "http://yahoo.com/allow123"}),
+                        "http://yahoo.com/abc",
+                        GURL("http://yahoo.com/allow123")}),
        true},
   };
 
@@ -564,10 +565,8 @@ TEST_P(RulesetManagerTest, PageAllowingAPI) {
       const FrameDataParams& frame_params = *test_case.frame_data_params;
       params.frame_data = ExtensionApiFrameIdMap::FrameData(
           frame_params.frame_id, frame_params.parent_frame_id, kDummyTabId,
-          kDummyWindowId, GURL(frame_params.last_committed_main_frame_url));
-      if (frame_params.pending_main_frame_url)
-        params.frame_data->pending_main_frame_url =
-            GURL(*frame_params.pending_main_frame_url);
+          kDummyWindowId, GURL(frame_params.last_committed_main_frame_url),
+          frame_params.pending_main_frame_url);
     }
 
     Action expected_action = test_case.expect_blocked_with_allowed_pages

chrome/browser/extensions/chrome_extension_api_frame_id_map_helper.cc

-// Copyright 2016 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 "chrome/browser/extensions/chrome_extension_api_frame_id_map_helper.h"
-
-#include "base/bind.h"
-#include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/sessions/session_tab_helper.h"
-#include "content/public/browser/notification_service.h"
-#include "content/public/browser/web_contents.h"
-#include "extensions/common/constants.h"
-
-namespace extensions {
-
-ChromeExtensionApiFrameIdMapHelper::ChromeExtensionApiFrameIdMapHelper(
-    ExtensionApiFrameIdMap* owner)
-    : owner_(owner) {
-  registrar_.Add(this, chrome::NOTIFICATION_TAB_PARENTED,
-                 content::NotificationService::AllBrowserContextsAndSources());
-}
-
-ChromeExtensionApiFrameIdMapHelper::~ChromeExtensionApiFrameIdMapHelper() =
-    default;
-
-// static
-bool ChromeExtensionApiFrameIdMapHelper::PopulateTabData(
-    content::WebContents* web_contents,
-    int* tab_id,
-    int* window_id) {
-  if (!web_contents)
-    return false;
-
-  SessionTabHelper* session_tab_helper =
-      SessionTabHelper::FromWebContents(web_contents);
-  if (!session_tab_helper)
-    return false;
-
-  *tab_id = session_tab_helper->session_id().id();
-  *window_id = session_tab_helper->window_id().id();
-  return true;
-}
-
-void ChromeExtensionApiFrameIdMapHelper::PopulateTabData(
-    content::RenderFrameHost* rfh,
-    int* tab_id,
-    int* window_id) {
-  PopulateTabData(content::WebContents::FromRenderFrameHost(rfh), tab_id,
-                  window_id);
-}
-
-void ChromeExtensionApiFrameIdMapHelper::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  DCHECK_EQ(chrome::NOTIFICATION_TAB_PARENTED, type);
-  content::WebContents* web_contents =
-      content::Source<content::WebContents>(source).ptr();
-
-  int tab_id = extension_misc::kUnknownTabId;
-  int window_id = extension_misc::kUnknownWindowId;
-  if (!PopulateTabData(web_contents, &tab_id, &window_id))
-    return;
-
-  web_contents->ForEachFrame(
-      base::BindRepeating(&ExtensionApiFrameIdMap::UpdateTabAndWindowId,
-                          base::Unretained(owner_), tab_id, window_id));
-}
-
-}  // namespace extensions

chrome/browser/extensions/chrome_extension_api_frame_id_map_helper.h

-// Copyright 2016 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.
-
-#ifndef CHROME_BROWSER_EXTENSIONS_CHROME_EXTENSION_API_FRAME_ID_MAP_HELPER_H_
-#define CHROME_BROWSER_EXTENSIONS_CHROME_EXTENSION_API_FRAME_ID_MAP_HELPER_H_
-
-#include "base/macros.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
-#include "extensions/browser/extension_api_frame_id_map.h"
-
-namespace content {
-class WebContents;
-}  // namespace content
-
-namespace extensions {
-
-class ChromeExtensionApiFrameIdMapHelper
-    : public ExtensionApiFrameIdMapHelper,
-      public content::NotificationObserver {
- public:
-  explicit ChromeExtensionApiFrameIdMapHelper(ExtensionApiFrameIdMap* owner);
-  ~ChromeExtensionApiFrameIdMapHelper() override;
-
-  // Populates the |tab_id| and |window_id| for the given |web_contents|.
-  // Returns true on success.
-  static bool PopulateTabData(content::WebContents* web_contents,
-                              int* tab_id,
-                              int* window_id);
-
- private:
-  // ChromeExtensionApiFrameIdMapHelper:
-  void PopulateTabData(content::RenderFrameHost* rfh,
-                       int* tab_id_out,
-                       int* window_id_out) override;
-
-  // content::NotificationObserver:
-  void Observe(int type,
-               const content::NotificationSource& source,
-               const content::NotificationDetails& details) override;
-
-  ExtensionApiFrameIdMap* owner_;
-
-  content::NotificationRegistrar registrar_;
-
-  DISALLOW_COPY_AND_ASSIGN(ChromeExtensionApiFrameIdMapHelper);
-};
-
-}  // namespace extensions
-
-#endif  // CHROME_BROWSER_EXTENSIONS_CHROME_EXTENSION_API_FRAME_ID_MAP_HELPER_H_

chrome/browser/extensions/chrome_extensions_browser_client.cc

@@ -19,7 +19,6 @@
 #include "chrome/browser/extensions/api/content_settings/content_settings_service.h"
 #include "chrome/browser/extensions/api/runtime/chrome_runtime_api_delegate.h"
 #include "chrome/browser/extensions/chrome_component_extension_resource_manager.h"
-#include "chrome/browser/extensions/chrome_extension_api_frame_id_map_helper.h"
 #include "chrome/browser/extensions/chrome_extension_host_delegate.h"
 #include "chrome/browser/extensions/chrome_extension_web_contents_observer.h"
 #include "chrome/browser/extensions/chrome_extensions_browser_api_provider.h"
@@ -451,12 +450,6 @@ ChromeExtensionsBrowserClient::CreateUpdateClient(
       ChromeUpdateClientConfig::Create(context));
 }
 
-std::unique_ptr<ExtensionApiFrameIdMapHelper>
-ChromeExtensionsBrowserClient::CreateExtensionApiFrameIdMapHelper(
-    ExtensionApiFrameIdMap* map) {
-  return std::make_unique<ChromeExtensionApiFrameIdMapHelper>(map);
-}
-
 std::unique_ptr<content::BluetoothChooser>
 ChromeExtensionsBrowserClient::CreateBluetoothChooser(
     content::RenderFrameHost* frame,

chrome/browser/extensions/chrome_extensions_browser_client.h

@@ -126,9 +126,6 @@ class ChromeExtensionsBrowserClient : public ExtensionsBrowserClient {
                                      ViewType view_type) override;
   scoped_refptr<update_client::UpdateClient> CreateUpdateClient(
       content::BrowserContext* context) override;
-  std::unique_ptr<ExtensionApiFrameIdMapHelper>
-  CreateExtensionApiFrameIdMapHelper(
-      ExtensionApiFrameIdMap* map) override;
   std::unique_ptr<content::BluetoothChooser> CreateBluetoothChooser(
       content::RenderFrameHost* frame,
       const content::BluetoothChooser::EventHandler& event_handler) override;

chrome/browser/extensions/web_contents_browsertest.cc

@@ -97,161 +97,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, WebContents) {
   EXPECT_TRUE(result);
 }
 
-// Test that the frame data for the Devtools main frame is cached. Regression
-// test for crbug.com/817075.
-IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, DevToolsMainFrameIsCached) {
-  auto test_devtools_main_frame_cached = [](Browser* browser, bool is_docked) {
-    SCOPED_TRACE(base::StringPrintf("Testing a %s devtools window",
-                                    is_docked ? "docked" : "undocked"));
-    const auto* api_frame_id_map = ExtensionApiFrameIdMap::Get();
-    size_t prior_count = api_frame_id_map->GetFrameDataCountForTesting();
-
-    // Open a devtools window.
-    DevToolsWindow* devtools_window =
-        DevToolsWindowTesting::OpenDevToolsWindowSync(browser, is_docked);
-
-    // Ensure that frame data for its main frame is cached.
-    content::WebContents* devtools_web_contents =
-        DevToolsWindow::GetInTabWebContents(
-            devtools_window->GetInspectedWebContents(), nullptr);
-    ASSERT_TRUE(devtools_web_contents);
-    EXPECT_TRUE(api_frame_id_map->HasCachedFrameDataForTesting(
-        devtools_web_contents->GetMainFrame()));
-    EXPECT_GT(api_frame_id_map->GetFrameDataCountForTesting(), prior_count);
-
-    DevToolsWindowTesting::CloseDevToolsWindowSync(devtools_window);
-
-    // Ensure that the frame data for the devtools main frame, which might have
-    // been destroyed by now, is deleted.
-    EXPECT_EQ(prior_count, api_frame_id_map->GetFrameDataCountForTesting());
-  };
-
-  test_devtools_main_frame_cached(browser(), true /*is_docked*/);
-  test_devtools_main_frame_cached(browser(), false /*is_docked*/);
-}
-
-// Test that we correctly cache frame data for all frames on creation.
-// Regression test for crbug.com/810614.
-IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, FrameDataCached) {
-  // Load an extension with a web accessible resource.
-  const Extension* extension =
-      LoadExtension(test_data_dir_.AppendASCII("web_accessible_resources"));
-  ASSERT_TRUE(extension);
-
-  ASSERT_TRUE(embedded_test_server()->Start());
-
-  // Some utility functions for the test.
-
-  // Returns whether the frame data for |rfh| is cached.
-  auto has_cached_frame_data = [](content::RenderFrameHost* rfh) {
-    return ExtensionApiFrameIdMap::Get()->HasCachedFrameDataForTesting(rfh);
-  };
-
-  // Returns the cached frame data for |rfh|.
-  auto get_frame_data = [](content::RenderFrameHost* rfh) {
-    return ExtensionApiFrameIdMap::Get()->GetFrameData(
-        rfh->GetProcess()->GetID(), rfh->GetRoutingID());
-  };
-
-  // Adds an iframe with the given |name| and |src| to the given |web_contents|
-  // and waits till it loads. Returns true if successful.
-  auto add_iframe = [](content::WebContents* web_contents,
-                       const std::string& name, const GURL& src) {
-    content::TestNavigationObserver observer(web_contents,
-                                             1 /*number_of_navigations*/);
-    const char* code = R"(
-      var iframe = document.createElement('iframe');
-      iframe.name = '%s';
-      iframe.src = '%s';
-      document.body.appendChild(iframe);
-    )";
-    content::ExecuteScriptAsync(
-        web_contents,
-        base::StringPrintf(code, name.c_str(), src.spec().c_str()));
-
-    observer.WaitForNavigationFinished();
-    return observer.last_navigation_succeeded() &&
-           observer.last_navigation_url() == src;
-  };
-
-  // Returns the frame with the given |name| in |web_contents|.
-  auto get_frame_by_name = [](content::WebContents* web_contents,
-                              const std::string& name) {
-    return content::FrameMatchingPredicate(
-        web_contents, base::Bind(&content::FrameMatchesName, name));
-  };
-
-  // Navigates the browser to |url|. Injects a web-frame and an extension frame
-  // into the page and ensures that extension frame data is correctly cached for
-  // each created frame.
-  auto load_page_and_test = [&](const GURL& url) {
-    using FrameData = ExtensionApiFrameIdMap::FrameData;
-
-    SCOPED_TRACE(base::StringPrintf("Testing %s", url.spec().c_str()));
-
-    ui_test_utils::NavigateToURL(browser(), url);
-    content::WebContents* web_contents = GetActiveWebContents(browser());
-    SessionTabHelper* session_tab_helper =
-        SessionTabHelper::FromWebContents(web_contents);
-    ASSERT_TRUE(session_tab_helper);
-    int expected_tab_id = session_tab_helper->session_id().id();
-    int expected_window_id = session_tab_helper->window_id().id();
-
-    // Ensure that the frame data for the main frame is cached.
-    content::RenderFrameHost* rfh = web_contents->GetMainFrame();
-    EXPECT_TRUE(has_cached_frame_data(rfh));
-    FrameData main_frame_data = get_frame_data(rfh);
-    EXPECT_EQ(ExtensionApiFrameIdMap::kTopFrameId, main_frame_data.frame_id);
-    EXPECT_EQ(ExtensionApiFrameIdMap::kInvalidFrameId,
-              main_frame_data.parent_frame_id);
-    EXPECT_EQ(expected_tab_id, main_frame_data.tab_id);
-    EXPECT_EQ(expected_window_id, main_frame_data.window_id);
-    EXPECT_EQ(url, main_frame_data.last_committed_main_frame_url);
-    EXPECT_FALSE(main_frame_data.pending_main_frame_url);
-
-    // Add an extension iframe to the page and ensure its frame data is cached.
-    ASSERT_TRUE(
-        add_iframe(web_contents, "extension_frame",
-                   extension->GetResourceURL("web_accessible_page.html")));
-    rfh = get_frame_by_name(web_contents, "extension_frame");
-    EXPECT_TRUE(has_cached_frame_data(rfh));
-    FrameData extension_frame_data = get_frame_data(rfh);
-    EXPECT_NE(ExtensionApiFrameIdMap::kInvalidFrameId,
-              extension_frame_data.frame_id);
-    EXPECT_NE(ExtensionApiFrameIdMap::kTopFrameId,
-              extension_frame_data.frame_id);
-    EXPECT_EQ(ExtensionApiFrameIdMap::kTopFrameId,
-              extension_frame_data.parent_frame_id);
-    EXPECT_EQ(expected_tab_id, extension_frame_data.tab_id);
-    EXPECT_EQ(expected_window_id, extension_frame_data.window_id);
-    EXPECT_EQ(url, extension_frame_data.last_committed_main_frame_url);
-    EXPECT_FALSE(extension_frame_data.pending_main_frame_url);
-
-    // Add a web frame to the page and ensure its frame data is cached.
-    ASSERT_TRUE(add_iframe(web_contents, "web_frame",
-                           embedded_test_server()->GetURL("/empty.html")));
-    rfh = get_frame_by_name(web_contents, "web_frame");
-    EXPECT_TRUE(has_cached_frame_data(rfh));
-    FrameData web_frame_data = get_frame_data(rfh);
-    EXPECT_NE(ExtensionApiFrameIdMap::kInvalidFrameId, web_frame_data.frame_id);
-    EXPECT_NE(ExtensionApiFrameIdMap::kTopFrameId, web_frame_data.frame_id);
-    EXPECT_NE(extension_frame_data.frame_id, web_frame_data.frame_id);
-    EXPECT_EQ(ExtensionApiFrameIdMap::kTopFrameId,
-              web_frame_data.parent_frame_id);
-    EXPECT_EQ(expected_tab_id, web_frame_data.tab_id);
-    EXPECT_EQ(expected_window_id, web_frame_data.window_id);
-    EXPECT_EQ(url, web_frame_data.last_committed_main_frame_url);
-    EXPECT_FALSE(web_frame_data.pending_main_frame_url);
-  };
-  // End utility functions.
-
-  // Test an extension page.
-  load_page_and_test(extension->GetResourceURL("extension_page.html"));
-
-  // Test a non-extension page.
-  load_page_and_test(embedded_test_server()->GetURL("/empty.html"));
-}
-
 // Test that we correctly set up the ExtensionNavigationUIData for each
 // navigation.
 IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, ExtensionNavigationUIData) {

chrome/browser/renderer_host/chrome_navigation_ui_data.cc

@@ -12,7 +12,7 @@
 #include "ui/base/window_open_disposition.h"
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
-#include "chrome/browser/extensions/chrome_extension_api_frame_id_map_helper.h"
+#include "extensions/browser/extensions_browser_client.h"
 #include "extensions/common/constants.h"
 #endif
 
@@ -26,8 +26,11 @@ ChromeNavigationUIData::ChromeNavigationUIData(
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   int tab_id = extension_misc::kUnknownTabId;
   int window_id = extension_misc::kUnknownWindowId;
-  extensions::ChromeExtensionApiFrameIdMapHelper::PopulateTabData(
-      web_contents, &tab_id, &window_id);
+  // The browser client can be null in unittests.
+  if (extensions::ExtensionsBrowserClient::Get()) {
+    extensions::ExtensionsBrowserClient::Get()->GetTabAndWindowIdForWebContents(
+        web_contents, &tab_id, &window_id);
+  }
   extension_data_ = std::make_unique<extensions::ExtensionNavigationUIData>(
       navigation_handle, tab_id, window_id);
 #endif
@@ -60,8 +63,11 @@ ChromeNavigationUIData::CreateForMainFrameNavigation(
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   int tab_id = extension_misc::kUnknownTabId;
   int window_id = extension_misc::kUnknownWindowId;
-  extensions::ChromeExtensionApiFrameIdMapHelper::PopulateTabData(
-      web_contents, &tab_id, &window_id);
+  // The browser client can be null in unittests.
+  if (extensions::ExtensionsBrowserClient::Get()) {
+    extensions::ExtensionsBrowserClient::Get()->GetTabAndWindowIdForWebContents(
+        web_contents, &tab_id, &window_id);
+  }
 
   navigation_ui_data->extension_data_ =
       extensions::ExtensionNavigationUIData::CreateForMainFrameNavigation(

extensions/browser/api/declarative_net_request/ruleset_manager.cc

@@ -87,28 +87,24 @@ bool IsRequestPageAllowed(const WebRequestInfo& request,
   if (request.type == content::ResourceType::kMainFrame)
     return allowed_pages.MatchesURL(request.url);
 
-  // This should happen for:
-  //  - Requests not corresponding to a render frame e.g. non-navigation
-  //    browser requests or service worker requests.
-  //  - Requests made by a render frame but when we don't have cached FrameData
-  //    for the request. This should occur rarely and is tracked by the
-  //    "Extensions.ExtensionFrameMapCacheHit" histogram
-  if (!request.frame_data)
+  // This should happen for requests not corresponding to a render frame e.g.
+  // non-navigation browser requests or service worker requests.
+  if (request.frame_data.frame_id == ExtensionApiFrameIdMap::kInvalidFrameId)
     return false;
 
   const bool evaluate_pending_main_frame_url =
-      request.frame_data->pending_main_frame_url &&
-      *request.frame_data->pending_main_frame_url !=
-          request.frame_data->last_committed_main_frame_url;
+      request.frame_data.pending_main_frame_url &&
+      *request.frame_data.pending_main_frame_url !=
+          request.frame_data.last_committed_main_frame_url;
 
   if (!evaluate_pending_main_frame_url) {
     return allowed_pages.MatchesURL(
-        request.frame_data->last_committed_main_frame_url);
+        request.frame_data.last_committed_main_frame_url);
   }
 
   // |pending_main_frame_url| should only be set for main-frame subresource
   // loads.
-  DCHECK_EQ(ExtensionApiFrameIdMap::kTopFrameId, request.frame_data->frame_id);
+  DCHECK_EQ(ExtensionApiFrameIdMap::kTopFrameId, request.frame_data.frame_id);
 
   auto log_uma = [](PageAllowingInitiatorCheck value) {
     UMA_HISTOGRAM_ENUMERATION(
@@ -127,18 +123,17 @@ bool IsRequestPageAllowed(const WebRequestInfo& request,
     log_uma(PageAllowingInitiatorCheck::kInitiatorAbsent);
   } else {
     const bool initiator_matches_pending_url =
-        url::Origin::Create(*request.frame_data->pending_main_frame_url) ==
+        url::Origin::Create(*request.frame_data.pending_main_frame_url) ==
         *request.initiator;
     const bool initiator_matches_committed_url =
-        url::Origin::Create(
-            request.frame_data->last_committed_main_frame_url) ==
+        url::Origin::Create(request.frame_data.last_committed_main_frame_url) ==
         *request.initiator;
 
     if (initiator_matches_pending_url && !initiator_matches_committed_url) {
       // We predict that |pending_main_frame_url| is the actual main frame url.
       log_uma(PageAllowingInitiatorCheck::kPendingCandidateMatchesInitiator);
       return allowed_pages.MatchesURL(
-          *request.frame_data->pending_main_frame_url);
+          *request.frame_data.pending_main_frame_url);
     }
 
     if (initiator_matches_committed_url && !initiator_matches_pending_url) {
@@ -146,7 +141,7 @@ bool IsRequestPageAllowed(const WebRequestInfo& request,
       // frame url.
       log_uma(PageAllowingInitiatorCheck::kCommittedCandidateMatchesInitiator);
       return allowed_pages.MatchesURL(
-          request.frame_data->last_committed_main_frame_url);
+          request.frame_data.last_committed_main_frame_url);
     }
 
     if (initiator_matches_pending_url && initiator_matches_committed_url) {
@@ -163,8 +158,8 @@ bool IsRequestPageAllowed(const WebRequestInfo& request,
   // subresource requests might be incorrectly allowed by the page
   // allowing API.
   return allowed_pages.MatchesURL(
-             request.frame_data->last_committed_main_frame_url) ||
-         allowed_pages.MatchesURL(*request.frame_data->pending_main_frame_url);
+             request.frame_data.last_committed_main_frame_url) ||
+         allowed_pages.MatchesURL(*request.frame_data.pending_main_frame_url);
 }
 
 bool ShouldCollapseResourceType(flat_rule::ElementType type) {
@@ -494,8 +489,7 @@ RulesetManager::Action RulesetManager::EvaluateRequestInternal(
       "Extensions.DeclarativeNetRequest.EvaluateRequestTime.AllExtensions2");
 
   const RequestParams params(request);
-  const int tab_id = request.frame_data ? request.frame_data->tab_id
-                                        : extension_misc::kUnknownTabId;
+  const int tab_id = request.frame_data.tab_id;
 
   // |crosses_incognito| is used to ensure that a split mode extension process
   // can't intercept requests from a cross browser context. Since declarative

extensions/browser/api/web_request/web_request_api.cc

@@ -1460,12 +1460,7 @@ bool ExtensionWebRequestEventRouter::DispatchEvent(
     }
   }
 
-  // TODO(http://crbug.com/980774): Investigate if this is necessary.
-  if (!request->frame_data) {
-    request->frame_data = ExtensionApiFrameIdMap::Get()->GetFrameData(
-        request->render_process_id, request->frame_id);
-  }
-  event_details->SetFrameData(request->frame_data.value());
+  event_details->SetFrameData(request->frame_data);
   DispatchEventToListeners(browser_context, std::move(listeners_to_dispatch),
                            std::move(event_details));
 
@@ -1882,10 +1877,10 @@ void ExtensionWebRequestEventRouter::GetMatchingListenersImpl(
 
     // Check if the tab id and window id match, if they were set in the
     // listener params.
-    if ((listener->filter.tab_id != -1 && request->frame_data &&
-         request->frame_data->tab_id != listener->filter.tab_id) ||
-        (listener->filter.window_id != -1 && request->frame_data &&
-         request->frame_data->window_id != listener->filter.window_id)) {
+    if ((listener->filter.tab_id != -1 &&
+         request->frame_data.tab_id != listener->filter.tab_id) ||
+        (listener->filter.window_id != -1 &&
+         request->frame_data.window_id != listener->filter.window_id)) {
       continue;
     }
 
@@ -1898,9 +1893,7 @@ void ExtensionWebRequestEventRouter::GetMatchingListenersImpl(
       PermissionsData::PageAccess access =
           WebRequestPermissions::CanExtensionAccessURL(
               PermissionHelper::Get(browser_context), listener->id.extension_id,
-              request->url,
-              request->frame_data ? request->frame_data->tab_id : -1,
-              crosses_incognito,
+              request->url, request->frame_data.tab_id, crosses_incognito,
               WebRequestPermissions::
                   REQUIRE_HOST_PERMISSION_FOR_URL_AND_INITIATOR,
               request->initiator, request->type);

extensions/browser/api/web_request/web_request_info.h

@@ -71,7 +71,7 @@ struct WebRequestInfoInitParams {
   int web_view_instance_id = -1;
   int web_view_rules_registry_id = -1;
   int web_view_embedder_process_id = -1;
-  base::Optional<ExtensionApiFrameIdMap::FrameData> frame_data;
+  ExtensionApiFrameIdMap::FrameData frame_data;
 
  private:
   void InitializeWebViewAndFrameData(
@@ -120,10 +120,8 @@ struct WebRequestInfo {
   const base::Optional<url::Origin> initiator;
 
   // Extension API frame data corresponding to details of the frame which
-  // initiate this request. May be null for renderer-initiated requests where
-  // some frame details are not known at WebRequestInfo construction time.
-  // Mutable since this is lazily computed.
-  mutable base::Optional<ExtensionApiFrameIdMap::FrameData> frame_data;
+  // initiate this request.
+  ExtensionApiFrameIdMap::FrameData frame_data;
 
   // The type of the request (e.g. main frame, subresource, XHR, etc). May have
   // no value if the request did not originate from a ResourceDispatcher.

extensions/browser/extension_api_frame_id_map.h

@@ -5,16 +5,13 @@
 #ifndef EXTENSIONS_BROWSER_EXTENSION_API_FRAME_ID_MAP_H_
 #define EXTENSIONS_BROWSER_EXTENSION_API_FRAME_ID_MAP_H_
 
-#include <list>
 #include <map>
 #include <memory>
 #include <set>
 
-#include "base/callback.h"
 #include "base/lazy_instance.h"
 #include "base/macros.h"
 #include "base/optional.h"
-#include "base/synchronization/lock.h"
 #include "url/gurl.h"
 
 namespace content {
@@ -25,16 +22,6 @@ class WebContents;
 
 namespace extensions {
 
-// TODO(http://crbug.com/980774): Investigate if this class and
-// ExtensionApiFrameIdMap are still needed.
-class ExtensionApiFrameIdMapHelper {
- public:
-  virtual void PopulateTabData(content::RenderFrameHost* rfh,
-                               int* tab_id_out,
-                               int* window_id_out) = 0;
-  virtual ~ExtensionApiFrameIdMapHelper() {}
-};
-
 // Extension frame IDs are exposed through the chrome.* APIs and have the
 // following characteristics:
 // - The top-level frame has ID 0.
@@ -53,8 +40,6 @@ class ExtensionApiFrameIdMapHelper {
 // Unless stated otherwise, the methods can only be called on the UI thread.
 //
 // The non-static methods of this class use an internal cache.
-// TODO(http://crbug.com/980774): This cache may not be necessary now that this
-// is not accessed on IO.
 class ExtensionApiFrameIdMap {
  public:
   // The data for a RenderFrame. Every RenderFrameIdKey maps to a FrameData.
@@ -64,7 +49,8 @@ class ExtensionApiFrameIdMap {
               int parent_frame_id,
               int tab_id,
               int window_id,
-              GURL last_committed_main_frame_url);
+              GURL last_committed_main_frame_url,
+              base::Optional<GURL> pending_main_frame_url);
     ~FrameData();
 
     FrameData(const FrameData&);
@@ -94,8 +80,6 @@ class ExtensionApiFrameIdMap {
     base::Optional<GURL> pending_main_frame_url;
   };
 
-  using FrameDataCallback = base::Callback<void(const FrameData&)>;
-
   // An invalid extension API frame ID.
   static const int kInvalidFrameId;
 
@@ -124,39 +108,15 @@ class ExtensionApiFrameIdMap {
       int frame_id);
 
   // Retrieves the FrameData for a given |render_process_id| and
-  // |render_frame_id|. The map may be updated with the result if the map did
-  // not contain the FrameData before the lookup. If a RenderFrameHost is not
-  // found, then the map is not modified.
+  // |render_frame_id|.
   FrameData GetFrameData(int render_process_id,
                          int render_frame_id) WARN_UNUSED_RESULT;
 
-  // Initializes the FrameData for the given |rfh|.
-  void InitializeRenderFrameData(content::RenderFrameHost* rfh);
-
-  // Called when a render frame is deleted. Removes the FrameData mapping for
-  // the given render frame.
+  // Called when a render frame is deleted. Stores the FrameData for |rfh| in
+  // the deleted frames map so it can still be accessed for beacon requests. The
+  // FrameData will be removed later in a task.
   void OnRenderFrameDeleted(content::RenderFrameHost* rfh);
 
-  // Updates the tab and window id for the given RenderFrameHost if necessary.
-  void UpdateTabAndWindowId(int tab_id,
-                            int window_id,
-                            content::RenderFrameHost* rfh);
-
-  // Called when WebContentsObserver::ReadyToCommitNavigation is dispatched for
-  // a main frame.
-  void OnMainFrameReadyToCommitNavigation(
-      content::NavigationHandle* navigation_handle);
-
-  // Called when WebContentsObserver::DidFinishNavigation is dispatched for a
-  // main frame.
-  void OnMainFrameDidFinishNavigation(
-      content::NavigationHandle* navigation_handle);
-
-  // Returns whether frame data for |rfh| is cached.
-  bool HasCachedFrameDataForTesting(content::RenderFrameHost* rfh) const;
-
-  size_t GetFrameDataCountForTesting() const;
-
  protected:
   friend struct base::LazyInstanceTraitsBase<ExtensionApiFrameIdMap>;
 
@@ -175,54 +135,22 @@ class ExtensionApiFrameIdMap {
     bool operator==(const RenderFrameIdKey& other) const;
   };
 
-  struct FrameDataCallbacks {
-    FrameDataCallbacks();
-    FrameDataCallbacks(const FrameDataCallbacks& other);
-    ~FrameDataCallbacks();
-
-    // This is a std::list so that iterators are not invalidated when the list
-    // is modified during an iteration.
-    std::list<FrameDataCallback> callbacks;
-
-    // To avoid re-entrant processing of callbacks.
-    bool is_iterating;
-  };
-
   using FrameDataMap = std::map<RenderFrameIdKey, FrameData>;
-  using FrameDataCallbacksMap = std::map<RenderFrameIdKey, FrameDataCallbacks>;
 
   ExtensionApiFrameIdMap();
-  virtual ~ExtensionApiFrameIdMap();
+  ~ExtensionApiFrameIdMap();
 
   // Determines the value to be stored in |frame_data_map_| for a given key.
   // Returns empty FrameData when the corresponding RenderFrameHost is not
   // alive. This method is only called when |key| is not in |frame_data_map_|.
   // Virtual for testing.
-  virtual FrameData KeyToValue(const RenderFrameIdKey& key) const;
-
-  // Looks up the data for the given |key| and adds it to the |frame_data_map_|.
-  // If |check_deleted_frames| is true, |deleted_frame_data_map_| will also be
-  // used.
-  FrameData LookupFrameDataOnUI(const RenderFrameIdKey& key,
-                                bool check_deleted_frames);
-
-  std::unique_ptr<ExtensionApiFrameIdMapHelper> helper_;
-
-  // This map holds a mapping of render frame key to FrameData.
-  // TODO(http://crbug.com/980774): Investigate if this is still needed.
-  FrameDataMap frame_data_map_;
+  FrameData KeyToValue(const RenderFrameIdKey& key) const;
 
   // Holds mappings of render frame key to FrameData from frames that have been
   // recently deleted. These are kept for a short time so beacon requests that
   // continue after a frame is unloaded can access the FrameData.
   FrameDataMap deleted_frame_data_map_;
 
-  // The set of pending main frame navigations for which ReadyToCommitNavigation
-  // has been fired. Only used on the UI thread. This is needed to clear state
-  // set up in OnMainFrameReadyToCommitNavigation for navigations which
-  // eventually do not commit.
-  std::set<content::NavigationHandle*> ready_to_commit_document_navigations_;
-
   DISALLOW_COPY_AND_ASSIGN(ExtensionApiFrameIdMap);
 };
 

extensions/browser/extension_navigation_ui_data.cc

@@ -71,7 +71,8 @@ ExtensionNavigationUIData::ExtensionNavigationUIData(
                   window_id,
                   // The RenderFrameHost may not have an associated WebContents
                   // in cases such as interstitial pages.
-                  web_contents ? web_contents->GetLastCommittedURL() : GURL()) {
+                  web_contents ? web_contents->GetLastCommittedURL() : GURL(),
+                  base::nullopt /* pending_main_frame_url */) {
   WebViewGuest* web_view = WebViewGuest::FromWebContents(web_contents);
   if (web_view) {
     is_web_view_ = true;

extensions/browser/extension_web_contents_observer.cc

@@ -47,10 +47,6 @@ void ExtensionWebContentsObserver::Initialize() {
     if (!rfh->IsRenderFrameLive())
       continue;
 
-    // Initialize the FrameData for this frame here since we didn't receive the
-    // RenderFrameCreated notification for it.
-    ExtensionApiFrameIdMap::Get()->InitializeRenderFrameData(rfh);
-
     InitializeRenderFrame(rfh);
   }
 }
@@ -111,11 +107,6 @@ content::WebContents* ExtensionWebContentsObserver::GetAssociatedWebContents()
 void ExtensionWebContentsObserver::RenderFrameCreated(
     content::RenderFrameHost* render_frame_host) {
   DCHECK(initialized_);
-  // Optimization: Look up the extension API frame ID to force the mapping to be
-  // cached. This minimizes the number of IO->UI->IO thread hops when the ID is
-  // looked up again on the IO thread for the webRequest API.
-  ExtensionApiFrameIdMap::Get()->InitializeRenderFrameData(render_frame_host);
-
   InitializeRenderFrame(render_frame_host);
 
   const Extension* extension = GetExtensionFromFrame(render_frame_host, false);
@@ -160,37 +151,14 @@ void ExtensionWebContentsObserver::RenderFrameDeleted(
   ExtensionApiFrameIdMap::Get()->OnRenderFrameDeleted(render_frame_host);
 }
 
-void ExtensionWebContentsObserver::RenderFrameHostChanged(
-    content::RenderFrameHost* old_host,
-    content::RenderFrameHost* new_host) {
-  // TODO(karandeepb): The |new_host| here may correspond to a RenderFrameHost
-  // we haven't seen yet, which means it might also need some other
-  // initialization. See crbug.com/817205.
-  if (new_host->IsRenderFrameLive()) {
-    ExtensionApiFrameIdMap::Get()->InitializeRenderFrameData(new_host);
-  }
-}
-
 void ExtensionWebContentsObserver::ReadyToCommitNavigation(
     content::NavigationHandle* navigation_handle) {
   URLLoaderFactoryManager::ReadyToCommitNavigation(navigation_handle);
-
-  if (navigation_handle->IsInMainFrame() &&
-      !navigation_handle->IsSameDocument()) {
-    ExtensionApiFrameIdMap::Get()->OnMainFrameReadyToCommitNavigation(
-        navigation_handle);
-  }
 }
 
 void ExtensionWebContentsObserver::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
   DCHECK(initialized_);
-  if (navigation_handle->IsInMainFrame() &&
-      !navigation_handle->IsSameDocument()) {
-    ExtensionApiFrameIdMap::Get()->OnMainFrameDidFinishNavigation(
-        navigation_handle);
-  }
-
   if (!navigation_handle->HasCommitted())
     return;
 

extensions/browser/extension_web_contents_observer.h

@@ -89,8 +89,6 @@ class ExtensionWebContentsObserver
   // content::WebContentsObserver overrides.
   void RenderFrameCreated(content::RenderFrameHost* render_frame_host) override;
   void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
-  void RenderFrameHostChanged(content::RenderFrameHost* old_host,
-                              content::RenderFrameHost* new_host) override;
   void ReadyToCommitNavigation(
       content::NavigationHandle* navigation_handle) override;
   void DidFinishNavigation(

extensions/browser/extensions_browser_client.cc

@@ -44,12 +44,6 @@ ExtensionsBrowserClient::CreateUpdateClient(content::BrowserContext* context) {
   return scoped_refptr<update_client::UpdateClient>(nullptr);
 }
 
-std::unique_ptr<ExtensionApiFrameIdMapHelper>
-ExtensionsBrowserClient::CreateExtensionApiFrameIdMapHelper(
-    ExtensionApiFrameIdMap* map) {
-  return nullptr;
-}
-
 std::unique_ptr<content::BluetoothChooser>
 ExtensionsBrowserClient::CreateBluetoothChooser(
     content::RenderFrameHost* frame,

extensions/browser/extensions_browser_client.h

@@ -55,8 +55,6 @@ class Extension;
 class ExtensionCache;
 class ExtensionError;
 class ExtensionHostDelegate;
-class ExtensionApiFrameIdMap;
-class ExtensionApiFrameIdMapHelper;
 class ExtensionSet;
 class ExtensionSystem;
 class ExtensionSystemProvider;
@@ -287,9 +285,6 @@ class ExtensionsBrowserClient {
   virtual scoped_refptr<update_client::UpdateClient> CreateUpdateClient(
       content::BrowserContext* context);
 
-  virtual std::unique_ptr<ExtensionApiFrameIdMapHelper>
-  CreateExtensionApiFrameIdMapHelper(ExtensionApiFrameIdMap* map);
-
   virtual std::unique_ptr<content::BluetoothChooser> CreateBluetoothChooser(
       content::RenderFrameHost* frame,
       const content::BluetoothChooser::EventHandler& event_handler);