Fixed up issue with changing screen resolution on Mac host causing crashes.

Fixed up issue with changing screen resolution on Mac host causing crashes.

BUG=92353
TEST=Start up remoting session with mac as host. Start changing the resolution.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=97046

[dmac, 2011-08-15 20:30:08]

PTAL

[Jamie, 2011-08-15 21:29:27]

http://codereview.chromium.org/7649025/diff/2001/remoting/host/capturer_mac.cc
File remoting/host/capturer_mac.cc (right):

http://codereview.chromium.org/7649025/diff/2001/remoting/host/capturer_mac.c...
remoting/host/capturer_mac.cc:189: // information. Usually owned by the
capture_thread or the main_thread.
Usually? If this is worth documenting, I think it should be more precise than
this.

http://codereview.chromium.org/7649025/diff/2001/remoting/host/capturer_mac.c...
remoting/host/capturer_mac.cc:235: last_buffer_ = NULL;
Why move this?

http://codereview.chromium.org/7649025/diff/2001/remoting/host/capturer_mac.c...
remoting/host/capturer_mac.cc:333: callback->Run(data);
Is it worth scoping the lock more tightly so that it's released before running
the callback, in case it ever does anything non-trivial?

http://codereview.chromium.org/7649025/diff/2001/remoting/host/capturer_mac.c...
remoting/host/capturer_mac.cc:343: SkIRect clip_rect =
SkIRect::MakeWH(buffer_width, buffer_height);
Is clipping required for any of the other Blit functions?

http://codereview.chromium.org/7649025/diff/2001/remoting/host/capturer_mac.c...
remoting/host/capturer_mac.cc:476:
display_configuration_capture_lock_.Acquire();
Does OS X impose a timeout on these callbacks? I'm a little concerned that if
this blocks for some reason (mismatched Begin/End flags or a hung capturer, for
example), then a mode switch might lock the machine permanently. I don't see any
AcquireWithTimeout method, so there's probably not a lot we can do about this.

[dmac, 2011-08-16 19:18:55]

PTAL

http://codereview.chromium.org/7649025/diff/2001/remoting/host/capturer_mac.cc
File remoting/host/capturer_mac.cc (right):

http://codereview.chromium.org/7649025/diff/2001/remoting/host/capturer_mac.c...
remoting/host/capturer_mac.cc:189: // information. Usually owned by the
capture_thread or the main_thread.
On 2011/08/15 21:29:27, Jamie wrote:
> Usually? If this is worth documenting, I think it should be more precise than
> this.

Done.

http://codereview.chromium.org/7649025/diff/2001/remoting/host/capturer_mac.c...
remoting/host/capturer_mac.cc:235: last_buffer_ = NULL;
On 2011/08/15 21:29:27, Jamie wrote:
> Why move this?

Done.

http://codereview.chromium.org/7649025/diff/2001/remoting/host/capturer_mac.c...
remoting/host/capturer_mac.cc:333: callback->Run(data);
On 2011/08/15 21:29:27, Jamie wrote:
> Is it worth scoping the lock more tightly so that it's released before running
> the callback, in case it ever does anything non-trivial?

Done.

http://codereview.chromium.org/7649025/diff/2001/remoting/host/capturer_mac.c...
remoting/host/capturer_mac.cc:343: SkIRect clip_rect =
SkIRect::MakeWH(buffer_width, buffer_height);
On 2011/08/15 21:29:27, Jamie wrote:
> Is clipping required for any of the other Blit functions?

Done.

http://codereview.chromium.org/7649025/diff/2001/remoting/host/capturer_mac.c...
remoting/host/capturer_mac.cc:476:
display_configuration_capture_lock_.Acquire();
On 2011/08/15 21:29:27, Jamie wrote:
> Does OS X impose a timeout on these callbacks? I'm a little concerned that if
> this blocks for some reason (mismatched Begin/End flags or a hung capturer,
for
> example), then a mode switch might lock the machine permanently. I don't see
any
> AcquireWithTimeout method, so there's probably not a lot we can do about this.

Done.

[Jamie, 2011-08-16 19:42:40]

LGTM.

[commit-bot: I haz the power, 2011-08-16 22:40:05]

Change committed as 97046