Tracking all content::WebContents instances

[Patrick]

Hey all,

I am interested in tracking all the WebContents instances in the browser as part of a new component named PriorityTracker but AFAIK, it's not possible to get notified of their creation. 

High-level design doc

Implementation proposal still in progress (only talks about tabs for now):

Note that I am looking to track more than just the ones that become tabs, so that subscribing as an observer to TabStripModel instances is not sufficient. As a reference, I made a document that details all the callsites to WebContents::Create() and WebContents::CreateWithSessionStorage() here.

I do care about which of these WebContents are tabs because some components can only act on those (e.g. Tab discarding component), so I need more context than just a WebContents has been created.

So far I've come up with 2 different solutions and I am interested in hearing about the opinion of other developers.

1. Make it possible to subscribe to WebContents create events, by adding code in WebContents::Create() that notifies observers. Extend the interface to pass-in a context to the Create() function so that observers can know what

type of WebContents it is.

This is a more intrusive change into the content/ directory.

2. Support observers for all components that calls WebContents::Create() or WebContents::CreateWithSessionStorage(), like TabStripModelObserver.

Example: Create new observers types like AppWindowObserver, PrerenderObserver, etc. and subscribe to all the related components.

3. Something else that I didn't think about?

Thanks for your time!

Patrick Monette

[Lucas Gadani]

+Nick Carter +Avi Drissman 

There's a test-only interface that does what you want: https://cs.chromium.org/chromium/src/content/public/test/browser_test_utils.h?rcl=1484151649&l=468 , but it's explicitly disallowed to expose that, see https://cs.chromium.org/chromium/src/content/browser/web_contents/web_contents_impl.h?l=1506.

What kind of WebContents do you need to track that aren't exposed by the TabStripModel?

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Charlie Reis]

[+jam]

[Patrick]

On Wednesday, January 11, 2017 at 4:14:50 PM UTC-5, Lucas Gadani wrote:

+Nick Carter +Avi Drissman 

There's a test-only interface that does what you want: https://cs.chromium.org/chromium/src/content/public/test/browser_test_utils.h?rcl=1484151649&l=468 , but it's explicitly disallowed to expose that, see https://cs.chromium.org/chromium/src/content/browser/web_contents/web_contents_impl.h?l=1506.

What kind of WebContents do you need to track that aren't exposed by the TabStripModel?

The biggest ones are prerenders. Also included are extensions pages, embedded <webview> and <appview> in extensions/apps. That's why I included the reference doc for callsites to WebContents::Create(). Basically any contents that requires resources (memory, network requests) and that a component that manages those resources would want to know which request to prioritize/which contents to optimize.

[Yuri Wiitala]

FYI--The chrome.tabCapture API also creates off-screen tabs (WebContents instances), mainly to support the web platform navigator.presentation API.

2017/01/11 午後1:21 "Patrick" <pmon...@chromium.org>:

[Lucas Gadani]

On Wed, Jan 11, 2017 at 4:21 PM Patrick <pmon...@chromium.org> wrote:

On Wednesday, January 11, 2017 at 4:14:50 PM UTC-5, Lucas Gadani wrote:

What kind of WebContents do you need to track that aren't exposed by the TabStripModel?

The biggest ones are prerenders. Also included are extensions pages, embedded <webview> and <appview> in extensions/apps. That's why I included the reference doc for callsites to WebContents::Create(). Basically any contents that requires resources (memory, network requests) and that a component that manages those resources would want to know which request to prioritize/which contents to optimize.

I believe all of the features you listed are outside of the content layer, so you'll probably need to implement feature-specific management of the priorities. At that point, you might as well add a callback to notify your component about the feature instantiation. This does look like a lot of extra complexity though, but hopefully the benefits are worth.

[Avi Drissman]

John was added but people aren't replying to the message in which he was added.

This has been requested before. The danger we've found in proposals of this interface is that it's too easy for code to tie into manipulating "all WebContents objects created" and thus create cross-module dependencies where code that fiddles with the WebContentses as they're created ends up creating spooky-action-at-a-distance problems and dependencies that make it hard to reason about what's going on.

Depending on the design, managing priorities could be something we might want for content.

John, thoughts?

Avi

[Marshall Greenblatt]

This may not be great for your use case, but you could potentially discover all WebContents using a polling approach. You can use RenderWidgetHost::GetRenderWidgetHosts to iterate all existing RenderWidgetHost instances, and then use RenderViewHost::From (which may return nullptr) and WebContents::FromRenderViewHost to work back to the WebContents instances from there. You can then create a WebContentsObserver to be notified of WebContents-related events.

--

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---

You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.

[John Abd-El-Malek]

The background is we've had layering violations in the past by exposing a way to get all the WebContents.

Stepping back a bit, this doesn't look like it's gone through browser-design-docs@. Can you please send the doc there so we can discuss it before the implementation?

[Patrick]

Thanks everyone for your input, I'll go ahead and update the design doc and share it with browser-design-docs@

[Nick Carter]

The TaskManager (in chrome/) manages to track all WebContentses without needing a hook; it just uses the TabHelper pattern.

This allows different flavors of WebContentses to be treated differently -- for the TM that means they are formatted differently; for priority management, for example, you will want to treat devtools windows (which are WebContentses), or extension browser actions, or panels, or a background printing task, or an extension background page, differently from regular tabs. If all you get is a stream of WebContents* pointers from the content layer, it would be practically hard to accurately know what it's for, since content/ has partial-to-no knowledge of these features.

[Patrick]

I didn't think to look at how TaskManager was doing it. Thanks!

jam@ (or anyone else) About browser-design-docs@, is this the right email? I can't find a reference to it anywhere. How are people supposed to know that they should share their design doc there? Thanks!

On Thursday, January 12, 2017 at 1:52:53 PM UTC-5, Nick Carter wrote:

The TaskManager (in chrome/) manages to track all WebContentses without needing a hook; it just uses the TabHelper pattern.

That's an neat way to do it and one that would definitely work for me. It's more than just the TabHelper pattern though; for WebContents that aren't tabs, the component must signal the TaskManager itself via WebContentsTags::CreateFor* functions.

This allows different flavors of WebContentses to be treated differently -- for the TM that means they are formatted differently; for priority management, for example, you will want to treat devtools windows (which are WebContentses), or extension browser actions, or panels, or a background printing task, or an extension background page, differently from regular tabs. If all you get is a stream of WebContents* pointers from the content layer, it would be practically hard to accurately know what it's for, since content/ has partial-to-no knowledge of these features.

Yeah my #1 proposal requires a change to the WebContents::Create() for the caller to pass-in context.

[John Abd-El-Malek]

On Thu, Jan 12, 2017 at 1:51 PM, Patrick <pmon...@chromium.org> wrote:

I didn't think to look at how TaskManager was doing it. Thanks!

jam@ (or anyone else) About browser-design-docs@, is this the right email? I can't find a reference to it anywhere. How are people supposed to know that they should share their design doc there? Thanks!

Please see go/newchromefeature which has more information, including templates.

[Marijn Kruisselbrink]

On Thu, Jan 12, 2017 at 2:33 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Thu, Jan 12, 2017 at 1:51 PM, Patrick <pmon...@chromium.org> wrote:

I didn't think to look at how TaskManager was doing it. Thanks!

jam@ (or anyone else) About browser-design-docs@, is this the right email? I can't find a reference to it anywhere. How are people supposed to know that they should share their design doc there? Thanks!

Please see go/newchromefeature which has more information, including templates.

I don't see where that document mentions browser-design-docs@? It gives three options: an internal google list, chromium-dev@ and blink-dev@ in the "where to post your design doc" section.

[Colin Blundell]

On Thu, Jan 12, 2017 at 11:46 PM Marijn Kruisselbrink <m...@chromium.org> wrote:

On Thu, Jan 12, 2017 at 2:33 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Thu, Jan 12, 2017 at 1:51 PM, Patrick <pmon...@chromium.org> wrote:

I didn't think to look at how TaskManager was doing it. Thanks!

jam@ (or anyone else) About browser-design-docs@, is this the right email? I can't find a reference to it anywhere. How are people supposed to know that they should share their design doc there? Thanks!

Please see go/newchromefeature which has more information, including templates.

I don't see where that document mentions browser-design-docs@? It gives three options: an internal google list, chromium-dev@ and blink-dev@ in the "where to post your design doc" section.

The first of those is what John was referring to.

[Marijn Kruisselbrink]

On Fri, Jan 13, 2017 at 2:02 AM, Colin Blundell <blun...@chromium.org> wrote:

On Thu, Jan 12, 2017 at 11:46 PM Marijn Kruisselbrink <m...@chromium.org> wrote:

On Thu, Jan 12, 2017 at 2:33 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Thu, Jan 12, 2017 at 1:51 PM, Patrick <pmon...@chromium.org> wrote:

I didn't think to look at how TaskManager was doing it. Thanks!

jam@ (or anyone else) About browser-design-docs@, is this the right email? I can't find a reference to it anywhere. How are people supposed to know that they should share their design doc there? Thanks!

Please see go/newchromefeature which has more information, including templates.

I don't see where that document mentions browser-design-docs@? It gives three options: an internal google list, chromium-dev@ and blink-dev@ in the "where to post your design doc" section.

The first of those is what John was referring to.

I still don't understand why this design doc would have to be sent to the internal mailing list rather than the public chromium-dev? John made it sound like the design doc should have been shared there before being shared in public, while the go/newchromefeature document seems to say that only particularly sensitive features that can't be discussed in public, or things needing to go through launch review. So is the argument that this feature is particularly sensitive and should not be discussed in public? Or that the feature has privacy or security implications that need to be discussed in private before discussing the implementation details in public? John's comment somehow sounded like he wanted to discuss the implementation design of the feature in private, rather than the cross functional aspects and/or sensitive issues, which as far as I can tell is the opposite of what go/newchromefeature suggests. Am I misreading go/newchromefeature or am I just misunderstanding what was being suggested here?

[John Abd-El-Malek]

On Fri, Jan 13, 2017 at 9:52 AM, Marijn Kruisselbrink <m...@chromium.org> wrote:

On Fri, Jan 13, 2017 at 2:02 AM, Colin Blundell <blun...@chromium.org> wrote:

On Thu, Jan 12, 2017 at 11:46 PM Marijn Kruisselbrink <m...@chromium.org> wrote:

On Thu, Jan 12, 2017 at 2:33 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Thu, Jan 12, 2017 at 1:51 PM, Patrick <pmon...@chromium.org> wrote:

I didn't think to look at how TaskManager was doing it. Thanks!

jam@ (or anyone else) About browser-design-docs@, is this the right email? I can't find a reference to it anywhere. How are people supposed to know that they should share their design doc there? Thanks!

Please see go/newchromefeature which has more information, including templates.

I don't see where that document mentions browser-design-docs@? It gives three options: an internal google list, chromium-dev@ and blink-dev@ in the "where to post your design doc" section.

The first of those is what John was referring to.

Sorry I meant chrome-design-docs@ and not browser-design-docs.

 

I still don't understand why this design doc would have to be sent to the internal mailing list rather than the public chromium-dev? John made it sound like the design doc should have been shared there before being shared in public, while the go/newchromefeature document seems to say that only particularly sensitive features that can't be discussed in public, or things needing to go through launch review. So is the argument that this feature is particularly sensitive and should not be discussed in public? Or that the feature has privacy or security implications that need to be discussed in private before discussing the implementation details in public? John's comment somehow sounded like he wanted to discuss the implementation design of the feature in private, rather than the cross functional aspects and/or sensitive issues, which as far as I can tell is the opposite of what go/newchromefeature suggests. Am I misreading go/newchromefeature or am I just misunderstanding what was being suggested here?

https://groups.google.com/a/google.com/d/msg/chrome-team/_mKOXRPMgsY/RVXULBEiAAAJ has the full nuance for when to send internal or external. If the work meets the criteria cited there for public discussion, then yes chromium-dev would be a better place. The current process though is that if the document has any aspects that need to be discussed internally, then the doc should just be sent to the internal list and all the discussion happens there. There was a lot of back and forth to decide on this policy, but the reason it's this way was that having some discussion on a doc publicly and some privately, and two versions of the doc, would make it too easy to mix the two discussions.