[PDF] Use chrome_pdf::Result in more code

Since Result is an enum and not an enum class, it is very easy to mix up
this enum and ints. To prepare for making Result an enum class, start
using it in some callbacks. This forces the compiler to be stricter
about type checking. Then use Result in more places to fix the compile
errors. For url_loader_unittest.cc in particular, split the existing
mock callback variable into two separate variables for open and read
operations.

Change-Id: I1c830fd88aba39542fa60508d9e1c26ed1744e5c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6216483
Reviewed-by: Andy Phan <andyphan@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1413268}

pdf/loader/url_loader.cc

@@ -80,8 +80,7 @@ UrlLoader::UrlLoader(base::WeakPtr<Client> client)
 UrlLoader::~UrlLoader() = default;
 
 // Modeled on `content::PepperURLLoaderHost::OnHostMsgOpen()`.
-void UrlLoader::Open(const UrlRequest& request,
-                     base::OnceCallback<void(int)> callback) {
+void UrlLoader::Open(const UrlRequest& request, OpenCallback callback) {
   DCHECK_EQ(state_, LoadingState::kWaitingToOpen);
   DCHECK(callback);
   state_ = LoadingState::kOpening;
@@ -243,7 +242,7 @@ void UrlLoader::DidFail(const blink::WebURLError& error) {
          state_ == LoadingState::kStreamingData)
       << static_cast<int>(state_);
 
-  int32_t pp_error = Result::kErrorFailed;
+  Result pp_error = Result::kErrorFailed;
   switch (error.reason()) {
     case net::ERR_ACCESS_DENIED:
     case net::ERR_NETWORK_ACCESS_DENIED:
@@ -259,8 +258,8 @@ void UrlLoader::DidFail(const blink::WebURLError& error) {
   AbortLoad(pp_error);
 }
 
-void UrlLoader::AbortLoad(int32_t result) {
-  DCHECK_LT(result, 0);
+void UrlLoader::AbortLoad(Result result) {
+  CHECK_NE(result, Result::kSuccess);
 
   SetLoadComplete(result);
   buffer_.clear();
@@ -304,9 +303,8 @@ void UrlLoader::RunReadCallback() {
   std::move(read_callback_).Run(num_bytes);
 }
 
-void UrlLoader::SetLoadComplete(int32_t result) {
+void UrlLoader::SetLoadComplete(Result result) {
   DCHECK_NE(state_, LoadingState::kLoadComplete);
-  DCHECK_LE(result, 0);
 
   state_ = LoadingState::kLoadComplete;
   complete_result_ = result;

pdf/loader/url_loader.h

@@ -16,6 +16,7 @@
 #include "base/functional/callback.h"
 #include "base/memory/raw_span.h"
 #include "base/memory/weak_ptr.h"
+#include "pdf/loader/result_codes.h"
 #include "third_party/blink/public/web/web_associated_url_loader_client.h"
 
 namespace blink {
@@ -91,6 +92,8 @@ struct UrlResponse final {
 // `content::PepperURLLoaderHost` and `ppapi::proxy::URLLoaderResource`.
 class UrlLoader final : public blink::WebAssociatedURLLoaderClient {
  public:
+  using OpenCallback = base::OnceCallback<void(Result)>;
+
   // Client interface required by `UrlLoader`. Instances should be passed using
   // weak pointers, as the loader can be shared, and may outlive the client.
   class Client {
@@ -127,7 +130,7 @@ class UrlLoader final : public blink::WebAssociatedURLLoaderClient {
   ~UrlLoader() override;
 
   // Mimic `pp::URLLoader`:
-  void Open(const UrlRequest& request, base::OnceCallback<void(int)> callback);
+  void Open(const UrlRequest& request, OpenCallback callback);
   void ReadResponseBody(base::span<char> buffer,
                         base::OnceCallback<void(int)> callback);
   void Close();
@@ -166,22 +169,22 @@ class UrlLoader final : public blink::WebAssociatedURLLoaderClient {
   };
 
   // Aborts the load with `result`. Runs callback if pending.
-  void AbortLoad(int32_t result);
+  void AbortLoad(Result result);
 
   // Runs callback for `ReadResponseBody()` if pending.
   void RunReadCallback();
 
-  void SetLoadComplete(int32_t result);
+  void SetLoadComplete(Result result);
 
   base::WeakPtr<Client> client_;
 
   LoadingState state_ = LoadingState::kWaitingToOpen;
-  int32_t complete_result_ = 0;
+  Result complete_result_ = Result::kSuccess;
 
   std::unique_ptr<blink::WebAssociatedURLLoader> blink_loader_;
 
   bool ignore_redirects_ = false;
-  base::OnceCallback<void(int)> open_callback_;
+  OpenCallback open_callback_;
 
   UrlResponse response_;
 

pdf/loader/url_loader_unittest.cc

@@ -152,21 +152,23 @@ class UrlLoaderTest : public testing::Test {
     UrlRequest request;
     request.buffer_lower_threshold = lower;
     request.buffer_upper_threshold = upper;
-    loader_->Open(request, mock_callback_.Get());
+    loader_->Open(request, mock_open_callback_.Get());
     loader_->DidReceiveResponse(blink::WebURLResponse());
   }
 
-  int32_t DidFailWithError(const blink::WebURLError& error) {
-    int32_t result = 0;
-    loader_->Open(UrlRequest(), mock_callback_.Get());
-    EXPECT_CALL(mock_callback_, Run).WillOnce(SaveArg<0>(&result));
+  Result DidFailWithError(const blink::WebURLError& error) {
+    Result result = Result::kSuccess;
+    loader_->Open(UrlRequest(), mock_open_callback_.Get());
+    EXPECT_CALL(mock_open_callback_, Run).WillOnce(SaveArg<0>(&result));
 
     loader_->DidFail(error);
     return result;
   }
 
   FakeUrlLoaderClient fake_client_;
-  NiceMock<base::MockCallback<base::OnceCallback<void(int)>>> mock_callback_;
+  NiceMock<base::MockCallback<UrlLoader::OpenCallback>> mock_open_callback_;
+  NiceMock<base::MockCallback<base::OnceCallback<void(int)>>>
+      mock_read_callback_;
   std::unique_ptr<UrlLoader> loader_;
 
   // Becomes invalid if `loader_` is closed or destructed.
@@ -178,12 +180,12 @@ class UrlLoaderTest : public testing::Test {
 
 TEST_F(UrlLoaderTest, Open) {
   EXPECT_CALL(*mock_url_loader_, LoadAsynchronously);
-  EXPECT_CALL(mock_callback_, Run).Times(0);
+  EXPECT_CALL(mock_open_callback_, Run).Times(0);
 
   UrlRequest request;
   request.url = "http://example.com/fake.pdf";
   request.method = "FAKE";
-  loader_->Open(request, mock_callback_.Get());
+  loader_->Open(request, mock_open_callback_.Get());
 
   EXPECT_TRUE(fake_client_.saved_options().grant_universal_access);
   EXPECT_EQ(GURL("http://example.com/fake.pdf"), GURL(saved_request_.Url()));
@@ -200,24 +202,24 @@ TEST_F(UrlLoaderTest, Open) {
 
 TEST_F(UrlLoaderTest, OpenWithInvalidatedClientWeakPtr) {
   EXPECT_CALL(*mock_url_loader_, LoadAsynchronously).Times(0);
-  EXPECT_CALL(mock_callback_, Run(Result::kErrorFailed));
+  EXPECT_CALL(mock_open_callback_, Run(Result::kErrorFailed));
 
   fake_client_.InvalidateWeakPtrs();
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
 }
 
 TEST_F(UrlLoaderTest, OpenWithInvalidatedClient) {
   EXPECT_CALL(*mock_url_loader_, LoadAsynchronously).Times(0);
-  EXPECT_CALL(mock_callback_, Run(Result::kErrorFailed));
+  EXPECT_CALL(mock_open_callback_, Run(Result::kErrorFailed));
 
   fake_client_.Invalidate();
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
 }
 
 TEST_F(UrlLoaderTest, OpenWithRelativeUrl) {
   UrlRequest request;
   request.url = "relative.pdf";
-  loader_->Open(request, mock_callback_.Get());
+  loader_->Open(request, mock_open_callback_.Get());
 
   EXPECT_EQ(GURL(kDocumentUrl).Resolve("relative.pdf"),
             GURL(saved_request_.Url()));
@@ -232,7 +234,7 @@ TEST_F(UrlLoaderTest, OpenWithHeaders) {
           "Non-ASCII-Value: 🙃",
       },
       "\n");
-  loader_->Open(request, mock_callback_.Get());
+  loader_->Open(request, mock_open_callback_.Get());
 
   EXPECT_EQ(3u, GetRequestHeaderCount(saved_request_));
   EXPECT_EQ("123", saved_request_.HttpHeaderField("Content-Length").Utf8());
@@ -245,7 +247,7 @@ TEST_F(UrlLoaderTest, OpenWithBody) {
   static constexpr char kBodyData[] = "fake body";
   UrlRequest request;
   request.body = kBodyData;
-  loader_->Open(request, mock_callback_.Get());
+  loader_->Open(request, mock_open_callback_.Get());
 
   blink::WebHTTPBody request_body = saved_request_.HttpBody();
   EXPECT_EQ(1u, request_body.ElementCount());
@@ -266,14 +268,14 @@ TEST_F(UrlLoaderTest, OpenWithBody) {
 TEST_F(UrlLoaderTest, OpenWithCustomReferrerUrl) {
   UrlRequest request;
   request.custom_referrer_url = "http://example.com/referrer";
-  loader_->Open(request, mock_callback_.Get());
+  loader_->Open(request, mock_open_callback_.Get());
 
   EXPECT_EQ("http://example.com/referrer",
             saved_request_.ReferrerString().Utf8());
 }
 
 TEST_F(UrlLoaderTest, WillFollowRedirect) {
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
 
   EXPECT_TRUE(loader_->WillFollowRedirect(GURL("http://example.com/login"),
                                           blink::WebURLResponse()));
@@ -282,15 +284,15 @@ TEST_F(UrlLoaderTest, WillFollowRedirect) {
 TEST_F(UrlLoaderTest, WillFollowRedirectWhileIgnoringRedirects) {
   UrlRequest request;
   request.ignore_redirects = true;
-  loader_->Open(request, mock_callback_.Get());
+  loader_->Open(request, mock_open_callback_.Get());
 
   EXPECT_FALSE(loader_->WillFollowRedirect(GURL("http://example.com/login"),
                                            blink::WebURLResponse()));
 }
 
 TEST_F(UrlLoaderTest, DidReceiveResponse) {
-  loader_->Open(UrlRequest(), mock_callback_.Get());
-  EXPECT_CALL(mock_callback_, Run(Result::kSuccess));
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
+  EXPECT_CALL(mock_open_callback_, Run(Result::kSuccess));
 
   blink::WebURLResponse response;
   response.SetHttpStatusCode(204);
@@ -301,7 +303,7 @@ TEST_F(UrlLoaderTest, DidReceiveResponse) {
 }
 
 TEST_F(UrlLoaderTest, DidReceiveResponseWithHeaders) {
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
 
   blink::WebURLResponse response;
   response.AddHttpHeaderField("Content-Length", "123");
@@ -321,10 +323,10 @@ TEST_F(UrlLoaderTest, DidReceiveResponseWithHeaders) {
 
 TEST_F(UrlLoaderTest, DidReceiveData) {
   char buffer[kFakeData.size()] = {};
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
-  EXPECT_CALL(mock_callback_, Run(kFakeData.size()));
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
+  EXPECT_CALL(mock_read_callback_, Run(kFakeData.size()));
 
   loader_->DidReceiveData(kFakeData);
 
@@ -333,10 +335,10 @@ TEST_F(UrlLoaderTest, DidReceiveData) {
 
 TEST_F(UrlLoaderTest, DidReceiveDataWithZeroLength) {
   char buffer[kFakeData.size()] = {};
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
-  EXPECT_CALL(mock_callback_, Run).Times(0);
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
+  EXPECT_CALL(mock_read_callback_, Run).Times(0);
 
   loader_->DidReceiveData(kFakeData.first(0u));
 
@@ -355,11 +357,11 @@ TEST_F(UrlLoaderTest, DidReceiveDataCrossUpperThreshold) {
   StartLoadWithThresholds(/*lower=*/2, /*upper=*/4);
 
   char read_buffer[1];
-  loader_->ReadResponseBody(read_buffer, mock_callback_.Get());
+  loader_->ReadResponseBody(read_buffer, mock_read_callback_.Get());
   {
     InSequence defer_before_read_callback;
     EXPECT_CALL(*mock_url_loader_, SetDefersLoading(true));
-    EXPECT_CALL(mock_callback_, Run);
+    EXPECT_CALL(mock_read_callback_, Run);
   }
 
   char buffer[4] = {};
@@ -377,119 +379,119 @@ TEST_F(UrlLoaderTest, DidReceiveDataAboveUpperThreshold) {
 }
 
 TEST_F(UrlLoaderTest, ReadResponseBody) {
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
   loader_->DidReceiveData(kFakeData);
-  EXPECT_CALL(mock_callback_, Run(kFakeData.size()));
+  EXPECT_CALL(mock_read_callback_, Run(kFakeData.size()));
 
   char buffer[kFakeData.size()] = {};
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
 
   EXPECT_THAT(buffer, ElementsAreArray(kFakeData));
 
   // Verify no more data returned on next call.
-  EXPECT_CALL(mock_callback_, Run).Times(0);
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+  EXPECT_CALL(mock_read_callback_, Run).Times(0);
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
 }
 
 TEST_F(UrlLoaderTest, ReadResponseBodyWithoutData) {
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
-  EXPECT_CALL(mock_callback_, Run).Times(0);
+  EXPECT_CALL(mock_read_callback_, Run).Times(0);
 
   char buffer[kFakeData.size()] = {};
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
 
   EXPECT_THAT(buffer, Each(0));
 }
 
 TEST_F(UrlLoaderTest, ReadResponseBodyWithEmptyBuffer) {
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
-  EXPECT_CALL(mock_callback_, Run(Result::kErrorBadArgument));
+  EXPECT_CALL(mock_read_callback_, Run(Result::kErrorBadArgument));
 
-  loader_->ReadResponseBody(base::span<char>(), mock_callback_.Get());
+  loader_->ReadResponseBody(base::span<char>(), mock_read_callback_.Get());
 }
 
 TEST_F(UrlLoaderTest, ReadResponseBodyWithSmallerBuffer) {
   static constexpr size_t kTailSize = 1;
   static constexpr size_t kBufferSize = kFakeData.size() - kTailSize;
 
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
   loader_->DidReceiveData(kFakeData);
-  EXPECT_CALL(mock_callback_, Run(kBufferSize));
+  EXPECT_CALL(mock_read_callback_, Run(kBufferSize));
 
   char buffer[kBufferSize] = {};
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
 
   EXPECT_THAT(buffer, ElementsAreArray(kFakeData.first(kBufferSize)));
 
   // Verify remaining data returned on next call.
   char tail_buffer[kTailSize];
-  EXPECT_CALL(mock_callback_, Run(kTailSize));
-  loader_->ReadResponseBody(tail_buffer, mock_callback_.Get());
+  EXPECT_CALL(mock_read_callback_, Run(kTailSize));
+  loader_->ReadResponseBody(tail_buffer, mock_read_callback_.Get());
   EXPECT_THAT(tail_buffer, ElementsAreArray(kFakeData.subspan(kBufferSize)));
 }
 
 TEST_F(UrlLoaderTest, ReadResponseBodyWithBiggerBuffer) {
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
   loader_->DidReceiveData(kFakeData);
-  EXPECT_CALL(mock_callback_, Run(kFakeData.size()));
+  EXPECT_CALL(mock_read_callback_, Run(kFakeData.size()));
 
   char buffer[kFakeData.size() + 1] = {};
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
 
   base::span<char> buffer_span = buffer;
   EXPECT_THAT(buffer_span.first(kFakeData.size()), ElementsAreArray(kFakeData));
   EXPECT_THAT(buffer_span.subspan(kFakeData.size()), Each(0));
 
   // Verify no more data returned on next call.
-  EXPECT_CALL(mock_callback_, Run).Times(0);
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+  EXPECT_CALL(mock_read_callback_, Run).Times(0);
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
 }
 
 TEST_F(UrlLoaderTest, ReadResponseBodyWhileLoadComplete) {
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
   loader_->DidReceiveData(kFakeData);
   loader_->DidFinishLoading();
-  EXPECT_CALL(mock_callback_, Run(kFakeData.size()));
+  EXPECT_CALL(mock_read_callback_, Run(kFakeData.size()));
 
   char buffer[kFakeData.size()] = {};
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
 
   EXPECT_THAT(buffer, ElementsAreArray(kFakeData));
 
   // Verify no more data returned on next call.
   char tail_buffer[kFakeData.size()] = {};
-  EXPECT_CALL(mock_callback_, Run(0));
-  loader_->ReadResponseBody(tail_buffer, mock_callback_.Get());
+  EXPECT_CALL(mock_read_callback_, Run(0));
+  loader_->ReadResponseBody(tail_buffer, mock_read_callback_.Get());
   EXPECT_THAT(tail_buffer, Each(0));
 }
 
 TEST_F(UrlLoaderTest, ReadResponseBodyWhileLoadCompleteWithoutData) {
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
   loader_->DidFinishLoading();
-  EXPECT_CALL(mock_callback_, Run(0));
+  EXPECT_CALL(mock_read_callback_, Run(0));
 
   char buffer[kFakeData.size()] = {};
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
 
   EXPECT_THAT(buffer, Each(0));
 }
 
 TEST_F(UrlLoaderTest, ReadResponseBodyWhileLoadCompleteWithError) {
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
   loader_->DidReceiveData(kFakeData);
   loader_->DidFail(MakeWebURLError(net::ERR_FAILED));
-  EXPECT_CALL(mock_callback_, Run(Result::kErrorFailed));
+  EXPECT_CALL(mock_read_callback_, Run(Result::kErrorFailed));
 
   char buffer[kFakeData.size()] = {};
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
 
   EXPECT_THAT(buffer, Each(0));
 }
@@ -502,7 +504,7 @@ TEST_F(UrlLoaderTest, ReadResponseBodyAboveLowerThreshold) {
   EXPECT_CALL(*mock_url_loader_, SetDefersLoading).Times(0);
 
   char buffer[2] = {};
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
 }
 
 TEST_F(UrlLoaderTest, ReadResponseBodyCrossLowerThreshold) {
@@ -513,11 +515,11 @@ TEST_F(UrlLoaderTest, ReadResponseBodyCrossLowerThreshold) {
   {
     InSequence resume_before_read_callback;
     EXPECT_CALL(*mock_url_loader_, SetDefersLoading(false));
-    EXPECT_CALL(mock_callback_, Run);
+    EXPECT_CALL(mock_read_callback_, Run);
   }
 
   char buffer[3] = {};
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
 }
 
 TEST_F(UrlLoaderTest, ReadResponseBodyBelowLowerThreshold) {
@@ -527,55 +529,55 @@ TEST_F(UrlLoaderTest, ReadResponseBodyBelowLowerThreshold) {
   loader_->DidReceiveData(write_buffer);
 
   char buffer[3] = {};
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
   EXPECT_CALL(*mock_url_loader_, SetDefersLoading).Times(0);
 
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
 }
 
 TEST_F(UrlLoaderTest, DidFinishLoading) {
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
-  EXPECT_CALL(mock_callback_, Run).Times(0);
+  EXPECT_CALL(mock_open_callback_, Run).Times(0);
 
   loader_->DidFinishLoading();
 }
 
 TEST_F(UrlLoaderTest, DidFinishLoadingWithPendingCallback) {
   char buffer[1];
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
-  EXPECT_CALL(mock_callback_, Run(0));  // Result represents read bytes.
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
+  EXPECT_CALL(mock_read_callback_, Run(0));  // Result represents read bytes.
 
   loader_->DidFinishLoading();
 }
 
 TEST_F(UrlLoaderTest, DidFailWhileOpening) {
-  loader_->Open(UrlRequest(), mock_callback_.Get());
-  EXPECT_CALL(mock_callback_, Run(Result::kErrorFailed));
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
+  EXPECT_CALL(mock_open_callback_, Run(Result::kErrorFailed));
 
   loader_->DidFail(MakeWebURLError(net::ERR_FAILED));
 }
 
 TEST_F(UrlLoaderTest, DidFailWhileStreamingData) {
   char buffer[1];
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
-  EXPECT_CALL(mock_callback_, Run(Result::kErrorFailed));
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
+  EXPECT_CALL(mock_read_callback_, Run(Result::kErrorFailed));
 
   loader_->DidFail(MakeWebURLError(net::ERR_FAILED));
 }
 
 TEST_F(UrlLoaderTest, DidFailWithErrorAccessDenied) {
-  int32_t result = DidFailWithError(MakeWebURLError(net::ERR_ACCESS_DENIED));
+  Result result = DidFailWithError(MakeWebURLError(net::ERR_ACCESS_DENIED));
 
   EXPECT_EQ(Result::kErrorNoAccess, result);
 }
 
 TEST_F(UrlLoaderTest, DidFailWithErrorNetworkAccessDenied) {
-  int32_t result =
+  Result result =
       DidFailWithError(MakeWebURLError(net::ERR_NETWORK_ACCESS_DENIED));
 
   EXPECT_EQ(Result::kErrorNoAccess, result);
@@ -587,55 +589,55 @@ TEST_F(UrlLoaderTest, DidFailWithWebSecurityViolationError) {
       blink::WebURLError::HasCopyInCache::kFalse, GURL());
   ASSERT_TRUE(error.is_web_security_violation());
 
-  int32_t result = DidFailWithError(error);
+  Result result = DidFailWithError(error);
 
   EXPECT_EQ(Result::kErrorNoAccess, result);
 }
 
 TEST_F(UrlLoaderTest, CloseWhileWaitingToOpen) {
-  EXPECT_CALL(mock_callback_, Run).Times(0);
+  EXPECT_CALL(mock_open_callback_, Run).Times(0);
 
   loader_->Close();
 }
 
 TEST_F(UrlLoaderTest, CloseWhileOpening) {
-  loader_->Open(UrlRequest(), mock_callback_.Get());
-  EXPECT_CALL(mock_callback_, Run(Result::kErrorAborted));
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
+  EXPECT_CALL(mock_open_callback_, Run(Result::kErrorAborted));
 
   loader_->Close();
 }
 
 TEST_F(UrlLoaderTest, CloseWhileStreamingData) {
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
-  EXPECT_CALL(mock_callback_, Run).Times(0);
+  EXPECT_CALL(mock_open_callback_, Run).Times(0);
 
   loader_->Close();
 }
 
 TEST_F(UrlLoaderTest, CloseWhileStreamingDataWithPendingCallback) {
   char buffer[1];
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
-  loader_->ReadResponseBody(buffer, mock_callback_.Get());
-  EXPECT_CALL(mock_callback_, Run(Result::kErrorAborted));
+  loader_->ReadResponseBody(buffer, mock_read_callback_.Get());
+  EXPECT_CALL(mock_read_callback_, Run(Result::kErrorAborted));
 
   loader_->Close();
 }
 
 TEST_F(UrlLoaderTest, CloseWhileLoadComplete) {
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
   loader_->DidFinishLoading();
-  EXPECT_CALL(mock_callback_, Run).Times(0);
+  EXPECT_CALL(mock_open_callback_, Run).Times(0);
 
   loader_->Close();
 }
 
 TEST_F(UrlLoaderTest, CloseAgain) {
-  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->Open(UrlRequest(), mock_open_callback_.Get());
   loader_->Close();
-  EXPECT_CALL(mock_callback_, Run).Times(0);
+  EXPECT_CALL(mock_open_callback_, Run).Times(0);
 
   loader_->Close();
 }

pdf/loader/url_loader_wrapper_impl.cc

@@ -246,7 +246,7 @@ void URLLoaderWrapperImpl::ParseHeaders(const std::string& response_headers) {
 }
 
 void URLLoaderWrapperImpl::DidOpen(base::OnceCallback<void(bool)> callback,
-                                   int32_t result) {
+                                   Result result) {
   SetHeadersFromLoader();
   std::move(callback).Run(result == Result::kSuccess);
 }

pdf/loader/url_loader_wrapper_impl.h

@@ -15,6 +15,7 @@
 #include "base/memory/raw_span.h"
 #include "base/memory/weak_ptr.h"
 #include "base/timer/timer.h"
+#include "pdf/loader/result_codes.h"
 #include "pdf/loader/url_loader_wrapper.h"
 #include "ui/gfx/range/range.h"
 
@@ -50,7 +51,7 @@ class URLLoaderWrapperImpl : public URLLoaderWrapper {
  private:
   void SetHeadersFromLoader();
   void ParseHeaders(const std::string& response_headers);
-  void DidOpen(base::OnceCallback<void(bool)> callback, int32_t result);
+  void DidOpen(base::OnceCallback<void(bool)> callback, Result result);
   void DidRead(base::OnceCallback<void(int)> callback, int32_t result);
 
   void ReadResponseBodyImpl(base::OnceCallback<void(int)> callback);

pdf/pdf_view_web_plugin.cc

@@ -513,13 +513,13 @@ void PdfViewWebPlugin::SendSetSmoothScrolling() {
 }
 
 void PdfViewWebPlugin::DidOpen(std::unique_ptr<UrlLoader> loader,
-                               int32_t result) {
-  if (result == kSuccess) {
+                               Result result) {
+  if (result == Result::kSuccess) {
     if (!engine_->HandleDocumentLoad(std::move(loader), url_)) {
       document_load_state_ = DocumentLoadState::kLoading;
       DocumentLoadFailed();
     }
-  } else if (result != kErrorAborted) {
+  } else if (result != Result::kErrorAborted) {
     DocumentLoadFailed();
   }
 }
@@ -1202,9 +1202,9 @@ void PdfViewWebPlugin::SubmitForm(const std::string& url,
                                              weak_factory_.GetWeakPtr()));
 }
 
-void PdfViewWebPlugin::DidFormOpen(int32_t result) {
+void PdfViewWebPlugin::DidFormOpen(Result result) {
   // TODO(crbug.com/41317525): Process response.
-  LOG_IF(ERROR, result != kSuccess) << "DidFormOpen failed: " << result;
+  LOG_IF(ERROR, result != Result::kSuccess) << "DidFormOpen failed: " << result;
   form_loader_.reset();
 }
 
@@ -2678,8 +2678,8 @@ void PdfViewWebPlugin::LoadAvailablePreviewPage() {
 }
 
 void PdfViewWebPlugin::DidOpenPreview(std::unique_ptr<UrlLoader> loader,
-                                      int32_t result) {
-  DCHECK_EQ(result, kSuccess);
+                                      Result result) {
+  DCHECK_EQ(result, Result::kSuccess);
 
   // `preview_engine_` holds a `raw_ptr` to `preview_client_`.
   // We need to explicitly destroy it before clobbering

pdf/pdf_view_web_plugin.h

@@ -24,6 +24,7 @@
 #include "mojo/public/cpp/bindings/receiver.h"
 #include "pdf/accessibility_structs.h"
 #include "pdf/buildflags.h"
+#include "pdf/loader/result_codes.h"
 #include "pdf/loader/url_loader.h"
 #include "pdf/mojom/pdf.mojom.h"
 #include "pdf/paint_manager.h"
@@ -487,7 +488,7 @@ class PdfViewWebPlugin final : public PDFiumEngineClient,
   // load the URL, and `result` is the result code for the load.
   using LoadUrlCallback =
       base::OnceCallback<void(std::unique_ptr<UrlLoader> loader,
-                              int32_t result)>;
+                              Result result)>;
 
   enum class AccessibilityState {
     kOff = 0,  // Off.
@@ -522,7 +523,7 @@ class PdfViewWebPlugin final : public PDFiumEngineClient,
   void SendSetSmoothScrolling();
 
   // Handles `LoadUrl()` result for the main document.
-  void DidOpen(std::unique_ptr<UrlLoader> loader, int32_t result);
+  void DidOpen(std::unique_ptr<UrlLoader> loader, Result result);
 
   // Updates the scroll position, which is in CSS pixels relative to the
   // top-left corner.
@@ -532,7 +533,7 @@ class PdfViewWebPlugin final : public PDFiumEngineClient,
   void LoadUrl(std::string_view url, LoadUrlCallback callback);
 
   // Handles `Open()` result for `form_loader_`.
-  void DidFormOpen(int32_t result);
+  void DidFormOpen(Result result);
 
   // Sends start/stop loading notifications to the plugin's render frame.
   void DidStartLoading();
@@ -669,7 +670,7 @@ class PdfViewWebPlugin final : public PDFiumEngineClient,
   void LoadAvailablePreviewPage();
 
   // Handles `LoadUrl()` result for a preview page.
-  void DidOpenPreview(std::unique_ptr<UrlLoader> loader, int32_t result);
+  void DidOpenPreview(std::unique_ptr<UrlLoader> loader, Result result);
 
   // Continues loading the next preview page.
   void LoadNextPreviewPage();