[webkit-reviews] review denied: [Bug 183048] The WebContent process should not use NSScreen in the screenDepth implementation. : [Attachment 334464] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has denied Per Arne Vollan
<pvollan at apple.com>'s request for review:
Bug 183048: The WebContent process should not use NSScreen in the screenDepth
implementation.
https://bugs.webkit.org/show_bug.cgi?id=183048

Attachment 334464: Patch

https://bugs.webkit.org/attachment.cgi?id=334464&action=review




--- Comment #4 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 334464
  --> https://bugs.webkit.org/attachment.cgi?id=334464
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=334464&action=review

This patch looks good, but I think it would be better if you got rid of the
duplicated code. Can you please revise that?

> Source/WebCore/platform/mac/PlatformScreenMac.mm:140
> +	   return screenProperties().get(iter->key).screenDepth;

This snippet of logic is also in 'screenRect' and 'screenAvailableRect', (and
perhaps others?). Can you encapsulate it in a common function that returns the
appropriate screenProperties, then each of these methods can just return the
member we are interested in?

Also: Should we ASSERT if the depth is zero, just in case the IPC didn't happen
for some reason?