ExtensionAPIFrameIDMap::CacheFrameData is not called for all frames. |
|||
Issue descriptionThis happens for frames which are live during ExtensionWebContentsObserver construction.
,
Feb 27 2018
Fixing this might also slightly improve Extensions.ExtensionFrameMapCacheHit, since the frame data for these would be cached early on.
,
Mar 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dc11e912fccca09e4c8a18e979bffac8993e7b41 commit dc11e912fccca09e4c8a18e979bffac8993e7b41 Author: Karan Bhatia <karandeepb@chromium.org> Date: Fri Mar 02 02:38:31 2018 Extensions: Ensure ExtensionAPIFrameIdMap::OnRenderFrameCreated is called. Currently ExtensionAPIFrameIDMap::OnRenderFrameCreated is not called for frames which are already alive during ExtensionWebContentsObserver construction. Ensure that we call OnRenderFrameCreated for them as well. Fixing this might also slightly improve Extensions.ExtensionFrameMapCacheHit, since the frame data for these would be cached early on. BUG= 817075 Change-Id: I9f378f515741c6954bb6a5fc55adae1ff6b6f000 Reviewed-on: https://chromium-review.googlesource.com/939717 Commit-Queue: Karan Bhatia <karandeepb@chromium.org> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org> Cr-Commit-Position: refs/heads/master@{#540423} [modify] https://crrev.com/dc11e912fccca09e4c8a18e979bffac8993e7b41/extensions/browser/extension_api_frame_id_map.cc [modify] https://crrev.com/dc11e912fccca09e4c8a18e979bffac8993e7b41/extensions/browser/extension_web_contents_observer.cc
,
Mar 2 2018
Posted on the wrong bug earlier. So another issue is that the ExtensionWebContentsObserver does not get the RenderFrameCreated notification for the Devtools main frame. This happens even though we are attaching it to main_web_contents_ of DevToolsWindow in ExtensionWebContentsObserver. Why it happens turns out to be quite obscure: 1) In DevToolsWindow::Create, we create the |main_web_contents| and load a url in it. During this navigation, DevtoolsUIBindings is created for the web contents. 2) Now during DevtoolsUIBindings construction, ChromeExtensionWebContentsObserver is created. This uses WebContents::ForEachFrame to intialize the currently live frames and calls ExtensionAPIFrameIDMap::OnRenderFrameCreated. WebContents enumerates all its frames using FrameTreeNode::current_frame_host(). 3) After some time, its speculative RFH becomes its active RFH. But EWCO doesn't listen to RenderFrameHostChanged and the RenderFrameCreated notification for this RFH had already been sent (before EWCO construction) So it seems for a WCO interested in knowing about all RenderFrameHosts related to a WebContents, all three are necessary: 1) Track all live hosts during WCO construction. 2) Track RenderFrameCreated/Destroyed. 3) Track the |new_host| in RenderFrameHostChanged. This seems quite subtle. Also, cc'ing clamy@ who can take a look and decide if something like this needs to be clarified better as part of the WebContentsObserver interface and if this is WAI.
,
Mar 5 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/399eed41d62b91b9d28b3447d870cca523b2b24d commit 399eed41d62b91b9d28b3447d870cca523b2b24d Author: Karan Bhatia <karandeepb@chromium.org> Date: Mon Mar 05 23:30:44 2018 Extensions: Ensure Devtools frames are correctly tracked by ExtensionWebContentsObserver. Currently ExtensionWebContentsObserver(EWCO) does not get RenderFrameCreated notification for the Devtools main frame. This happens because the frame is created before the EWCO construction. Also, at the time of ExtensionWebContentsObserver construction, this frame happens to be the speculative RenderFrameHost of the root FrameTreeNode and hence is not enumerated via WebContents::ForEachFrame. To fix this, also track the new hosts seen through the WebContentsObserver::RenderFrameHostChanged method. Add a browser test for the same. Also, rename ExtensionAPIFrameIdMap::OnRenderFrameCreated to InitializeRenderFrameHost to clarify the new usage. BUG= 817075 Change-Id: I81213f0441162a483fda9191991763a3fee65905 Reviewed-on: https://chromium-review.googlesource.com/942684 Commit-Queue: Karan Bhatia <karandeepb@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Cr-Commit-Position: refs/heads/master@{#540978} [modify] https://crrev.com/399eed41d62b91b9d28b3447d870cca523b2b24d/chrome/browser/extensions/web_contents_browsertest.cc [modify] https://crrev.com/399eed41d62b91b9d28b3447d870cca523b2b24d/extensions/browser/extension_api_frame_id_map.cc [modify] https://crrev.com/399eed41d62b91b9d28b3447d870cca523b2b24d/extensions/browser/extension_api_frame_id_map.h [modify] https://crrev.com/399eed41d62b91b9d28b3447d870cca523b2b24d/extensions/browser/extension_web_contents_observer.cc [modify] https://crrev.com/399eed41d62b91b9d28b3447d870cca523b2b24d/extensions/browser/extension_web_contents_observer.h
,
Mar 5 2018
|
|||
►
Sign in to add a comment |
|||
Comment 1 by karandeepb@chromium.org
, Feb 27 2018