Test for power of two safely

Test for power of two safely

Use integer operations to test if an integer is a power of two.  And
check that the value is in range before testing for a power.

BUG=675570
TEST=

Committed: https://crrev.com/8c22c74f1f57853ea427fa5f21f170144d4103ed
Cr-Commit-Position: refs/heads/master@{#439603}

[Raymond Toy, 2016-12-19 17:34:15]

The CQ bit was checked by rtoy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-19 17:34:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Raymond Toy, 2016-12-19 17:34:51]

rtoy@chromium.org changed reviewers:
+ hongchan@chromium.org

[Raymond Toy, 2016-12-19 17:34:52]

PTAL

[commit-bot: I haz the power, 2016-12-19 19:51:41]

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

[commit-bot: I haz the power, 2016-12-19 19:51:42]

Dry run: This issue passed the CQ dry run.

[hongchan, 2016-12-19 21:57:48]

lgtm with nit

https://codereview.chromium.org/2582323003/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/AudioUtilities.h (right):

https://codereview.chromium.org/2582323003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/AudioUtilities.h:34: namespace
AudioUtilities {
Just an idea:

Make AudioUtilities as a class and define as PLATFORM_EXPORT?

[Raymond Toy, 2016-12-19 21:59:57]

On 2016/12/19 21:57:48, hongchan wrote:
> lgtm with nit
> 
>
https://codereview.chromium.org/2582323003/diff/1/third_party/WebKit/Source/p...
> File third_party/WebKit/Source/platform/audio/AudioUtilities.h (right):
> 
>
https://codereview.chromium.org/2582323003/diff/1/third_party/WebKit/Source/p...
> third_party/WebKit/Source/platform/audio/AudioUtilities.h:34: namespace
> AudioUtilities {
> Just an idea:
> 
> Make AudioUtilities as a class and define as PLATFORM_EXPORT?

We could, but I find classes that contain a random grab bag of random stuff,
just too random. :-)  There's no underlying unifying idea for what the class is
or should be.

[blink-reviews, 2016-12-19 22:18:35]

Sure - LGTM again.

On Mon, Dec 19, 2016 at 1:59 PM rtoy@chromium.org via
codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote:

> On 2016/12/19 21:57:48, hongchan wrote:
> > lgtm with nit
> >
> >
>
>
https://codereview.chromium.org/2582323003/diff/1/third_party/WebKit/Source/p...
> > File third_party/WebKit/Source/platform/audio/AudioUtilities.h (right):
> >
> >
>
>
https://codereview.chromium.org/2582323003/diff/1/third_party/WebKit/Source/p...
> > third_party/WebKit/Source/platform/audio/AudioUtilities.h:34: namespace
> > AudioUtilities {
> > Just an idea:
> >
> > Make AudioUtilities as a class and define as PLATFORM_EXPORT?
>
> We could, but I find classes that contain a random grab bag of random
> stuff,
> just too random. :-) There's no underlying unifying idea for what the
> class is
> or should be.
>
> https://codereview.chromium.org/2582323003/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-12-19 22:18:35]

Sure - LGTM again.

On Mon, Dec 19, 2016 at 1:59 PM rtoy@chromium.org via
codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote:

> On 2016/12/19 21:57:48, hongchan wrote:
> > lgtm with nit
> >
> >
>
>
https://codereview.chromium.org/2582323003/diff/1/third_party/WebKit/Source/p...
> > File third_party/WebKit/Source/platform/audio/AudioUtilities.h (right):
> >
> >
>
>
https://codereview.chromium.org/2582323003/diff/1/third_party/WebKit/Source/p...
> > third_party/WebKit/Source/platform/audio/AudioUtilities.h:34: namespace
> > AudioUtilities {
> > Just an idea:
> >
> > Make AudioUtilities as a class and define as PLATFORM_EXPORT?
>
> We could, but I find classes that contain a random grab bag of random
> stuff,
> just too random. :-) There's no underlying unifying idea for what the
> class is
> or should be.
>
> https://codereview.chromium.org/2582323003/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Raymond Toy, 2016-12-19 22:52:03]

The CQ bit was checked by rtoy@chromium.org

[commit-bot: I haz the power, 2016-12-19 22:52:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-19 22:58:28]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1482187923546800, "parent_rev":
"7866f6af1522890499a9b9523a0a2dccf668d8bc", "commit_rev":
"252b4d000e4bccdd694b9233513f6a286538738a"}

[commit-bot: I haz the power, 2016-12-19 22:58:55]

Description was changed from

==========
Test for power of two safely

Use integer operations to test if an integer is a power of two.  And
check that the value is in range before testing for a power.

BUG=675570
TEST=
==========

to

==========
Test for power of two safely

Use integer operations to test if an integer is a power of two.  And
check that the value is in range before testing for a power.

BUG=675570
TEST=

Review-Url: https://codereview.chromium.org/2582323003
==========

[commit-bot: I haz the power, 2016-12-19 22:58:56]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-12-19 23:00:48]

Description was changed from

==========
Test for power of two safely

Use integer operations to test if an integer is a power of two.  And
check that the value is in range before testing for a power.

BUG=675570
TEST=

Review-Url: https://codereview.chromium.org/2582323003
==========

to

==========
Test for power of two safely

Use integer operations to test if an integer is a power of two.  And
check that the value is in range before testing for a power.

BUG=675570
TEST=

Committed: https://crrev.com/8c22c74f1f57853ea427fa5f21f170144d4103ed
Cr-Commit-Position: refs/heads/master@{#439603}
==========

[commit-bot: I haz the power, 2016-12-19 23:00:48]

Patchset 1 (id:??) landed as
https://crrev.com/8c22c74f1f57853ea427fa5f21f170144d4103ed
Cr-Commit-Position: refs/heads/master@{#439603}