Add a couple UIKit util functions in src/ui/ios/

Add a couple UIKit util functions in src/ui/ios/

BUG=None

Committed: https://crrev.com/baf439d8500da3284a73f56324d13916590f8f7a
Cr-Commit-Position: refs/heads/master@{#293933}

[lliabraa, 2014-09-04 19:21:05]

lliabraa@chromium.org changed reviewers:
+ stuartmorgan@chromium.org

[stuartmorgan, 2014-09-04 22:16:11]

https://codereview.chromium.org/539253002/diff/1/ui/ios/ui_ios_tests.gyp
File ui/ios/ui_ios_tests.gyp (right):

https://codereview.chromium.org/539253002/diff/1/ui/ios/ui_ios_tests.gyp#newc...
ui/ios/ui_ios_tests.gyp:11: 'target_name': 'ui_ios_unittests',
Belatedly: did this get added to bots?

https://codereview.chromium.org/539253002/diff/1/ui/ios/uikit_util.h
File ui/ios/uikit_util.h (right):

https://codereview.chromium.org/539253002/diff/1/ui/ios/uikit_util.h#newcode16
ui/ios/uikit_util.h:16: CGFloat alignValueToUpperPixel(CGFloat value);
s/align/Align/ (same below).

I also wonder if we should call it AlignedValueAtUpperPixel (similarly below) so
that nobody writes this bug:
  // get a value
  AlignValueToUpperPixel(value);
  // |value| is aligned now, right?

https://codereview.chromium.org/539253002/diff/1/ui/ios/uikit_util.mm
File ui/ios/uikit_util.mm (right):

https://codereview.chromium.org/539253002/diff/1/ui/ios/uikit_util.mm#newcode13
ui/ios/uikit_util.mm:13: return ceil(value * scale) / scale;
std::ceil so it does the right thing with 64-bit?

https://codereview.chromium.org/539253002/diff/1/ui/ios/uikit_util.mm#newcode18
ui/ios/uikit_util.mm:18: alignValueToUpperPixel(size.height));
Should align inside the (

https://codereview.chromium.org/539253002/diff/1/ui/ios/uikit_util_unittest.mm
File ui/ios/uikit_util_unittest.mm (right):

https://codereview.chromium.org/539253002/diff/1/ui/ios/uikit_util_unittest.m...
ui/ios/uikit_util_unittest.mm:51: }  // anonymous namespace
Just
}  // namespace

[lliabraa, 2014-09-05 16:53:45]

Thanks for the review

https://chromiumcodereview.appspot.com/539253002/diff/1/ui/ios/ui_ios_tests.gyp
File ui/ios/ui_ios_tests.gyp (right):

https://chromiumcodereview.appspot.com/539253002/diff/1/ui/ios/ui_ios_tests.g...
ui/ios/ui_ios_tests.gyp:11: 'target_name': 'ui_ios_unittests',
On 2014/09/04 22:16:10, stuartmorgan wrote:
> Belatedly: did this get added to bots?

Yep. Upstream is done, downstream is waiting on a merge. (crbug.com/410843)

https://chromiumcodereview.appspot.com/539253002/diff/1/ui/ios/uikit_util.h
File ui/ios/uikit_util.h (right):

https://chromiumcodereview.appspot.com/539253002/diff/1/ui/ios/uikit_util.h#n...
ui/ios/uikit_util.h:16: CGFloat alignValueToUpperPixel(CGFloat value);
On 2014/09/04 22:16:10, stuartmorgan wrote:
> s/align/Align/ (same below).

Done.

> 
> I also wonder if we should call it AlignedValueAtUpperPixel (similarly below)
so
> that nobody writes this bug:
>   // get a value
>   AlignValueToUpperPixel(value);
>   // |value| is aligned now, right?

I see your point, but this type of method signature seems fairly common, e.g.
ceil. I've dropped the "Value" and "Size" from the method names so at least the
parameter name is no longer in the method name. Do you think that helps?

Another alternative would be GetValueAlignedToUpperPixel();
GetSizeAlignedToUpperPixel().

https://chromiumcodereview.appspot.com/539253002/diff/1/ui/ios/uikit_util.mm
File ui/ios/uikit_util.mm (right):

https://chromiumcodereview.appspot.com/539253002/diff/1/ui/ios/uikit_util.mm#...
ui/ios/uikit_util.mm:13: return ceil(value * scale) / scale;
On 2014/09/04 22:16:11, stuartmorgan wrote:
> std::ceil so it does the right thing with 64-bit?

Done.

https://chromiumcodereview.appspot.com/539253002/diff/1/ui/ios/uikit_util.mm#...
ui/ios/uikit_util.mm:18: alignValueToUpperPixel(size.height));
On 2014/09/04 22:16:11, stuartmorgan wrote:
> Should align inside the (

Done.

https://chromiumcodereview.appspot.com/539253002/diff/1/ui/ios/uikit_util_uni...
File ui/ios/uikit_util_unittest.mm (right):

https://chromiumcodereview.appspot.com/539253002/diff/1/ui/ios/uikit_util_uni...
ui/ios/uikit_util_unittest.mm:51: }  // anonymous namespace
On 2014/09/04 22:16:11, stuartmorgan wrote:
> Just
> }  // namespace

Done.

[stuartmorgan, 2014-09-05 17:03:15]

Actually, I bet WARN_UNUSED_RESULT works for us. Can you annotate with that, and
verify that it works as expected in iOS?

If so, LGTM with that change (if not I'd prefer the other names you suggested)

[lliabraa, 2014-09-05 17:11:29]

Uploaded a new patch with WARN_UNUSED_RESULT. Thanks for the suggestion. I think
I'll wait for the ui/ios/OWNERS cl to land before sending for OWNERS review.

[lliabraa, 2014-09-05 17:47:40]

lliabraa@chromium.org changed reviewers:
+ rohitrao@chromium.org

[lliabraa, 2014-09-05 17:47:41]

+rohitrao for OWNERS

[lliabraa, 2014-09-05 18:53:44]

The CQ bit was checked by lliabraa@chromium.org

[commit-bot: I haz the power, 2014-09-05 18:54:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/lliabraa@chromium.org/539253002/40001

[commit-bot: I haz the power, 2014-09-05 19:25:00]

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

[commit-bot: I haz the power, 2014-09-05 19:25:01]

Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[lliabraa, 2014-09-09 12:45:50]

The CQ bit was checked by lliabraa@chromium.org

[commit-bot: I haz the power, 2014-09-09 12:46:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/lliabraa@chromium.org/539253002/40001

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

Committed patchset #3 (id:40001) as aa12f7524dbe42ce64a45414cf6300d55260471e

[commit-bot: I haz the power, 2014-09-10 03:53:11]

Patchset 3 (id:??) landed as
https://crrev.com/baf439d8500da3284a73f56324d13916590f8f7a
Cr-Commit-Position: refs/heads/master@{#293933}