AW NS: implement MIME type sniffing for Android Streams

Only affects the NetworkService code path. This implements MIME type
sniffing when the app provides a null MIME type in any of the following
cases:

 - shouldInterceptRequest() callback API
 - ContentProvider (loaded from content:// URL)
 - file:///android_{asset,res} URL (when we can't already guess the MIME
   type from the path name).

This is done to accommodate legacy apps, which relied on this internal
detail of the legacy network path.

This implementation is behind a feature flag (enabled by default) as an
extra precaution, in case it causes unexpected problems.

Bug: 985491
Test: manual - verify this fixes the GoldenDict application
Test: run_android_webview_unittests -f AndroidStreamReaderURLLoaderTest.*
Test: run_webview_instrumentation_test_apk -f AwContentsClientFullScreenTest.*
Change-Id: I4f81657206c218e4835f3d0a59ce5f5ce65932c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1721878
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682483}

android_webview/browser/aw_feature_list.cc

@@ -51,6 +51,19 @@ const base::Feature kWebViewBrotliSupport{"WebViewBrotliSupport",
 const base::Feature kWebViewConnectionlessSafeBrowsing{
     "WebViewConnectionlessSafeBrowsing", base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Sniff the content stream to guess the MIME type when the application doesn't
+// tell us the MIME type explicitly.
+//
+// This only applies:
+// * when NetworkService is enabled (if disabled, the legacy net path sniffs
+//   content anyway, as an implementation detail).
+// * to app-provided content (shouldInterceptRequest,
+//   file:///android_{asset,res} URLs, content:// URLs), rather than content
+//   from the net stack (we may sniff content from the net stack anyway,
+//   depending on headers, but that's a NetworkService implementation detail).
+const base::Feature kWebViewSniffMimeType{"WebViewSniffMimeType",
+                                          base::FEATURE_ENABLED_BY_DEFAULT};
+
 // Enable raster in wide color gamut for apps that use webview in a wide color
 // gamut activity.
 const base::Feature kWebViewWideColorGamutSupport{

android_webview/browser/aw_feature_list.h

@@ -16,6 +16,7 @@ namespace features {
 // Alphabetical:
 extern const base::Feature kWebViewBrotliSupport;
 extern const base::Feature kWebViewConnectionlessSafeBrowsing;
+extern const base::Feature kWebViewSniffMimeType;
 extern const base::Feature kWebViewWideColorGamutSupport;
 
 }  // namespace features

android_webview/browser/network_service/android_stream_reader_url_loader.cc

@@ -4,16 +4,19 @@
 
 #include "android_webview/browser/network_service/android_stream_reader_url_loader.h"
 
+#include "android_webview/browser/aw_feature_list.h"
 #include "android_webview/browser/input_stream.h"
 #include "android_webview/browser/net/input_stream_reader.h"
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/feature_list.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/task/post_task.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/base/io_buffer.h"
+#include "net/base/mime_sniffer.h"
 #include "net/http/http_status_code.h"
 #include "net/http/http_util.h"
 #include "services/network/public/cpp/url_loader_completion_status.h"
@@ -96,6 +99,8 @@ AndroidStreamReaderURLLoader::AndroidStreamReaderURLLoader(
     const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
     std::unique_ptr<ResponseDelegate> response_delegate)
     : resource_request_(resource_request),
+      resource_response_head_(
+          std::make_unique<network::ResourceResponseHead>()),
       client_(std::move(client)),
       traffic_annotation_(traffic_annotation),
       response_delegate_(std::move(response_delegate)),
@@ -213,7 +218,7 @@ void AndroidStreamReaderURLLoader::HeadersComplete(
   // HttpResponseHeaders expects its input string to be terminated by two NULs.
   status.append("\0\0", 2);
 
-  network::ResourceResponseHead head;
+  network::ResourceResponseHead& head = *resource_response_head_;
   head.request_start = base::TimeTicks::Now();
   head.response_start = base::TimeTicks::Now();
   head.headers = new net::HttpResponseHeaders(status);
@@ -256,18 +261,14 @@ void AndroidStreamReaderURLLoader::HeadersComplete(
   // file resources?). The old path does this as well.
   head.headers->AddHeader(kResponseHeaderViaShouldInterceptRequest);
 
-  DCHECK(client_.is_bound());
-  client_->OnReceiveResponse(head);
-
   SendBody();
 }
 
 void AndroidStreamReaderURLLoader::SendBody() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  mojo::ScopedDataPipeConsumerHandle consumer_handle;
   if (CreateDataPipe(nullptr /*options*/, &producer_handle_,
-                     &consumer_handle) != MOJO_RESULT_OK) {
+                     &consumer_handle_) != MOJO_RESULT_OK) {
     RequestComplete(net::ERR_FAILED);
     return;
   }
@@ -275,11 +276,30 @@ void AndroidStreamReaderURLLoader::SendBody() {
       producer_handle_.get(), MOJO_HANDLE_SIGNAL_WRITABLE,
       base::BindRepeating(&AndroidStreamReaderURLLoader::OnDataPipeWritable,
                           base::Unretained(this)));
-  client_->OnStartLoadingResponseBody(std::move(consumer_handle));
 
+  // Send the response if possible now for 2 reasons:
+  // 1. If we don't need any more MIME type sniffing, there's no reason not to
+  //    tell the URLLoaderClient right away. Sending now should preserve
+  //    ordering between app-visible callbacks and the first read of the
+  //    InputStream (although we do not generally guarantee the ordering).
+  // 2. Sending this now lets us unittest the net::ERR_ABORTED case. The case
+  //    needs the ability to break the stream after getting the headers but
+  //    before finishing the read.
+  if (!base::FeatureList::IsEnabled(features::kWebViewSniffMimeType) ||
+      !resource_response_head_->mime_type.empty()) {
+    SendResponseToClient();
+  }
   ReadMore();
 }
 
+void AndroidStreamReaderURLLoader::SendResponseToClient() {
+  DCHECK(consumer_handle_.is_valid());
+  DCHECK(client_.is_bound());
+  client_->OnReceiveResponse(*resource_response_head_);
+  resource_response_head_ = nullptr;
+  client_->OnStartLoadingResponseBody(std::move(consumer_handle_));
+}
+
 void AndroidStreamReaderURLLoader::ReadMore() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!pending_buffer_.get());
@@ -336,6 +356,30 @@ void AndroidStreamReaderURLLoader::DidRead(int result) {
     RequestComplete(net::OK);
     return;
   }
+  if (consumer_handle_.is_valid()) {
+    // We only hit this on for the first buffer read, which we expect to be
+    // enough to determine the MIME type.
+    DCHECK(base::FeatureList::IsEnabled(features::kWebViewSniffMimeType));
+    if (resource_response_head_->mime_type.empty()) {
+      // Limit sniffing to the first net::kMaxBytesToSniff.
+      size_t data_length = result;
+      if (data_length > net::kMaxBytesToSniff)
+        data_length = net::kMaxBytesToSniff;
+
+      std::string new_type;
+      net::SniffMimeType(pending_buffer_->buffer(), data_length,
+                         resource_request_.url, std::string(),
+                         net::ForceSniffFileUrlsForHtml::kDisabled, &new_type);
+      // SniffMimeType() returns false if there is not enough data to
+      // determine the mime type. However, even if it returns false, it
+      // returns a new type that is probably better than the current one.
+      resource_response_head_->mime_type.assign(new_type);
+      resource_response_head_->did_mime_sniff = true;
+    }
+
+    SendResponseToClient();
+  }
+
   producer_handle_ = pending_buffer_->Complete(result);
   pending_buffer_ = nullptr;
 
@@ -357,6 +401,10 @@ void AndroidStreamReaderURLLoader::OnDataPipeWritable(MojoResult result) {
 
 void AndroidStreamReaderURLLoader::RequestComplete(int status_code) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  if (consumer_handle_.is_valid()) {
+    // We can hit this before reading any buffers under error conditions.
+    SendResponseToClient();
+  }
 
   client_->OnComplete(network::URLLoaderCompletionStatus(status_code));
   CleanUp();

android_webview/browser/network_service/android_stream_reader_url_loader.h

@@ -88,14 +88,26 @@ class AndroidStreamReaderURLLoader : public network::mojom::URLLoader {
 
   void OnDataPipeWritable(MojoResult result);
   void CleanUp();
+
+  // Called after trying to read some bytes from the stream. |result| can be a
+  // positive number (the number of bytes read), zero (no bytes were read
+  // because the stream is finished), or negative (error condition).
   void DidRead(int result);
+  // Reads some bytes from the stream. Calls |DidRead| after each read (also, in
+  // the case where it fails to read due to an error).
   void ReadMore();
+  // Send response headers and the data pipe consumer handle (for the body) to
+  // the URLLoaderClient. Requires |consumer_handle_| to be valid, and will make
+  // |consumer_handle_| invalid after running.
+  void SendResponseToClient();
 
   // Expected content size
   int64_t expected_content_size_ = -1;
+  mojo::ScopedDataPipeConsumerHandle consumer_handle_;
 
   net::HttpByteRange byte_range_;
   network::ResourceRequest resource_request_;
+  std::unique_ptr<network::ResourceResponseHead> resource_response_head_;
   network::mojom::URLLoaderClientPtr client_;
   const net::MutableNetworkTrafficAnnotationTag traffic_annotation_;
   std::unique_ptr<ResponseDelegate> response_delegate_;

android_webview/browser/network_service/android_stream_reader_url_loader_unittest.cc

@@ -88,6 +88,10 @@ class TestResponseDelegate
  public:
   TestResponseDelegate(std::unique_ptr<InputStream> input_stream)
       : input_stream_(std::move(input_stream)) {}
+  TestResponseDelegate(std::unique_ptr<InputStream> input_stream,
+                       const std::string custom_mime_type)
+      : input_stream_(std::move(input_stream)),
+        custom_mime_type_(custom_mime_type) {}
   TestResponseDelegate(std::unique_ptr<InputStream> input_stream,
                        const std::string& custom_status,
                        const std::string& custom_header_name,
@@ -112,6 +116,10 @@ class TestResponseDelegate
                    const GURL& url,
                    android_webview::InputStream* stream,
                    std::string* mime_type) override {
+    if (!custom_mime_type_.empty()) {
+      *mime_type = custom_mime_type_;
+      return true;
+    }
     return false;
   }
 
@@ -138,6 +146,7 @@ class TestResponseDelegate
 
  private:
   std::unique_ptr<InputStream> input_stream_;
+  const std::string custom_mime_type_;
   const std::string custom_status_;
   const std::string custom_header_name_;
   const std::string custom_header_value_;
@@ -175,6 +184,19 @@ class AndroidStreamReaderURLLoaderTest : public ::testing::Test {
         std::make_unique<TestResponseDelegate>(std::move(input_stream)));
   }
 
+  // helper method for creating loaders given a stream and MIME type
+  AndroidStreamReaderURLLoader* CreateLoaderWithMimeType(
+      const network::ResourceRequest& request,
+      network::TestURLLoaderClient* client,
+      std::unique_ptr<InputStream> input_stream,
+      const std::string custom_mime_type) {
+    return new AndroidStreamReaderURLLoader(
+        request, client->CreateInterfacePtr(),
+        net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS),
+        std::make_unique<TestResponseDelegate>(std::move(input_stream),
+                                               custom_mime_type));
+  }
+
   // helper method for creating loaders given a stream and response header
   // values
   AndroidStreamReaderURLLoader* CreateLoaderWithCustomizedResponseHeader(
@@ -335,8 +357,12 @@ TEST_F(AndroidStreamReaderURLLoaderTest,
   std::string expected_body("test");
   std::unique_ptr<network::TestURLLoaderClient> client =
       std::make_unique<network::TestURLLoaderClient>();
-  AndroidStreamReaderURLLoader* loader = CreateLoader(
-      request, client.get(), std::make_unique<FakeInputStream>(expected_body));
+  // Need a valid MIME type, otherwise we won't get headers until we've already
+  // read the input stream (and we need to interrupt the read in this test).
+  std::string valid_mime_type("text/html");
+  AndroidStreamReaderURLLoader* loader = CreateLoaderWithMimeType(
+      request, client.get(), std::make_unique<FakeInputStream>(expected_body),
+      valid_mime_type);
   loader->Start();
   client->RunUntilResponseBodyArrived();
   EXPECT_TRUE(client->has_received_response());