[unseasoned-pdf] Replace PPAPI error codes with enum Result.

Provide a way to use well known PPAPI error codes, which are just ints,
without having to directly reference PPAPI.

Bug: 1128159
Change-Id: Ia2da772541b60c6e5869d07dc12d1e919614c2d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2978172
Reviewed-by: Daniel Hosseinian <dhoss@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#894845}

pdf/BUILD.gn

@@ -276,6 +276,7 @@ if (enable_pdf) {
       "ppapi_migration/input_event_conversions.h",
       "ppapi_migration/printing_conversions.cc",
       "ppapi_migration/printing_conversions.h",
+      "ppapi_migration/result_codes.h",
       "ppapi_migration/url_loader.cc",
       "ppapi_migration/url_loader.h",
       "ppapi_migration/value_conversions.cc",

pdf/document_loader_impl.cc

@@ -18,8 +18,8 @@
 #include "base/numerics/safe_math.h"
 #include "base/strings/string_util.h"
 #include "pdf/pdf_features.h"
+#include "pdf/ppapi_migration/result_codes.h"
 #include "pdf/url_loader_wrapper.h"
-#include "ppapi/c/pp_errors.h"
 #include "ui/gfx/range/range.h"
 
 namespace chrome_pdf {
@@ -260,9 +260,8 @@ void DocumentLoaderImpl::ContinueDownload() {
 }
 
 void DocumentLoaderImpl::DidOpenPartial(int32_t result) {
-  if (result != PP_OK) {
+  if (result != Result::kSuccess)
     return ReadComplete();
-  }
 
   if (!ResponseStatusSuccess(loader_.get()))
     return ReadComplete();

pdf/pdf_view_plugin_base.cc

@@ -46,6 +46,7 @@
 #include "pdf/pdf_features.h"
 #include "pdf/pdfium/pdfium_engine.h"
 #include "pdf/ppapi_migration/image.h"
+#include "pdf/ppapi_migration/result_codes.h"
 #include "pdf/ppapi_migration/url_loader.h"
 #include "pdf/ui/document_properties.h"
 #include "pdf/ui/file_name.h"
@@ -1545,7 +1546,7 @@ void PdfViewPluginBase::HistogramCustomCounts(const char* name,
 
 void PdfViewPluginBase::DidOpenPreview(std::unique_ptr<UrlLoader> loader,
                                        int32_t result) {
-  DCHECK_EQ(result, PP_OK);
+  DCHECK_EQ(result, Result::kSuccess);
   preview_client_ = std::make_unique<PreviewModeClient>(this);
   preview_engine_ = std::make_unique<PDFiumEngine>(
       preview_client_.get(), PDFiumFormFiller::ScriptOption::kNoJavaScript);

pdf/pdf_view_web_plugin.cc

@@ -37,8 +37,8 @@
 #include "pdf/post_message_receiver.h"
 #include "pdf/ppapi_migration/bitmap.h"
 #include "pdf/ppapi_migration/graphics.h"
+#include "pdf/ppapi_migration/result_codes.h"
 #include "pdf/ppapi_migration/url_loader.h"
-#include "ppapi/c/pp_errors.h"
 #include "printing/metafile_skia.h"
 #include "services/network/public/mojom/referrer_policy.mojom-shared.h"
 #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
@@ -619,7 +619,7 @@ std::unique_ptr<UrlLoader> PdfViewWebPlugin::CreateUrlLoaderInternal() {
 // Modeled on `OutOfProcessInstance::DidOpen()`.
 void PdfViewWebPlugin::DidOpen(std::unique_ptr<UrlLoader> loader,
                                int32_t result) {
-  if (result == PP_OK) {
+  if (result == Result::kSuccess) {
     if (!engine()->HandleDocumentLoad(std::move(loader)))
       DocumentLoadFailed();
   } else {

pdf/ppapi_migration/graphics.cc

@@ -17,7 +17,7 @@
 #include "pdf/ppapi_migration/callback.h"
 #include "pdf/ppapi_migration/geometry_conversions.h"
 #include "pdf/ppapi_migration/image.h"
-#include "ppapi/c/pp_errors.h"
+#include "pdf/ppapi_migration/result_codes.h"
 #include "ppapi/cpp/completion_callback.h"
 #include "ppapi/cpp/instance_handle.h"
 #include "ppapi/cpp/point.h"
@@ -53,7 +53,7 @@ bool PepperGraphics::Flush(ResultCallback callback) {
   // Should only happen if pp::Graphics2D::Flush() is called while a callback is
   // still pending, which should never happen if PaintManager is managing all
   // flushes.
-  DCHECK_EQ(PP_OK, result);
+  DCHECK_EQ(Result::kSuccess, result);
   pp_callback.Run(result);
   return false;
 }

pdf/ppapi_migration/ppapi_assert_matching_enums.cc

@@ -4,6 +4,8 @@
 
 #include "pdf/accessibility_structs.h"
 #include "pdf/content_restriction.h"
+#include "pdf/ppapi_migration/result_codes.h"
+#include "ppapi/c/pp_errors.h"
 #include "ppapi/c/private/ppb_pdf.h"
 #include "ppapi/c/private/ppp_pdf.h"
 
@@ -103,4 +105,10 @@ STATIC_ASSERT_ENUM(kContentRestrictionPaste, PP_CONTENT_RESTRICTION_PASTE);
 STATIC_ASSERT_ENUM(kContentRestrictionPrint, PP_CONTENT_RESTRICTION_PRINT);
 STATIC_ASSERT_ENUM(kContentRestrictionSave, PP_CONTENT_RESTRICTION_SAVE);
 
+STATIC_ASSERT_ENUM(Result::kSuccess, PP_OK);
+STATIC_ASSERT_ENUM(Result::kErrorFailed, PP_ERROR_FAILED);
+STATIC_ASSERT_ENUM(Result::kErrorAborted, PP_ERROR_ABORTED);
+STATIC_ASSERT_ENUM(Result::kErrorBadArgument, PP_ERROR_BADARGUMENT);
+STATIC_ASSERT_ENUM(Result::kErrorNoAccess, PP_ERROR_NOACCESS);
+
 }  // namespace chrome_pdf

pdf/ppapi_migration/result_codes.h

@@ -0,0 +1,32 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef PDF_PPAPI_MIGRATION_RESULT_CODES_H_
+#define PDF_PPAPI_MIGRATION_RESULT_CODES_H_
+
+namespace chrome_pdf {
+
+// TODO(crbug.com/702993): After migrating away from PPAPI, re-evaluate where
+// this enum should live, whether it should become an enum class, and what
+// values it should contain.
+enum Result {
+  // Must match `PP_OK`.
+  kSuccess = 0,
+
+  // Must match `PP_ERROR_FAILED`.
+  kErrorFailed = -2,
+
+  // Must match `PP_ERROR_ABORTED`.
+  kErrorAborted = -3,
+
+  // Must match `PP_ERROR_BADARGUMENT`.
+  kErrorBadArgument = -4,
+
+  // Must match `PP_ERROR_NOACCESS`.
+  kErrorNoAccess = -7,
+};
+
+}  // namespace chrome_pdf
+
+#endif  // PDF_PPAPI_MIGRATION_RESULT_CODES_H_

pdf/ppapi_migration/url_loader.cc

@@ -20,7 +20,7 @@
 #include "net/cookies/site_for_cookies.h"
 #include "net/http/http_util.h"
 #include "pdf/ppapi_migration/callback.h"
-#include "ppapi/c/pp_errors.h"
+#include "pdf/ppapi_migration/result_codes.h"
 #include "ppapi/c/trusted/ppb_url_loader_trusted.h"
 #include "ppapi/cpp/completion_callback.h"
 #include "ppapi/cpp/instance_handle.h"
@@ -103,7 +103,7 @@ void BlinkUrlLoader::Open(const UrlRequest& request, ResultCallback callback) {
   open_callback_ = std::move(callback);
 
   if (!client_ || !client_->IsValid()) {
-    AbortLoad(PP_ERROR_FAILED);
+    AbortLoad(Result::kErrorFailed);
     return;
   }
 
@@ -166,7 +166,7 @@ void BlinkUrlLoader::ReadResponseBody(base::span<char> buffer,
       << static_cast<int>(state_);
 
   if (buffer.empty()) {
-    std::move(callback).Run(PP_ERROR_BADARGUMENT);
+    std::move(callback).Run(Result::kErrorBadArgument);
     return;
   }
 
@@ -182,7 +182,7 @@ void BlinkUrlLoader::ReadResponseBody(base::span<char> buffer,
 // Modeled on `ppapi::proxy::URLLoadResource::Close()`.
 void BlinkUrlLoader::Close() {
   if (state_ != LoadingState::kLoadComplete)
-    AbortLoad(PP_ERROR_ABORTED);
+    AbortLoad(Result::kErrorAborted);
 }
 
 // Modeled on `content::PepperURLLoaderHost::WillFollowRedirect()`.
@@ -216,7 +216,7 @@ void BlinkUrlLoader::DidReceiveResponse(const blink::WebURLResponse& response) {
   response.VisitHttpHeaderFields(&headers_to_string);
 
   state_ = LoadingState::kStreamingData;
-  std::move(open_callback_).Run(PP_OK);
+  std::move(open_callback_).Run(Result::kSuccess);
 }
 
 void BlinkUrlLoader::DidDownloadData(uint64_t data_length) {
@@ -247,7 +247,7 @@ void BlinkUrlLoader::DidReceiveData(const char* data, int data_length) {
 void BlinkUrlLoader::DidFinishLoading() {
   DCHECK_EQ(state_, LoadingState::kStreamingData);
 
-  SetLoadComplete(PP_OK);
+  SetLoadComplete(Result::kSuccess);
   RunReadCallback();
 }
 
@@ -257,16 +257,16 @@ void BlinkUrlLoader::DidFail(const blink::WebURLError& error) {
          state_ == LoadingState::kStreamingData)
       << static_cast<int>(state_);
 
-  int32_t pp_error = PP_ERROR_FAILED;
+  int32_t pp_error = Result::kErrorFailed;
   switch (error.reason()) {
     case net::ERR_ACCESS_DENIED:
     case net::ERR_NETWORK_ACCESS_DENIED:
-      pp_error = PP_ERROR_NOACCESS;
+      pp_error = Result::kErrorNoAccess;
       break;
 
     default:
       if (error.is_web_security_violation())
-        pp_error = PP_ERROR_NOACCESS;
+        pp_error = Result::kErrorNoAccess;
       break;
   }
 
@@ -310,7 +310,8 @@ void BlinkUrlLoader::RunReadCallback() {
     DCHECK_EQ(state_, LoadingState::kLoadComplete);
     num_bytes = complete_result_;
     DCHECK_LE(num_bytes, 0);
-    static_assert(PP_OK == 0, "PP_OK should be equivalent to 0 bytes");
+    static_assert(Result::kSuccess == 0,
+                  "Result::kSuccess should be equivalent to 0 bytes");
   }
 
   client_buffer_ = {};

pdf/ppapi_migration/url_loader_unittest.cc

@@ -22,7 +22,7 @@
 #include "net/base/net_errors.h"
 #include "net/cookies/site_for_cookies.h"
 #include "pdf/ppapi_migration/callback.h"
-#include "ppapi/c/pp_errors.h"
+#include "pdf/ppapi_migration/result_codes.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom-shared.h"
@@ -219,7 +219,7 @@ TEST_F(BlinkUrlLoaderTest, Open) {
 
 TEST_F(BlinkUrlLoaderTest, OpenWithInvalidatedClientWeakPtr) {
   EXPECT_CALL(*mock_url_loader_, LoadAsynchronously).Times(0);
-  EXPECT_CALL(mock_callback_, Run(PP_ERROR_FAILED));
+  EXPECT_CALL(mock_callback_, Run(Result::kErrorFailed));
 
   fake_client_.InvalidateWeakPtrs();
   loader_->Open(UrlRequest(), mock_callback_.Get());
@@ -227,7 +227,7 @@ TEST_F(BlinkUrlLoaderTest, OpenWithInvalidatedClientWeakPtr) {
 
 TEST_F(BlinkUrlLoaderTest, OpenWithInvalidatedClient) {
   EXPECT_CALL(*mock_url_loader_, LoadAsynchronously).Times(0);
-  EXPECT_CALL(mock_callback_, Run(PP_ERROR_FAILED));
+  EXPECT_CALL(mock_callback_, Run(Result::kErrorFailed));
 
   fake_client_.Invalidate();
   loader_->Open(UrlRequest(), mock_callback_.Get());
@@ -308,7 +308,7 @@ TEST_F(BlinkUrlLoaderTest, WillFollowRedirectWhileIgnoringRedirects) {
 
 TEST_F(BlinkUrlLoaderTest, DidReceiveResponse) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
-  EXPECT_CALL(mock_callback_, Run(PP_OK));
+  EXPECT_CALL(mock_callback_, Run(Result::kSuccess));
 
   blink::WebURLResponse response;
   response.SetHttpStatusCode(204);
@@ -424,7 +424,7 @@ TEST_F(BlinkUrlLoaderTest, ReadResponseBodyWithoutData) {
 TEST_F(BlinkUrlLoaderTest, ReadResponseBodyWithEmptyBuffer) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
-  EXPECT_CALL(mock_callback_, Run(PP_ERROR_BADARGUMENT));
+  EXPECT_CALL(mock_callback_, Run(Result::kErrorBadArgument));
 
   loader_->ReadResponseBody(base::span<char>(), mock_callback_.Get());
 }
@@ -504,7 +504,7 @@ TEST_F(BlinkUrlLoaderTest, ReadResponseBodyWhileLoadCompleteWithError) {
   loader_->DidReceiveResponse(blink::WebURLResponse());
   loader_->DidReceiveData(kFakeData.data(), kFakeData.size());
   loader_->DidFail(MakeWebURLError(net::ERR_FAILED));
-  EXPECT_CALL(mock_callback_, Run(PP_ERROR_FAILED));
+  EXPECT_CALL(mock_callback_, Run(Result::kErrorFailed));
 
   char buffer[kFakeData.size()] = {};
   loader_->ReadResponseBody(buffer, mock_callback_.Get());
@@ -571,7 +571,7 @@ TEST_F(BlinkUrlLoaderTest, DidFinishLoadingWithPendingCallback) {
 
 TEST_F(BlinkUrlLoaderTest, DidFailWhileOpening) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
-  EXPECT_CALL(mock_callback_, Run(PP_ERROR_FAILED));
+  EXPECT_CALL(mock_callback_, Run(Result::kErrorFailed));
 
   loader_->DidFail(MakeWebURLError(net::ERR_FAILED));
 }
@@ -581,7 +581,7 @@ TEST_F(BlinkUrlLoaderTest, DidFailWhileStreamingData) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
   loader_->ReadResponseBody(buffer, mock_callback_.Get());
-  EXPECT_CALL(mock_callback_, Run(PP_ERROR_FAILED));
+  EXPECT_CALL(mock_callback_, Run(Result::kErrorFailed));
 
   loader_->DidFail(MakeWebURLError(net::ERR_FAILED));
 }
@@ -589,14 +589,14 @@ TEST_F(BlinkUrlLoaderTest, DidFailWhileStreamingData) {
 TEST_F(BlinkUrlLoaderTest, DidFailWithErrorAccessDenied) {
   int32_t result = DidFailWithError(MakeWebURLError(net::ERR_ACCESS_DENIED));
 
-  EXPECT_EQ(PP_ERROR_NOACCESS, result);
+  EXPECT_EQ(Result::kErrorNoAccess, result);
 }
 
 TEST_F(BlinkUrlLoaderTest, DidFailWithErrorNetworkAccessDenied) {
   int32_t result =
       DidFailWithError(MakeWebURLError(net::ERR_NETWORK_ACCESS_DENIED));
 
-  EXPECT_EQ(PP_ERROR_NOACCESS, result);
+  EXPECT_EQ(Result::kErrorNoAccess, result);
 }
 
 TEST_F(BlinkUrlLoaderTest, DidFailWithWebSecurityViolationError) {
@@ -606,7 +606,7 @@ TEST_F(BlinkUrlLoaderTest, DidFailWithWebSecurityViolationError) {
 
   int32_t result = DidFailWithError(error);
 
-  EXPECT_EQ(PP_ERROR_NOACCESS, result);
+  EXPECT_EQ(Result::kErrorNoAccess, result);
 }
 
 TEST_F(BlinkUrlLoaderTest, CloseWhileWaitingToOpen) {
@@ -617,7 +617,7 @@ TEST_F(BlinkUrlLoaderTest, CloseWhileWaitingToOpen) {
 
 TEST_F(BlinkUrlLoaderTest, CloseWhileOpening) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
-  EXPECT_CALL(mock_callback_, Run(PP_ERROR_ABORTED));
+  EXPECT_CALL(mock_callback_, Run(Result::kErrorAborted));
 
   loader_->Close();
 }
@@ -635,7 +635,7 @@ TEST_F(BlinkUrlLoaderTest, CloseWhileStreamingDataWithPendingCallback) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
   loader_->ReadResponseBody(buffer, mock_callback_.Get());
-  EXPECT_CALL(mock_callback_, Run(PP_ERROR_ABORTED));
+  EXPECT_CALL(mock_callback_, Run(Result::kErrorAborted));
 
   loader_->Close();
 }