Tweaks to SharedMemory to avoid CHECK when printing

Tweaks to SharedMemory to avoid CHECK when printing

The PDF printing code set INVALID_HANDLE_VALUE but SharedMemory could
only correctly handle NULL. This change fixes SharedMemory to be more
forgiving and fixes the PDF printing code to be consistent.

I'm glad that the new SharedMemory error checking found a bug, but I
wish it had been something more meaningful.

R=rvargas@chromium.org,sdefresne@chromium.org
BUG=533310

[brucedawson, 2015-09-18 19:57:52]

I spot checked five of the crashes caused by my error checking. In all cases the
handle was INVALID_HANDLE_VALUE so I am confident that this will fix the problem
and let me reland change https://codereview.chromium.org/1350493002.

The change to print_web_view_helper_pdf_win.cc is not strictly necessary and
could be omitted, but using NULL is more consistent.

Alternately the change to shared_memory_win.cc could be omitted, but that seems
needlessly fragile.

[rvargas (doing something else), 2015-09-18 22:36:26]

Note: <rants> are not directed at you. You're just the unlucky guy trying to fix
this.

https://codereview.chromium.org/1351373002/diff/1/base/memory/shared_memory_w...
File base/memory/shared_memory_win.cc (right):

https://codereview.chromium.org/1351373002/diff/1/base/memory/shared_memory_w...
base/memory/shared_memory_win.cc:77: return handle != NULL && handle !=
INVALID_HANDLE_VALUE;
<rant>
I know I'm grumpy, but this pattern doesn't make sense with or without this
change :(.

I know this is not what this method is promising, but there's no way to see if a
random handle is valid, and I'd rather remove this method completely and leave
that job to whoever is creating the handle (aka, remove raw handles completely).

Now, I understand that fix would be out of the scope of the current problem.

If we go with the current API, if a caller grabs a number from a hat and uses
it, it's basically depending on the internal implementation of this class, so
widening the range of values that we won't complain about is... kind of silly? I
hear that we should make this less prone to misuse, but really, we should fix
the API instead of adding knee pads to the cutting edges. We should be using
ScopedHandles here, and the caller should be using ScopedHandles, even if we end
up passing the raw handle through the interface.
</rant>

https://codereview.chromium.org/1351373002/diff/1/base/memory/shared_memory_w...
base/memory/shared_memory_win.cc:87: DCHECK(IsHandleValid(handle));
ScopedHandle closer(handle);

<rant> receiving const SharedHandle& for something that we'll be essentially
destroying doesn't make sense to me. This method also has to go away </rant>

https://codereview.chromium.org/1351373002/diff/1/base/memory/shared_memory_w...
base/memory/shared_memory_win.cc:181: DCHECK(!IsHandleValid(mapped_file_));
DCHECK(mapped_file_.Isvalid());

(+ making the member a ScopedHandle). etc.

[sdefresne, 2016-01-12 08:53:41]

sdefresne@chromium.org changed reviewers:
- sdefresne@chromium.org