Skip to content

Commit

Permalink
Revert of Convert network hints to Mojo (patchset Pissandshittium#16
Browse files Browse the repository at this point in the history
…id:300001 of https://codereview.chromium.org/2144533002/ )

BUG=630749
NOPRESUBMIT=true

Skipping presubmit as I'm removing OWNERS files I added.

Reason for revert:
Use-after-free issue:

https://bugs.chromium.org/p/chromium/issues/detail?id=630749

Original issue's description:
> Convert network hints to Mojo
>
> Previously landed as: refs/heads/master@{#406780}
>
> Committed: https://crrev.com/1efd0ad3153587609220eeec1be6c1b2bbf9964b
> Cr-Commit-Position: refs/heads/master@{#407030}

TBR=sammc@chromium.org,dcheng@chromium.org,juliatuttle@chromium.org,halliwell@chromium.org,sky@chromium.org,mbarbella@chromium.org,blundell@chromium.org,mbarbella@google.com
# Not skipping CQ checks because original CL landed more than 1 days ago.

Review-Url: https://codereview.chromium.org/2179693002
Cr-Commit-Position: refs/heads/master@{#407395}
  • Loading branch information
tibell authored and Commit bot committed Jul 25, 2016
1 parent 9756dca commit 0773f76
Show file tree
Hide file tree
Showing 40 changed files with 193 additions and 318 deletions.
1 change: 0 additions & 1 deletion chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,6 @@ split_static_library("browser") {
"//components/navigation_interception",
"//components/net_log",
"//components/network_hints/common",
"//components/network_hints/public/interfaces:network_hints_mojom",
"//components/password_manager/content/browser",
"//components/password_manager/sync/browser",
"//components/profile_metrics",
Expand Down
9 changes: 1 addition & 8 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -957,14 +957,7 @@ void ChromeContentBrowserClient::RenderProcessWillLaunch(
net::URLRequestContextGetter* context =
host->GetStoragePartition()->GetURLRequestContext();

// The host owns both |chrome_render| and the interface registry, which will
// be destroyed before the filter.
auto* chrome_render_filter = new ChromeRenderMessageFilter(id, profile);
host->GetInterfaceRegistry()->AddInterface(
base::Bind(&ChromeRenderMessageFilter::BindNetworkHints,
base::Unretained(chrome_render_filter)),
BrowserThread::GetTaskRunnerForThread(BrowserThread::IO));
host->AddFilter(chrome_render_filter);
host->AddFilter(new ChromeRenderMessageFilter(id, profile));
#if defined(ENABLE_EXTENSIONS)
host->AddFilter(new cast::CastTransportHostFilter);
#endif
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/extensions/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ static_library("extensions") {
"//chrome/installer/util:with_no_strings",
"//components/data_reduction_proxy/proto:data_reduction_proxy_proto",
"//components/dom_distiller/core",
"//components/network_hints/public/interfaces:network_hints_mojom",
"//components/onc",
"//components/policy",
"//components/proximity_auth",
Expand Down
23 changes: 11 additions & 12 deletions chrome/browser/renderer_host/chrome_render_message_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "components/content_settings/content/common/content_settings_messages.h"
#include "components/content_settings/core/browser/cookie_settings.h"
#include "components/network_hints/common/network_hints_common.h"
#include "components/network_hints/common/network_hints_messages.h"
#include "components/rappor/rappor_service.h"
#include "components/rappor/rappor_utils.h"
#include "components/web_cache/browser/web_cache_manager.h"
Expand All @@ -41,8 +42,9 @@ using content::BrowserThread;

namespace {

const uint32_t kFilteredMessageClasses[] = {ChromeMsgStart,
ContentSettingsMsgStart};
const uint32_t kFilteredMessageClasses[] = {
ChromeMsgStart, ContentSettingsMsgStart, NetworkHintsMsgStart,
};

} // namespace

Expand All @@ -62,6 +64,8 @@ ChromeRenderMessageFilter::~ChromeRenderMessageFilter() {
bool ChromeRenderMessageFilter::OnMessageReceived(const IPC::Message& message) {
bool handled = true;
IPC_BEGIN_MESSAGE_MAP(ChromeRenderMessageFilter, message)
IPC_MESSAGE_HANDLER(NetworkHintsMsg_DNSPrefetch, OnDnsPrefetch)
IPC_MESSAGE_HANDLER(NetworkHintsMsg_Preconnect, OnPreconnect)
IPC_MESSAGE_HANDLER(ChromeViewHostMsg_UpdatedCacheStats,
OnUpdatedCacheStats)
IPC_MESSAGE_HANDLER(ChromeViewHostMsg_AllowDatabase, OnAllowDatabase)
Expand Down Expand Up @@ -102,17 +106,17 @@ void ChromeRenderMessageFilter::OverrideThreadForMessage(
}
}

void ChromeRenderMessageFilter::DNSPrefetch(
void ChromeRenderMessageFilter::OnDnsPrefetch(
const network_hints::LookupRequest& request) {
if (predictor_)
predictor_->DnsPrefetchList(request.hostname_list);
}

void ChromeRenderMessageFilter::Preconnect(const GURL& url,
bool allow_credentials,
int count) {
void ChromeRenderMessageFilter::OnPreconnect(const GURL& url,
bool allow_credentials,
int count) {
if (count < 1) {
LOG(WARNING) << "NetworkHints::Preconnect IPC with invalid count: "
LOG(WARNING) << "NetworkHintsMsg_Preconnect IPC with invalid count: "
<< count;
return;
}
Expand All @@ -124,11 +128,6 @@ void ChromeRenderMessageFilter::Preconnect(const GURL& url,
}
}

void ChromeRenderMessageFilter::BindNetworkHints(
network_hints::mojom::NetworkHintsRequest request) {
bindings_.AddBinding(this, std::move(request));
}

void ChromeRenderMessageFilter::OnUpdatedCacheStats(
uint64_t min_dead_capacity,
uint64_t max_dead_capacity,
Expand Down
15 changes: 3 additions & 12 deletions chrome/browser/renderer_host/chrome_render_message_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@
#include "base/callback.h"
#include "base/macros.h"
#include "base/sequenced_task_runner_helpers.h"
#include "components/network_hints/public/interfaces/network_hints.mojom.h"
#include "content/public/browser/browser_message_filter.h"
#include "mojo/public/cpp/bindings/binding_set.h"

class GURL;
class Profile;
Expand All @@ -36,8 +34,7 @@ class InfoMap;

// This class filters out incoming Chrome-specific IPC messages for the renderer
// process on the IPC thread.
class ChromeRenderMessageFilter : public content::BrowserMessageFilter,
public network_hints::mojom::NetworkHints {
class ChromeRenderMessageFilter : public content::BrowserMessageFilter {
public:
ChromeRenderMessageFilter(int render_process_id, Profile* profile);

Expand All @@ -46,18 +43,14 @@ class ChromeRenderMessageFilter : public content::BrowserMessageFilter,
void OverrideThreadForMessage(const IPC::Message& message,
content::BrowserThread::ID* thread) override;

void BindNetworkHints(network_hints::mojom::NetworkHintsRequest request);

private:
friend class content::BrowserThread;
friend class base::DeleteHelper<ChromeRenderMessageFilter>;

~ChromeRenderMessageFilter() override;

// mojom::NetworkHints methods:
void DNSPrefetch(const network_hints::LookupRequest& request) override;
void Preconnect(const GURL& url, bool allow_credentials, int count) override;

void OnDnsPrefetch(const network_hints::LookupRequest& request);
void OnPreconnect(const GURL& url, bool allow_credentials, int count);
void OnUpdatedCacheStats(uint64_t min_capacity,
uint64_t max_capacity,
uint64_t capacity,
Expand Down Expand Up @@ -139,8 +132,6 @@ class ChromeRenderMessageFilter : public content::BrowserMessageFilter,
// Used to look up permissions at database creation time.
scoped_refptr<content_settings::CookieSettings> cookie_settings_;

mojo::BindingSet<network_hints::mojom::NetworkHints> bindings_;

DISALLOW_COPY_AND_ASSIGN(ChromeRenderMessageFilter);
};

Expand Down
3 changes: 0 additions & 3 deletions chrome/chrome_browser.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -3297,7 +3297,6 @@
'../components/components.gyp:metrics_services_manager',
'../components/components.gyp:metrics_ui',
'../components/components.gyp:navigation_metrics',
'../components/components.gyp:network_hints_mojom',
'../components/components.gyp:network_time',
'../components/components.gyp:offline_pages',
'../components/components.gyp:omnibox_browser',
Expand Down Expand Up @@ -3430,8 +3429,6 @@
'../components/components.gyp:navigation_interception',
'../components/components.gyp:net_log',
'../components/components.gyp:network_hints_common',
'../components/components.gyp:network_hints_mojom',
'../components/components.gyp:network_hints_public_cpp',
'../components/components.gyp:network_session_configurator',
'../components/components.gyp:ntp_snippets',
'../components/components.gyp:ntp_tiles',
Expand Down
4 changes: 3 additions & 1 deletion chrome/renderer/chrome_content_renderer_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,8 @@ void ChromeContentRendererClient::RenderThreadStarted() {
ChromeExtensionsRendererClient::GetInstance()->RenderThreadStarted();
#endif

prescient_networking_dispatcher_.reset(
new network_hints::PrescientNetworkingDispatcher());
#if defined(ENABLE_SPELLCHECK)
// ChromeRenderViewTest::SetUp() creates a Spellcheck and injects it using
// SetSpellcheck(). Don't overwrite it.
Expand Down Expand Up @@ -1138,7 +1140,7 @@ bool ChromeContentRendererClient::IsLinkVisited(unsigned long long link_hash) {

blink::WebPrescientNetworking*
ChromeContentRendererClient::GetPrescientNetworking() {
return chrome_observer_->prescient_networking_dispatcher();
return prescient_networking_dispatcher_.get();
}

bool ChromeContentRendererClient::ShouldOverridePageVisibilityState(
Expand Down
3 changes: 3 additions & 0 deletions chrome/renderer/chrome_content_renderer_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ class ChromeContentRendererClient : public content::ContentRendererClient {
std::unique_ptr<ChromeRenderThreadObserver> chrome_observer_;
std::unique_ptr<web_cache::WebCacheImpl> web_cache_impl_;

std::unique_ptr<network_hints::PrescientNetworkingDispatcher>
prescient_networking_dispatcher_;

#if defined(ENABLE_SPELLCHECK)
std::unique_ptr<SpellCheck> spellcheck_;
#endif
Expand Down
10 changes: 1 addition & 9 deletions chrome/renderer/chrome_render_thread_observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
#include "chrome/common/url_constants.h"
#include "chrome/renderer/content_settings_observer.h"
#include "chrome/renderer/security_filter_peer.h"
#include "components/network_hints/renderer/prescient_networking_dispatcher.h"
#include "content/public/child/resource_dispatcher_delegate.h"
#include "content/public/renderer/render_thread.h"
#include "content/public/renderer/render_view.h"
Expand Down Expand Up @@ -236,10 +235,7 @@ void CreateResourceUsageReporter(
bool ChromeRenderThreadObserver::is_incognito_process_ = false;

ChromeRenderThreadObserver::ChromeRenderThreadObserver()
: field_trial_syncer_(this),
prescient_networking_dispatcher_(
new network_hints::PrescientNetworkingDispatcher()),
weak_factory_(this) {
: field_trial_syncer_(this), weak_factory_(this) {
const base::CommandLine& command_line =
*base::CommandLine::ForCurrentProcess();

Expand Down Expand Up @@ -287,10 +283,6 @@ bool ChromeRenderThreadObserver::OnControlMessageReceived(
return handled;
}

void ChromeRenderThreadObserver::OnRenderProcessShutdown() {
prescient_networking_dispatcher_.reset();
}

void ChromeRenderThreadObserver::OnFieldTrialGroupFinalized(
const std::string& trial_name,
const std::string& group_name) {
Expand Down
13 changes: 0 additions & 13 deletions chrome/renderer/chrome_render_thread_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ namespace content {
class ResourceDispatcherDelegate;
}

namespace network_hints {
class PrescientNetworkingDispatcher;
}

// This class filters the incoming control messages (i.e. ones not destined for
// a RenderView) for Chrome specific messages that the content layer doesn't
// happen. If a few messages are related, they should probably have their own
Expand All @@ -47,19 +43,13 @@ class ChromeRenderThreadObserver : public content::RenderThreadObserver,
// |ChromeRenderThreadObserver|.
const RendererContentSettingRules* content_setting_rules() const;

network_hints::PrescientNetworkingDispatcher*
prescient_networking_dispatcher() {
return prescient_networking_dispatcher_.get();
}

private:
// Initializes field trial state change observation and notifies the browser
// of any field trials that might have already been activated.
void InitFieldTrialObserving(const base::CommandLine& command_line);

// content::RenderThreadObserver:
bool OnControlMessageReceived(const IPC::Message& message) override;
void OnRenderProcessShutdown() override;

// base::FieldTrialList::Observer:
void OnFieldTrialGroupFinalized(const std::string& trial_name,
Expand All @@ -78,9 +68,6 @@ class ChromeRenderThreadObserver : public content::RenderThreadObserver,
RendererContentSettingRules content_setting_rules_;
chrome_variations::ChildProcessFieldTrialSyncer field_trial_syncer_;

std::unique_ptr<network_hints::PrescientNetworkingDispatcher>
prescient_networking_dispatcher_;

base::WeakPtrFactory<ChromeRenderThreadObserver> weak_factory_;

DISALLOW_COPY_AND_ASSIGN(ChromeRenderThreadObserver);
Expand Down
29 changes: 11 additions & 18 deletions chromecast/browser/cast_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
#include "chromecast/public/media/media_pipeline_backend.h"
#include "components/crash/content/app/breakpad_linux.h"
#include "components/crash/content/browser/crash_handler_host_linux.h"
#include "components/network_hints/browser/network_hints_impl.h"
#include "components/network_hints/browser/network_hints_message_filter.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/certificate_request_result_type.h"
#include "content/public/browser/client_certificate_delegate.h"
Expand All @@ -50,7 +50,6 @@
#include "content/public/common/web_preferences.h"
#include "net/ssl/ssl_cert_request_info.h"
#include "net/url_request/url_request_context_getter.h"
#include "services/shell/public/cpp/interface_registry.h"
#include "ui/display/display.h"
#include "ui/display/screen.h"
#include "ui/gl/gl_switches.h"
Expand Down Expand Up @@ -192,32 +191,26 @@ void CastContentBrowserClient::RenderProcessWillLaunch(
// getting HostResolver.
content::BrowserThread::PostTaskAndReplyWithResult(
content::BrowserThread::IO, FROM_HERE,
base::Bind(
&net::URLRequestContextGetter::GetURLRequestContext,
base::Unretained(url_request_context_factory_->GetSystemGetter())),
base::Bind(&CastContentBrowserClient::AddNetworkHintsImpl,
base::Bind(&net::URLRequestContextGetter::GetURLRequestContext,
base::Unretained(
url_request_context_factory_->GetSystemGetter())),
base::Bind(&CastContentBrowserClient::AddNetworkHintsMessageFilter,
base::Unretained(this), host->GetID()));
}

void CastContentBrowserClient::AddNetworkHintsImpl(
int render_process_id,
net::URLRequestContext* context) {
void CastContentBrowserClient::AddNetworkHintsMessageFilter(
int render_process_id, net::URLRequestContext* context) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

content::RenderProcessHost* host =
content::RenderProcessHost::FromID(render_process_id);
if (!host)
return;

network_hints_.reset(new network_hints::NetworkHintsImpl(
url_request_context_factory_->host_resolver()));
// The client outlives the host, which owns the registry, and thus
// |network_hints_| will outlive the registry.
host->GetInterfaceRegistry()->AddInterface(
base::Bind(&network_hints::NetworkHintsImpl::Bind,
base::Unretained(network_hints_.get())),
content::BrowserThread::GetTaskRunnerForThread(
content::BrowserThread::IO));
scoped_refptr<content::BrowserMessageFilter> network_hints_message_filter(
new network_hints::NetworkHintsMessageFilter(
url_request_context_factory_->host_resolver()));
host->AddFilter(network_hints_message_filter.get());
}

bool CastContentBrowserClient::IsHandledURL(const GURL& url) {
Expand Down
10 changes: 2 additions & 8 deletions chromecast/browser/cast_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ namespace metrics {
class MetricsService;
}

namespace network_hints {
class NetworkHintsImpl;
}

namespace chromecast {
class CastService;

Expand Down Expand Up @@ -173,8 +169,8 @@ class CastContentBrowserClient : public content::ContentBrowserClient {
}

private:
void AddNetworkHintsImpl(int render_process_id,
net::URLRequestContext* context);
void AddNetworkHintsMessageFilter(int render_process_id,
net::URLRequestContext* context);

net::X509Certificate* SelectClientCertificateOnIOThread(
GURL requesting_url,
Expand All @@ -196,8 +192,6 @@ class CastContentBrowserClient : public content::ContentBrowserClient {
CastBrowserMainParts* cast_browser_main_parts_;
std::unique_ptr<URLRequestContextFactory> url_request_context_factory_;

std::unique_ptr<network_hints::NetworkHintsImpl> network_hints_;

DISALLOW_COPY_AND_ASSIGN(CastContentBrowserClient);
};

Expand Down
Loading

0 comments on commit 0773f76

Please sign in to comment.