Added SharedMemory::MapFrom.

Added SharedMemory::MapFrom.

Fragmentation of address space may prevents process from allocate entire block of shared memory. Particularly in attached bug browser process can't allocate 250Mb to accept data from renderer Mapping smaller blocks is workaround.

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

[Vitaly Buka (NO REVIEWS), 2013-01-15 18:27:49]

Jim, please take a look.

[Mark Mentovai, 2013-01-15 21:51:41]

https://codereview.chromium.org/11876037/diff/19001/base/shared_memory.h
File base/shared_memory.h (right):

https://codereview.chromium.org/11876037/diff/19001/base/shared_memory.h#newc...
base/shared_memory.h:154: bool MapFrom(off_t offset, size_t bytes);
Isn’t this more MapAt than MapFrom?

https://codereview.chromium.org/11876037/diff/19001/base/shared_memory_posix.cc
File base/shared_memory_posix.cc (right):

https://codereview.chromium.org/11876037/diff/19001/base/shared_memory_posix....
base/shared_memory_posix.cc:247: offset & (SysInfo::VMAllocationGranularity() -
1));
Is this really any of our business?

https://codereview.chromium.org/11876037/diff/19001/base/shared_memory_win.cc
File base/shared_memory_win.cc (right):

https://codereview.chromium.org/11876037/diff/19001/base/shared_memory_win.cc...
base/shared_memory_win.cc:145: DCHECK_EQ(0U, offset &
(SysInfo::VMAllocationGranularity() - 1));
Same.

https://codereview.chromium.org/11876037/diff/19001/base/shared_memory_win.cc...
base/shared_memory_win.cc:151: int e = GetLastError();
Unused.

[Vitaly Buka (NO REVIEWS), 2013-01-15 22:13:05]

https://codereview.chromium.org/11876037/diff/19001/base/shared_memory.h
File base/shared_memory.h (right):

https://codereview.chromium.org/11876037/diff/19001/base/shared_memory.h#newc...
base/shared_memory.h:154: bool MapFrom(off_t offset, size_t bytes);
On 2013/01/15 21:51:42, Mark Mentovai wrote:
> Isn’t this more MapAt than MapFrom?

Done.

Ideally I'd included offest to Map() interface, but it requires huge CL to
update code.

https://codereview.chromium.org/11876037/diff/19001/base/shared_memory_posix.cc
File base/shared_memory_posix.cc (right):

https://codereview.chromium.org/11876037/diff/19001/base/shared_memory_posix....
base/shared_memory_posix.cc:247: offset & (SysInfo::VMAllocationGranularity() -
1));
On 2013/01/15 21:51:42, Mark Mentovai wrote:
> Is this really any of our business?

memory_ will be NULL, if value is not aligned. Just hit for developer to fix
calling code.

https://codereview.chromium.org/11876037/diff/19001/base/shared_memory_win.cc
File base/shared_memory_win.cc (right):

https://codereview.chromium.org/11876037/diff/19001/base/shared_memory_win.cc...
base/shared_memory_win.cc:145: DCHECK_EQ(0U, offset &
(SysInfo::VMAllocationGranularity() - 1));
On 2013/01/15 21:51:42, Mark Mentovai wrote:
> Same.

So do you advise to remove DCHECK?

https://codereview.chromium.org/11876037/diff/19001/base/shared_memory_win.cc...
base/shared_memory_win.cc:151: int e = GetLastError();
On 2013/01/15 21:51:42, Mark Mentovai wrote:
> Unused.

Done.

[jar (doing other things), 2013-01-15 22:20:29]

Drive by (Mark seems to be looking closely):

https://codereview.chromium.org/11876037/diff/27001/base/shared_memory_posix.cc
File base/shared_memory_posix.cc (right):

https://codereview.chromium.org/11876037/diff/27001/base/shared_memory_posix....
base/shared_memory_posix.cc:246: DCHECK_EQ(static_cast<size_t>(0),
I assume this call is not made that often... so how about doing a real check,
rather then a DCHECK (or at least returning false when this is violated)?

[Mark Mentovai, 2013-01-15 22:26:01]

https://codereview.chromium.org/11876037/diff/27001/base/shared_memory_posix.cc
File base/shared_memory_posix.cc (right):

https://codereview.chromium.org/11876037/diff/27001/base/shared_memory_posix....
base/shared_memory_posix.cc:247: offset & (SysInfo::VMAllocationGranularity() -
1));
My point was that this is sort of an implementation detail. There are a few
different reasons that memory_ might wind up NULL. Why are you singling this one
out and DCHECKing for it? mmap parameter validity checks are mmap’s
responsibility. If we want to crash when mmap returns NULL, we should
CHECK(memory_). Otherwise, why are you picking on one parameter and writing your
own debug-only check for it?

[Vitaly Buka (NO REVIEWS), 2013-01-15 22:58:54]

https://codereview.chromium.org/11876037/diff/27001/base/shared_memory_posix.cc
File base/shared_memory_posix.cc (right):

https://codereview.chromium.org/11876037/diff/27001/base/shared_memory_posix....
base/shared_memory_posix.cc:246: DCHECK_EQ(static_cast<size_t>(0),
On 2013/01/15 22:20:29, jar wrote:
> I assume this call is not made that often... so how about doing a real check,
> rather then a DCHECK (or at least returning false when this is violated)?

it will return false because memory_ will be invalid.

https://codereview.chromium.org/11876037/diff/27001/base/shared_memory_posix....
base/shared_memory_posix.cc:247: offset & (SysInfo::VMAllocationGranularity() -
1));
On 2013/01/15 22:26:02, Mark Mentovai wrote:
> My point was that this is sort of an implementation detail. There are a few
> different reasons that memory_ might wind up NULL. Why are you singling this
one
> out and DCHECKing for it? mmap parameter validity checks are mmap’s
> responsibility. If we want to crash when mmap returns NULL, we should
> CHECK(memory_). Otherwise, why are you picking on one parameter and writing
your
> own debug-only check for it?

I don't believe we want crash on mmap returns null, it may happen on reasonable
runtime conditions.

So then I just remove dcheck.

[Mark Mentovai, 2013-01-15 23:02:44]

LGTM

[commit-bot: I haz the power, 2013-01-16 05:45:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vitalybuka@chromium.org/11876037/41007

[commit-bot: I haz the power, 2013-01-16 09:02:36]

Change committed as 177109