Mojofy content/common/page_messages.h |
||||||
Issue description- Convert all IPCs to mojo - Delete content/common/page_messages.h - Onion Soup files into //blink
,
Dec 6 2017
Sorry for the delay replying, I was OOO the last few days. Option 1 seems like the best option here. Option 2 has the issue of deprecating the RenderView IPCs you already mentioned, and option 3 conflicts with the effort to split RenderView and RenderWidget (see issue 419087).
,
Dec 6 2017
No problem, thanks for the feedback. I was leaning towards Option 1 too, so I'll go ahead with that one for now.
,
Dec 7 2017
Why are we deprecating RenderView? If I understand the above correctly, option 1 means adding interface implementations to RenderFrameImpl and RenderFrameProxy, but each will only ever receive connections when they represent the main frame, and each will proxy to the appropriate RenderView methods. I don't think we generally want to migrate RenderView state to RenderFrameImpl/RenderFrameProxy, so why would this be preferred over having RenderView implement the interface? What happens if/when RenderView gets removed?
,
Dec 8 2017
Yeah, you understood option 1 correctly (with the minor detail that RFI and RFP will probably share a common PageControlImpl that delegates to the RVI). My only context on RenderView being deprecated is the class level comment that pretty strongly discourages adding anything to it https://cs.chromium.org/chromium/src/content/renderer/render_view_impl.h?type=cs&sq=package:chromium&l=107
,
Dec 8 2017
creis or nasko, any thoughts on below? My understanding of that notice was to try to get people adding new code to RenderFrameImpl and RenderWidget where possible, since most new features belong in one of those. But I think a small subset of the things in RenderViewImpl don't belong in frames or widgets -- specifically, the page control things. To me, it makes sense to add this interface implementation to RenderViewImpl. If we later end up removing or renaming RenderViewImpl then the page control code would still belong in an object with that scope.
,
Dec 13 2017
Friendly ping creis, nasko.
,
Dec 15 2017
I think Ken's suggestion makes sense. We'll still need a page-level object interacting with the various renderers, and RenderView{,Host} seems like the best candidate to become this object. One of the ideas we had in the past was to rename RenderView to RenderPage to make this more explicitly, but no one finished design work on this area, since there are still several more important issues.
The deprecation of RenderView{,Host} is more about not using the same IPC channel for things that are widget-related or frame-related. While the recommendation so far has been to use WebContentsImpl for page-related things, I think having RenderView fulfilling at least some of this role would make sense.
This is, however, somewhat different than how things are done today, so you may run into some issues you'll need to address while designing this solution.
,
Jan 9 2018
Unfortunately I don't think I'll have time to finish the design of this before starting on a new team, so I'll leave it for someone else to pick up, and I'll put a TODO in the code to link back here.
,
Jan 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/07c71f41d989e9da75e0f211dec182b545496e56 commit 07c71f41d989e9da75e0f211dec182b545496e56 Author: Chris Watkins <watk@chromium.org> Date: Tue Jan 09 05:44:46 2018 Add a TODO with bug trackback for mojoifying page_messages.h Bug: 775827 Change-Id: I4b446c0ea798a2a95a0351a0163aa3f0b3f09ff5 TBR: nasko@chromium.org Reviewed-on: https://chromium-review.googlesource.com/855717 Commit-Queue: Chris Watkins <watk@chromium.org> Reviewed-by: Chris Watkins <watk@chromium.org> Cr-Commit-Position: refs/heads/master@{#527911} [modify] https://crrev.com/07c71f41d989e9da75e0f211dec182b545496e56/content/common/page_messages.h
,
Feb 26 2018
Sorry for the delayed reply-- I think Ken is right that we may want to evolve RenderView into RenderPage, rather than entirely removing RenderView. There are certain page-level messages and concepts that need to be preserved, and don't belong on frames or widgets. Let's update the deprecation message accordingly. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by w...@chromium.org
, Nov 30 2017