Remove some PlatformFile uses from zlib

Remove some PlatformFile uses from zlib

BUG=322664
R=hshi@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=253727

[rvargas (doing something else), 2014-02-18 16:52:38]

PTAL

[hshi1, 2014-02-19 01:26:36]

lgtm

[rvargas (doing something else), 2014-02-19 14:55:02]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2014-02-19 15:08:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/166573007/1

[commit-bot: I haz the power, 2014-02-19 15:36:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/166573007/1

[commit-bot: I haz the power, 2014-02-19 15:45:59]

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

[commit-bot: I haz the power, 2014-02-19 15:45:59]

Try jobs failed on following builders: linux_chromium_rel

[rvargas (doing something else), 2014-02-19 20:40:55]

+haven

[Drew Haven, 2014-02-20 18:34:07]

Wow, we've had an auto-closing File class all along and for some reason I was
still using PlatformFiles?  I've been complaining to people about this without
knowing it was already fixed.

Is this part of a larger push to get rid of direct PlatformFile usage?

https://codereview.chromium.org/166573007/diff/380001/chrome/browser/extensio...
File chrome/browser/extensions/api/image_writer_private/operation_linux.cc
(right):

https://codereview.chromium.org/166573007/diff/380001/chrome/browser/extensio...
chrome/browser/extensions/api/image_writer_private/operation_linux.cc:6:
#include "base/files/file.h"
Is this import dangling from elsewhere?  I don't see any other changes to files
in this API.

[Drew Haven, 2014-02-20 18:38:13]

https://codereview.chromium.org/166573007/diff/380001/third_party/zlib/google...
File third_party/zlib/google/zip_reader.cc (right):

https://codereview.chromium.org/166573007/diff/380001/third_party/zlib/google...
third_party/zlib/google/zip_reader.cc:404: Passed(output_file.Pass()),
Where does this Pass come from?  output_file is not a scoped_ptr and I don't see
the Pass method on base::File.  (Forgive me, I'm just trying to understand
what's going on here as I'm not that great with C++.)

[hshi1, 2014-02-20 18:39:00]

On 2014/02/20 18:38:13, Drew Haven wrote:
>
https://codereview.chromium.org/166573007/diff/380001/third_party/zlib/google...
> File third_party/zlib/google/zip_reader.cc (right):
> 
>
https://codereview.chromium.org/166573007/diff/380001/third_party/zlib/google...
> third_party/zlib/google/zip_reader.cc:404: Passed(output_file.Pass()),
> Where does this Pass come from?  output_file is not a scoped_ptr and I don't
see
> the Pass method on base::File.  (Forgive me, I'm just trying to understand
> what's going on here as I'm not that great with C++.)

The Pass comes from the macro MOVE_ONLY_TYPE_FOR_CPP_03 (see base/move.h)

[hshi1, 2014-02-20 18:53:13]

https://codereview.chromium.org/166573007/diff/380001/third_party/zlib/google...
File third_party/zlib/google/zip_reader.cc (right):

https://codereview.chromium.org/166573007/diff/380001/third_party/zlib/google...
third_party/zlib/google/zip_reader.cc:404: Passed(output_file.Pass()),
On 2014/02/20 18:38:14, Drew Haven wrote:
> Where does this Pass come from?  output_file is not a scoped_ptr and I don't
see
> the Pass method on base::File.  (Forgive me, I'm just trying to understand
> what's going on here as I'm not that great with C++.)

Interestingly, scoped_ptr does not explicitly define the Pass() method either.
It is also using the MOVE_ONLY_TYPE_FOR_CPP_03 macro in base/move.h.

[rvargas (doing something else), 2014-02-20 19:38:03]

Yes, PlatformFile is currently deprecated and it should not be used in new code.
I'm removing current users.

https://codereview.chromium.org/166573007/diff/380001/chrome/browser/extensio...
File chrome/browser/extensions/api/image_writer_private/operation_linux.cc
(right):

https://codereview.chromium.org/166573007/diff/380001/chrome/browser/extensio...
chrome/browser/extensions/api/image_writer_private/operation_linux.cc:6:
#include "base/files/file.h"
On 2014/02/20 18:34:08, Drew Haven wrote:
> Is this import dangling from elsewhere?  I don't see any other changes to
files
> in this API.

You added PlatformFile to this file without adding an include. It worked for you
because third_party/zlib/google/zip_reader.h is included from
src/chrome/browser/extensions/api/image_writer_private/operation.h, but I'm
removing PlatformFile from zlib.

[Drew Haven, 2014-02-20 20:30:22]

lgtm

Is there a way that we could add something like upload warnings when code
directly references deprecated code?  As someone who isn't a full Chrome
developer it is hard to keep up on these things.  Just a thought.

[rvargas (doing something else), 2014-02-20 20:53:37]

On 2014/02/20 20:30:22, Drew Haven wrote:
> lgtm
> 
> Is there a way that we could add something like upload warnings when code
> directly references deprecated code?  As someone who isn't a full Chrome
> developer it is hard to keep up on these things.  Just a thought.

Thanks.

We could add presubmit warnings, but I'm just trying to remove the old code as
soon as possible to eliminate the issue from the root. It doesn't seem so bad so
far.

[rvargas (doing something else), 2014-02-21 15:12:57]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2014-02-21 15:13:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/166573007/560001

[commit-bot: I haz the power, 2014-02-21 15:13:09]

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

[commit-bot: I haz the power, 2014-02-21 15:13:09]

Failed to apply patch for
chrome/browser/extensions/api/image_writer_private/operation_linux.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index: chrome/browser/extensions/api/image_writer_private/operation_linux.cc
  |diff --git
a/chrome/browser/extensions/api/image_writer_private/operation_linux.cc
b/chrome/browser/extensions/api/image_writer_private/operation_linux.cc
  |index
efba6311138f218927e9d7e07f4f51216accf058..01aa1405ec2a2ef33eafabbd4a69be04f931a916
100644
  |--- a/chrome/browser/extensions/api/image_writer_private/operation_linux.cc
  |+++ b/chrome/browser/extensions/api/image_writer_private/operation_linux.cc
  --------------------------
  No file to patch.  Skipping patch.
  1 out of 1 hunk ignored

Patch:      
chrome/browser/extensions/api/image_writer_private/operation_linux.cc
Index: chrome/browser/extensions/api/image_writer_private/operation_linux.cc
diff --git
a/chrome/browser/extensions/api/image_writer_private/operation_linux.cc
b/chrome/browser/extensions/api/image_writer_private/operation_linux.cc
index
efba6311138f218927e9d7e07f4f51216accf058..01aa1405ec2a2ef33eafabbd4a69be04f931a916
100644
--- a/chrome/browser/extensions/api/image_writer_private/operation_linux.cc
+++ b/chrome/browser/extensions/api/image_writer_private/operation_linux.cc
@@ -4,6 +4,7 @@
 
 #include "base/file_util.h"
 #include "base/files/file_enumerator.h"
+#include "base/platform_file.h"
 #include "base/threading/worker_pool.h"
 #include "chrome/browser/extensions/api/image_writer_private/error_messages.h"
 #include "chrome/browser/extensions/api/image_writer_private/operation.h"

[rvargas (doing something else), 2014-02-21 18:10:21]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2014-02-21 18:10:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/166573007/680001

[commit-bot: I haz the power, 2014-02-21 18:23:50]

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

[commit-bot: I haz the power, 2014-02-21 18:23:51]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[commit-bot: I haz the power, 2014-02-21 18:24:21]

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

[commit-bot: I haz the power, 2014-02-21 18:24:22]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[rvargas (doing something else), 2014-02-21 19:21:20]

On 2014/02/21 18:24:22, I haz the power (commit-bot) wrote:
> No LGTM from a valid reviewer yet. Only full committers are accepted.
> Even if an LGTM may have been provided, it was from a non-committer or
> a lowly provisional committer, _not_ a full super star committer.
> See http://www.chromium.org/getting-involved/become-a-committer
> Note that this has nothing to do with OWNERS files.

Interesting... do you guys know who is this referring to?. I mean, do I kneed
another owner of zlib or of image writer?

[Sergey Berezin, 2014-02-21 19:34:52]

The CQ bit was checked by sergeyberezin@chromium.org

[commit-bot: I haz the power, 2014-02-21 19:37:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/166573007/680001

[commit-bot: I haz the power, 2014-02-21 21:58:29]

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

[commit-bot: I haz the power, 2014-02-21 21:58:30]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[rvargas (doing something else), 2014-02-24 20:19:36]

+asargent + satorux for owners

[rvargas (doing something else), 2014-02-24 22:58:34]

The CQ bit was checked by rvargas@chromium.org

[rvargas (doing something else), 2014-02-24 22:58:57]

The CQ bit was unchecked by rvargas@chromium.org

[rvargas (doing something else), 2014-02-24 23:00:32]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2014-02-24 23:04:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/166573007/680001

[asargent_no_longer_on_chrome, 2014-02-24 23:54:55]

lgtm

[commit-bot: I haz the power, 2014-02-25 01:36:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/166573007/680001

[commit-bot: I haz the power, 2014-02-25 04:07:29]

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

[commit-bot: I haz the power, 2014-02-25 04:07:29]

Retried try job too often on linux_rel for step(s) telemetry_perf_unittests,
telemetry_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[satorux1, 2014-02-25 06:05:41]

lgtm

[rvargas (doing something else), 2014-02-25 19:00:48]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2014-02-25 19:02:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/166573007/680001

[commit-bot: I haz the power, 2014-02-25 19:04:34]

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

[commit-bot: I haz the power, 2014-02-25 19:04:35]

Try jobs failed on following builders: linux_chromium_clang_dbg

[rvargas (doing something else), 2014-02-26 22:58:20]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2014-02-26 23:02:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/166573007/1010001

[commit-bot: I haz the power, 2014-02-27 05:58:16]

Change committed as 253727