Clarify StoragePartition::GetNetworkContext() contract and fix devtools_server.cc

The raw pointer returned by GetNetworkContext() shouldn't be hold onto. This
CL adds documentation to the method and fixes devtools_server.cc to not hold
onto the raw pointer as a member.

Bug: 821000
Change-Id: If6e1a7c9a6742980755ffabd4c3b0d8fe0fa392e
Reviewed-on: https://chromium-review.googlesource.com/1042352Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555792}

components/ui_devtools/devtools_server.cc

@@ -67,12 +67,9 @@ constexpr net::NetworkTrafficAnnotationTag kUIDevtoolsServer =
 
 UiDevToolsServer* UiDevToolsServer::devtools_server_ = nullptr;
 
-UiDevToolsServer::UiDevToolsServer(
-    network::mojom::NetworkContext* network_context,
-    const char* enable_devtools_flag,
+UiDevToolsServer::UiDevToolsServer(const char* enable_devtools_flag,
                                    int default_port)
-    : network_context_(network_context),
-      port_(GetUiDevToolsPort(enable_devtools_flag, default_port)),
+    : port_(GetUiDevToolsPort(enable_devtools_flag, default_port)),
       weak_ptr_factory_(this) {
   DCHECK(!devtools_server_);
   devtools_server_ = this;
@@ -90,9 +87,8 @@ std::unique_ptr<UiDevToolsServer> UiDevToolsServer::Create(
   std::unique_ptr<UiDevToolsServer> server;
   if (IsDevToolsEnabled(enable_devtools_flag) && !devtools_server_) {
     // TODO(mhashmi): Change port if more than one inspectable clients
-    server.reset(new UiDevToolsServer(network_context, enable_devtools_flag,
-                                      default_port));
-    server->Start("0.0.0.0");
+    server.reset(new UiDevToolsServer(enable_devtools_flag, default_port));
+    server->Start(network_context, "0.0.0.0");
   }
   return server;
 }
@@ -125,7 +121,8 @@ void UiDevToolsServer::SendOverWebSocket(int connection_id,
   server_->SendOverWebSocket(connection_id, message, kUIDevtoolsServer);
 }
 
-void UiDevToolsServer::Start(const std::string& address_string) {
+void UiDevToolsServer::Start(network::mojom::NetworkContext* network_context,
+                             const std::string& address_string) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(devtools_server_sequence_);
   DCHECK(!server_);
 
@@ -136,7 +133,7 @@ void UiDevToolsServer::Start(const std::string& address_string) {
     return;
 
   constexpr int kBacklog = 1;
-  network_context_->CreateTCPServerSocket(
+  network_context->CreateTCPServerSocket(
       net::IPEndPoint(address, port_), kBacklog,
       net::MutableNetworkTrafficAnnotationTag(kUIDevtoolsServer),
       mojo::MakeRequest(&server_socket),

components/ui_devtools/devtools_server.h

@@ -44,11 +44,10 @@ class UI_DEVTOOLS_EXPORT UiDevToolsServer
   int port() const { return port_; }
 
  private:
-  UiDevToolsServer(network::mojom::NetworkContext* network_context,
-                   const char* enable_devtools_flag,
-                   int default_port);
+  UiDevToolsServer(const char* enable_devtools_flag, int default_port);
 
-  void Start(const std::string& address_string);
+  void Start(network::mojom::NetworkContext* network_context,
+             const std::string& address_string);
   void MakeServer(network::mojom::TCPServerSocketPtr server_socket,
                   int result,
                   const base::Optional<net::IPEndPoint>& local_addr);
@@ -70,7 +69,6 @@ class UI_DEVTOOLS_EXPORT UiDevToolsServer
   ConnectionsMap connections_;
 
   std::unique_ptr<network::server::HttpServer> server_;
-  network::mojom::NetworkContext* network_context_;
 
   // The port the devtools server listens on
   const int port_;

content/public/browser/storage_partition.h

@@ -76,7 +76,12 @@ class CONTENT_EXPORT StoragePartition {
   virtual base::FilePath GetPath() = 0;
   virtual net::URLRequestContextGetter* GetURLRequestContext() = 0;
   virtual net::URLRequestContextGetter* GetMediaURLRequestContext() = 0;
+
+  // Returns a raw mojom::NetworkContext pointer. When network service crashes
+  // or restarts, the raw pointer will not be valid or safe to use. Therefore,
+  // caller should not hold onto this pointer beyond the same message loop task.
   virtual network::mojom::NetworkContext* GetNetworkContext() = 0;
+
   // Returns a pointer/info to a URLLoaderFactory/CookieManager owned by
   // the storage partition. Prefer to use this instead of creating a new
   // URLLoaderFactory when issuing requests from the Browser process, to