New issue
Advanced search Search tips

Issue 811384 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Extensions: EWCO::InitializeRenderFrame is not called for non-extension frames and shouldn't be virtual.

Project Member Reported by karandeepb@chromium.org, Feb 12 2018

Issue description

- This causes us to not send the window ID to the renderer for these frames. 
- Also, the call to InitializeRenderFrame (a virtual function) from EWCO's constructor does not reach CEWCO::InitializeRenderFrame, since it is called from a derived class constructor.

TODO:
- Decide whether we should send the window ID to the renderer for non-extension frames.
- Ensure CEWCO::InitializeRenderFrame is also called during construction of the WebContentsObserver.
 
ok
Summary: Extensions: EWCO::InitializeRenderFrame is not called for non-extension frames and shouldn't be virtual. (was: Extensions: EWCO::InitializeRenderFrame is not called for non-extension frames.)
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/399b6c3d211c09e1e0a003e8edb547043535bf77

commit 399b6c3d211c09e1e0a003e8edb547043535bf77
Author: Karan Bhatia <karandeepb@chromium.org>
Date: Tue Mar 06 11:11:23 2018

Extensions: Ensure ExtensionWebContentsObserver:InitializeRenderFrame is not called from constructor.

ExtensionWebContentsObserver:InitializeRenderFrame is also called from its
constructor. But InitializeRenderFrame is a virtual function, hence for this
invocation from the constructor, the derived class version
(ChromeExtensionWebContentsObserver:InitializeRenderFrame) won't be called.
Correct this by having a public Initialize() method on
ExtensionWebContentsObserver and introducing a static CreateForWebContents on
derived classes to initialize the instance.

BUG= 811384 

Change-Id: I38d2daa81202520121c78dd473c5eea1ea599ad5
Reviewed-on: https://chromium-review.googlesource.com/940863
Reviewed-by: Sergey Volk <servolk@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541069}
[modify] https://crrev.com/399b6c3d211c09e1e0a003e8edb547043535bf77/chrome/browser/extensions/chrome_extension_web_contents_observer.cc
[modify] https://crrev.com/399b6c3d211c09e1e0a003e8edb547043535bf77/chrome/browser/extensions/chrome_extension_web_contents_observer.h
[modify] https://crrev.com/399b6c3d211c09e1e0a003e8edb547043535bf77/chromecast/browser/extensions/cast_extension_web_contents_observer.cc
[modify] https://crrev.com/399b6c3d211c09e1e0a003e8edb547043535bf77/chromecast/browser/extensions/cast_extension_web_contents_observer.h
[modify] https://crrev.com/399b6c3d211c09e1e0a003e8edb547043535bf77/extensions/browser/extension_web_contents_observer.cc
[modify] https://crrev.com/399b6c3d211c09e1e0a003e8edb547043535bf77/extensions/browser/extension_web_contents_observer.h
[modify] https://crrev.com/399b6c3d211c09e1e0a003e8edb547043535bf77/extensions/shell/browser/shell_extension_web_contents_observer.cc
[modify] https://crrev.com/399b6c3d211c09e1e0a003e8edb547043535bf77/extensions/shell/browser/shell_extension_web_contents_observer.h

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a3b9162cb2150a6fb6443e38c60035b76ef1bd1e

commit a3b9162cb2150a6fb6443e38c60035b76ef1bd1e
Author: Karan Bhatia <karandeepb@chromium.org>
Date: Fri Mar 09 03:30:50 2018

Extensions: Ensure InitializeRenderFrame is called for non-extension frames as well.

r501291 changed ExtensionWebContentsObserver::RenderFrameCreated such that
InitializeRenderFrame was no longer called for non-extension frames. This CL
changes it back so that InitializeRenderFrame is called for all frames; and
documents the same in the header. This would cause us to initialize the window
ID for non- extension render frames as well (which is consistent with other
code).

BUG= 811384 

Change-Id: I20a7039f8ba82fffbb1d17b635cfe0beb0279469
Reviewed-on: https://chromium-review.googlesource.com/956549
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542011}
[modify] https://crrev.com/a3b9162cb2150a6fb6443e38c60035b76ef1bd1e/extensions/browser/extension_web_contents_observer.cc
[modify] https://crrev.com/a3b9162cb2150a6fb6443e38c60035b76ef1bd1e/extensions/browser/extension_web_contents_observer.h

Status: Fixed (was: Assigned)

Sign in to add a comment