Remove some WrapUnique() calls by returing unique_ptrs

Remove some WrapUnique() calls by returing unique_ptrs

Return these from underlying methods as appropriate.

Committed: https://pdfium.googlesource.com/pdfium/+/06104a8abc71ecd824d6a461b6f6f31c32fd2135

[Tom Sepez, 2016-11-21 22:03:21]

The CQ bit was checked by tsepez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 22:03:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-21 22:07:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 22:07:20]

Dry run: Try jobs failed on following builders:
  android on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/android/builds/...)

[Tom Sepez, 2016-11-21 22:11:14]

tsepez@chromium.org changed reviewers:
+ dsinclair@chromium.org

[Tom Sepez, 2016-11-21 22:11:14]

Dan, for review.

https://codereview.chromium.org/2520133002/diff/1/core/fpdfapi/page/cpdf_stre...
File core/fpdfapi/page/cpdf_streamcontentparser.h (right):

https://codereview.chromium.org/2520133002/diff/1/core/fpdfapi/page/cpdf_stre...
core/fpdfapi/page/cpdf_streamcontentparser.h:56: std::unique_ptr<CPDF_Object>
m_pObject;
note: made this into a struct rather than a union, as having a non-pod member
overlap other memory isn't going to fly.  Marginal waste of memory, but the nice
part is it makes it harder to misinterpret a sequence of bytes as a pointer if
one botches the type ...

[Tom Sepez, 2016-11-21 22:11:17]

The CQ bit was checked by tsepez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 22:11:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dsinclair, 2016-11-21 22:20:56]

https://codereview.chromium.org/2520133002/diff/20001/core/fpdfapi/page/cpdf_...
File core/fpdfapi/page/cpdf_streamparser.cpp (right):

https://codereview.chromium.org/2520133002/diff/20001/core/fpdfapi/page/cpdf_...
core/fpdfapi/page/cpdf_streamparser.cpp:334:
pdfium::MakeUnique<CPDF_Name>(m_pPool, name);
Missing return?

https://codereview.chromium.org/2520133002/diff/20001/core/fpdfapi/page/cpdf_...
core/fpdfapi/page/cpdf_streamparser.cpp:339:
pdfium::MakeUnique<CPDF_String>(m_pPool, str, false);
missing return?

https://codereview.chromium.org/2520133002/diff/20001/core/fpdfapi/page/cpdf_...
core/fpdfapi/page/cpdf_streamparser.cpp:344:
pdfium::MakeUnique<CPDF_String>(m_pPool, ReadHexString(), true);
ditto

https://codereview.chromium.org/2520133002/diff/20001/core/fpdfapi/page/cpdf_...
core/fpdfapi/page/cpdf_streamparser.cpp:388:
pdfium::MakeUnique<CPDF_Boolean>(false);
"

https://codereview.chromium.org/2520133002/diff/20001/core/fpdfapi/page/cpdf_...
core/fpdfapi/page/cpdf_streamparser.cpp:392:
pdfium::MakeUnique<CPDF_Boolean>(true);
"

https://codereview.chromium.org/2520133002/diff/20001/core/fpdfapi/page/cpdf_...
core/fpdfapi/page/cpdf_streamparser.cpp:395: pdfium::MakeUnique<CPDF_Null>();
"

[Tom Sepez, 2016-11-21 22:26:02]

The CQ bit was checked by tsepez@chromium.org to run a CQ dry run

[Tom Sepez, 2016-11-21 22:26:07]

https://codereview.chromium.org/2520133002/diff/20001/core/fpdfapi/page/cpdf_...
File core/fpdfapi/page/cpdf_streamparser.cpp (right):

https://codereview.chromium.org/2520133002/diff/20001/core/fpdfapi/page/cpdf_...
core/fpdfapi/page/cpdf_streamparser.cpp:334:
pdfium::MakeUnique<CPDF_Name>(m_pPool, name);
On 2016/11/21 22:20:56, dsinclair wrote:
> Missing return?
Argh.  thanks.

[commit-bot: I haz the power, 2016-11-21 22:26:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dsinclair, 2016-11-21 22:51:00]

On 2016/11/21 22:26:09, commit-bot: I haz the power wrote:
> Dry run: CQ is trying da patch. Follow status at
>  
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

Bots are unhappy:

error: [chromium-style] Complex class/struct needs an explicit out-of-line
constructor.
  struct ContentParam {

[commit-bot: I haz the power, 2016-11-21 22:51:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 22:51:05]

Dry run: Try jobs failed on following builders:
  linux_xfa_asan_lsan on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/linux_xfa_asan_...)

[Tom Sepez, 2016-11-21 22:57:55]

The CQ bit was checked by tsepez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 22:58:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tom Sepez, 2016-11-21 23:00:59]

e unhappy:
> 
> error: [chromium-style] Complex class/struct needs an explicit out-of-line
> constructor.
>   struct ContentParam {
Done.

[dsinclair, 2016-11-21 23:02:36]

lgtm

[Tom Sepez, 2016-11-21 23:03:29]

The CQ bit was unchecked by tsepez@chromium.org

[Tom Sepez, 2016-11-21 23:03:34]

The CQ bit was checked by tsepez@chromium.org

[commit-bot: I haz the power, 2016-11-21 23:03:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-21 23:41:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 23:41:08]

Try jobs failed on following builders:
  win_xfa_clang_32 on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/win_xfa_clang_3...)

[Tom Sepez, 2016-11-21 23:53:25]

The CQ bit was checked by tsepez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 23:53:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tom Sepez, 2016-11-21 23:54:42]

The CQ bit was unchecked by tsepez@chromium.org

[Tom Sepez, 2016-11-21 23:54:46]

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2016-11-21 23:54:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
dsinclair@chromium.org
Link to the patchset: https://codereview.chromium.org/2520133002/#ps80001
(title: "Fix win build")

[commit-bot: I haz the power, 2016-11-21 23:54:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tom Sepez, 2016-11-21 23:56:40]

Description was changed from

==========
Remove some WrapUnique() calls by returing unique_ptrs

Return these from underlying methods as appropriate.
==========

to

==========
Remove some WrapUnique() calls by returing unique_ptrs

Return these from underlying methods as appropriate.
==========

[Tom Sepez, 2016-11-21 23:56:40]

tsepez@chromium.org changed reviewers:
+ thestig@chromium.org

[commit-bot: I haz the power, 2016-11-22 00:08:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-22 00:08:22]

Failed to apply patch for core/fpdfapi/page/cpdf_image.cpp:
While running git apply --index -p1;
  error: patch failed: core/fpdfapi/page/cpdf_image.cpp:150
  error: core/fpdfapi/page/cpdf_image.cpp: patch does not apply

Patch:       core/fpdfapi/page/cpdf_image.cpp
Index: core/fpdfapi/page/cpdf_image.cpp
diff --git a/core/fpdfapi/page/cpdf_image.cpp b/core/fpdfapi/page/cpdf_image.cpp
index
7513403918d4898c727851dce6aac4875a08c06c..f0ef7c99c706c40267025dd5819d6e828a601513
100644
--- a/core/fpdfapi/page/cpdf_image.cpp
+++ b/core/fpdfapi/page/cpdf_image.cpp
@@ -26,6 +26,7 @@
 #include "core/fpdfapi/render/render_int.h"
 #include "core/fxcodec/fx_codec.h"
 #include "core/fxge/fx_dib.h"
+#include "third_party/base/numerics/safe_conversions.h"
 #include "third_party/base/ptr_util.h"
 
 CPDF_Image::CPDF_Image(CPDF_Document* pDoc) : m_pDocument(pDoc) {}
@@ -86,7 +87,8 @@ void CPDF_Image::ConvertStreamToIndirectObject() {
   m_pDocument->AddIndirectObject(std::move(m_pOwnedStream));
 }
 
-CPDF_Dictionary* CPDF_Image::InitJPEG(uint8_t* pData, uint32_t size) {
+std::unique_ptr<CPDF_Dictionary> CPDF_Image::InitJPEG(uint8_t* pData,
+                                                      uint32_t size) {
   int32_t width;
   int32_t height;
   int32_t num_comps;
@@ -97,8 +99,8 @@ CPDF_Dictionary* CPDF_Image::InitJPEG(uint8_t* pData, uint32_t
size) {
     return nullptr;
   }
 
-  CPDF_Dictionary* pDict =
-      new CPDF_Dictionary(m_pDocument->GetByteStringPool());
+  auto pDict =
+      pdfium::MakeUnique<CPDF_Dictionary>(m_pDocument->GetByteStringPool());
   pDict->SetNewFor<CPDF_Name>("Type", "XObject");
   pDict->SetNewFor<CPDF_Name>("Subtype", "Image");
   pDict->SetNewFor<CPDF_Number>("Width", width);
@@ -134,14 +136,17 @@ CPDF_Dictionary* CPDF_Image::InitJPEG(uint8_t* pData,
uint32_t size) {
 }
 
 void CPDF_Image::SetJpegImage(IFX_SeekableReadStream* pFile) {
-  uint32_t size = (uint32_t)pFile->GetSize();
+  uint32_t size = pdfium::base::checked_cast<uint32_t>(pFile->GetSize());
   if (!size)
     return;
 
   uint32_t dwEstimateSize = std::min(size, 8192U);
   std::vector<uint8_t> data(dwEstimateSize);
-  pFile->ReadBlock(data.data(), 0, dwEstimateSize);
-  CPDF_Dictionary* pDict = InitJPEG(data.data(), dwEstimateSize);
+  if (!pFile->ReadBlock(data.data(), 0, dwEstimateSize))
+    return;
+
+  std::unique_ptr<CPDF_Dictionary> pDict =
+      InitJPEG(data.data(), dwEstimateSize);
   if (!pDict && size > dwEstimateSize) {
     data.resize(size);
     pFile->ReadBlock(data.data(), 0, size);
@@ -150,7 +155,7 @@ void CPDF_Image::SetJpegImage(IFX_SeekableReadStream* pFile)
{
   if (!pDict)
     return;
 
-  m_pStream->InitStreamFromFile(pFile, pdfium::WrapUnique(pDict));
+  m_pStream->InitStreamFromFile(pFile, std::move(pDict));
 }
 
 void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap, int32_t iCompress) {

[Tom Sepez, 2016-11-22 00:10:03]

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2016-11-22 00:10:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
dsinclair@chromium.org
Link to the patchset: https://codereview.chromium.org/2520133002/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2016-11-22 00:10:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-22 00:22:10]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1479773403637960,
"parent_rev": "dc7ec035a6962b788e75f4beaa4cf8fbddd83d39", "commit_rev":
"06104a8abc71ecd824d6a461b6f6f31c32fd2135"}

[commit-bot: I haz the power, 2016-11-22 00:22:14]

Description was changed from

==========
Remove some WrapUnique() calls by returing unique_ptrs

Return these from underlying methods as appropriate.
==========

to

==========
Remove some WrapUnique() calls by returing unique_ptrs

Return these from underlying methods as appropriate.

Committed:
https://pdfium.googlesource.com/pdfium/+/06104a8abc71ecd824d6a461b6f6f31c32fd...
==========

[commit-bot: I haz the power, 2016-11-22 00:22:15]

Committed patchset #6 (id:100001) as
https://pdfium.googlesource.com/pdfium/+/06104a8abc71ecd824d6a461b6f6f31c32fd...