[remoting host] Fix crash in ProtobufHttpStreamRequest

For authenticated requests, ProtobufHttpClient first invokes
OAuthTokenGetter to get the access token, then enqueue and start the
request and pass in the invalidator. But if the access token fails to
fetch, ProtobufHttpClient will instead just call OnAuthFailed() without
starting the request or passing in the invalidator.

The previous OnAuthFailed() implementation of ProtobufHttpStreamRequest
was broken, since it just called OnStreamClosed(), which assumes the
invalidator_ exists and calls it. ProtobufHttpRequest OTOH seems to be
intact and we have test to guard that.

This CL fixes this issue on ProtobufHttpStreamRequest and add a test
case for it.

Bug: 1129567
Change-Id: I45d295b5a744657673568e937cf638f2013f188e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2417143
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Auto-Submit: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Joe Downing <joedow@google.com>
Cr-Commit-Position: refs/heads/master@{#808106}

remoting/base/protobuf_http_client_unittest.cc

@@ -433,6 +433,31 @@ TEST_F(ProtobufHttpClientTest, DeletesRequestHolderAfterResponseIsReceived) {
 
 // Stream request tests.
 
+TEST_F(ProtobufHttpClientTest,
+       StreamRequestFailedToFetchAuthToken_RejectsWithUnauthorizedError) {
+  base::MockOnceClosure stream_ready_callback;
+  MockEchoMessageCallback message_callback;
+  MockStreamClosedCallback stream_closed_callback;
+
+  base::RunLoop run_loop;
+
+  ExpectCallWithToken(/* success= */ false);
+
+  MockEchoResponseCallback response_callback;
+  EXPECT_CALL(stream_closed_callback,
+              Run(HasErrorCode(ProtobufHttpStatus::Code::UNAUTHENTICATED)))
+      .WillOnce([&]() { run_loop.Quit(); });
+
+  auto request = CreateDefaultTestStreamRequest();
+  request->SetStreamReadyCallback(stream_ready_callback.Get());
+  request->SetMessageCallback(message_callback.Get());
+  request->SetStreamClosedCallback(stream_closed_callback.Get());
+  client_.ExecuteRequest(std::move(request));
+
+  run_loop.Run();
+  ASSERT_FALSE(client_.HasPendingRequests());
+}
+
 TEST_F(ProtobufHttpClientTest, StartStreamRequestAndDecodeMessages) {
   base::MockOnceClosure stream_ready_callback;
   MockEchoMessageCallback message_callback;

remoting/base/protobuf_http_request_base.h

@@ -46,7 +46,11 @@ class ProtobufHttpRequestBase {
   const ProtobufHttpRequestConfig& config() const { return *config_; }
 
  protected:
+  // Called when the protobuf HTTP client failed to get the access token. Note
+  // that the subclass implementation should not invoke |invalidator_| since the
+  // request has never been started.
   virtual void OnAuthFailed(const ProtobufHttpStatus& status) = 0;
+
   virtual void StartRequestInternal(
       network::mojom::URLLoaderFactory* loader_factory) = 0;
 

remoting/base/protobuf_http_stream_request.cc

@@ -58,7 +58,8 @@ void ProtobufHttpStreamRequest::OnStreamClosed(
 }
 
 void ProtobufHttpStreamRequest::OnAuthFailed(const ProtobufHttpStatus& status) {
-  OnStreamClosed(status);
+  // Can't call OnStreamClosed here since it invokes the |invalidator_|.
+  std::move(stream_closed_callback_).Run(status);
 }
 
 void ProtobufHttpStreamRequest::StartRequestInternal(