mojo, OOPIFs, multiple renderers, ChromeViewMsg_SetContentSettingRules

[Nigel Tao]

Hi again, chromium-mojo.

It's been around 6 months since I've done anything Mojo related, but
one of my old conversions from legacy IPC to Mojo has lead to
https://bugs.chromium.org/p/chromium/issues/detail?id=767539
"TabSpecificContentSettings::OnContentSettingChanged is not aware of
OOPIFs" being filed.

The issue report is:

"TabSpecificContentSettings::OnContentSettingChanged is sending
RendererContentSettingRules to the renderer process. The problem is
that the rules are only sent to the main frame's renderer and are not
sent to other renderers (i.e. to renderers hosting OOPIFs)."

My Mojofication of ChromeViewMsg_SetContentSettingRules CLs were
https://codereview.chromium.org/2582203003 and
https://codereview.chromium.org/2614033004

I can't remember much about that specific change. I didn't and still
don't know content settings rules very well, and similarly for
multiple renderer situations like OOPIFs. From my point of view, that
CL was just one of a number of mechanical "upgrade from legacy IPC"
changes that I made across the entire codebase.

Basically, I (and lukasza@, the issue reporter) need help. How do
OOPIFs and Mojo interact (or meant to interact)? I found
https://www.chromium.org/developers/design-documents/oop-iframes but
it doesn't discuss Mojo. I don't have a good sense of where to start.
I don't even know how to tell if the legacy
ChromeViewMsg_SetContentSettingRules IPC message handled OOPIFs
properly.

[Charlie Reis]

[+site-isolation-dev, dcheng]

In general, most ViewMsgs don't handle OOPIFs properly, and assume all the frames of a page are in one process.  The general practice had been to convert them to FrameMsgs, but with Mojo I imagine there's frame equivalents to use, allowing you to notify each frame in a page regardless of which process it is in.  dcheng@ or other Mojo folks can probably fill more details.

Charlie

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.
To post to this group, send email to chromi...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CAEdON6a%3DnFyrF0%2B9NeE_p_sd%2BNcRS-LyF2KA4QY_C9OFG%2BHxEQ%40mail.gmail.com.

[Ken Buchanan]

I'm not familiar with TabSpecificContentSettings either but I have some thoughts on the message passing that might be helpful.

I don't think we want to send this to all Frames, since this is something that applies to the entire tab. For legacy IPC we use WebContentsImpl::SendPageMessage() that dispatches ViewMsgs to all relevant renderer processes, but there isn't a convenient facility for that with Mojo (AFAIK).

I suspect that ChromeViewMsg_SetContentSettingRules never handled OOPIFs correctly either.

Here, we are sending the update only to the main frame's renderer process:

https://cs.chromium.org/chromium/src/chrome/browser/content_settings/tab_specific_content_settings.cc?l=749&rcl=41b4a5de14f4d78376a53193a4fce2aef71b4f09

It's possible I might be missing relevant details, but I think you need to be iterating that for each RenderProcessHost associated with frames in the given WebContents, and that likely needs a new method in WebContentsImpl to allow that to happen. You can see the logic for identifying all the targets for a page-level message is not quite trivial:

https://cs.chromium.org/chromium/src/content/browser/frame_host/render_frame_host_manager.cc?l=2638&rcl=ac1986b32351290a8a90d688f5604bca8d3a464b

[Nigel Tao]

On Sat, Sep 23, 2017 at 2:02 AM, Ken Buchanan <ke...@chromium.org> wrote:
> I suspect that ChromeViewMsg_SetContentSettingRules never handled OOPIFs
> correctly either.

Thanks, it doesn't sound like a regression.

I'll assign the bug (http://crbug.com/767539) to me, but I don't work
on Mojo per se any more, so I don't expect to work on this Priority-3
bug any time soon. Anyone else (perhaps from site-isolation-dev) is
free to take it.