New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 622050 link

Starred by 2 users
Status: Fixed
Owner:
scottmg@chromium.org
Closed: Jun 2016
Cc:
yhirano@chromium.org
jam@chromium.org
kinuko@chromium.org
█ ananta@chromium.org
█ yzshen@chromium.org
scottmg@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug
Proj-Servicification

Blocking:
issue 598073



Sign in to add a comment

Remove RenderViewHostImpl usage from ResourceDispatcherHostImpl

Project Member Reported by scottmg@chromium.org, Jun 21 2016 
During loading, ResourceDispatcherHostImpl starts upload_load_states_timer_, which is a repeating timer that periodically calls UploadLoadInfo().

This in turn gets load info for all pending loaders and informs the RenderViewHostImpl that something is going on via LoadStateChanged(), which goes to RenderViewHostDelegate::LoadStateChanged(), and then to WebContentsImpl::LoadStateChanged().

In order to remove the dependency from ResourceDispatcherHostImpl to renderer_host/render_view_host_impl.h, I propose to instead have UpdateLoadInfo() build a mojo message containing the data that is currently in ResourceDispatcherHostImpl::LoadInfoMap and send it to the browser.

I think it can be handled directly by WebContentsImpl, as that's the only place that uses it, but I'm not sure about that yet.

Comment 1 by scottmg@chromium.org, Jun 22 2016 

Does it make sense to have ResourceDispatchHostImpl speak to RenderFrameHostImpl via mojo? It sort of made sense to me (i.e. loader -> browser) but we don't have this yet.

Meant to mention in the original comment, this change shouldn't be latency or ordering sensitive, as it's already on a sort of "when we get around to it" 250ms timer.
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 29 2016 

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

commit 5cbd1cf54e80149c704295563e9466712f86db56
Author: scottmg <scottmg@chromium.org>
Date: Wed Jun 29 02:34:52 2016

Remove dependency of ResourceDispatcherHostImpl on RenderViewHostImpl

Create an async interface LoaderDelegate to delegate from c/b/loader to
the rest of content. This allows decoupling of loader so that it can
eventually be moved to a separate service.

Start by using it to remove the dependency on RenderViewHostImpl.

R=jam
BUG= 622050 

Review-Url: https://codereview.chromium.org/2098923002
Cr-Commit-Position: refs/heads/master@{#402680}

[modify] https://crrev.com/5cbd1cf54e80149c704295563e9466712f86db56/content/browser/browser_main_loop.cc
[modify] https://crrev.com/5cbd1cf54e80149c704295563e9466712f86db56/content/browser/browser_main_loop.h
[modify] https://crrev.com/5cbd1cf54e80149c704295563e9466712f86db56/content/browser/loader/DEPS
[add] https://crrev.com/5cbd1cf54e80149c704295563e9466712f86db56/content/browser/loader/loader_delegate.h
[modify] https://crrev.com/5cbd1cf54e80149c704295563e9466712f86db56/content/browser/loader/resource_dispatcher_host_impl.cc
[modify] https://crrev.com/5cbd1cf54e80149c704295563e9466712f86db56/content/browser/loader/resource_dispatcher_host_impl.h
[add] https://crrev.com/5cbd1cf54e80149c704295563e9466712f86db56/content/browser/loader_delegate_impl.cc
[add] https://crrev.com/5cbd1cf54e80149c704295563e9466712f86db56/content/browser/loader_delegate_impl.h
[modify] https://crrev.com/5cbd1cf54e80149c704295563e9466712f86db56/content/content_browser.gypi

Comment 3 by scottmg@chromium.org, Jun 29 2016

Status: Fixed (was: Started)

Comment 4 by laforge@google.com, Nov 7 2017

Components: Internals>Network>Service

Comment 5 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
►

Sign in to add a comment