The snap ratio is wrong

The snap ratio is wrong

Snapping which is intended to snap to existing scale during pinch
is not working. That is, because the snap ratio constant is wrong,
snapping logic do not work and might make extra tiling during pinch.

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

[yongha78.lee, 2014-04-29 12:06:29]

PTAL

[danakj, 2014-04-29 17:30:04]

https://codereview.chromium.org/256343002/diff/10001/cc/layers/picture_layer_...
File cc/layers/picture_layer_impl.cc (right):

https://codereview.chromium.org/256343002/diff/10001/cc/layers/picture_layer_...
cc/layers/picture_layer_impl.cc:1054: float snapped_ratio =
kMaxScaleRatioDuringPinch;
This was purposely written as a separate constant. Can you give an example of
when this is doing the wrong thing?

[yongha78.lee, 2014-04-30 00:52:19]

On 2014/04/29 17:30:04, danakj wrote:
>
https://codereview.chromium.org/256343002/diff/10001/cc/layers/picture_layer_...
> File cc/layers/picture_layer_impl.cc (right):
> 
>
https://codereview.chromium.org/256343002/diff/10001/cc/layers/picture_layer_...
> cc/layers/picture_layer_impl.cc:1054: float snapped_ratio =
> kMaxScaleRatioDuringPinch;
> This was purposely written as a separate constant. Can you give an example of
> when this is doing the wrong thing?

Test case is just pinch zoom scenario.

float ratio = PositiveRatio(tiling_contents_scale, scale);

The ratio which is calculated by 'PositiveRatio' API is always bigger than 1.
Then, because 'snapped_ratio' value is constant 0.2, it never goes into 'if'
statement.

if (ratio < snapped_ratio) <== it is always FALSE

But if we have change the constant value bigger than 1, we can set
'raster_contents_scale_' to existing scale.

For example, desired Scale = 6, existing scale=4. PositiveRatio return 1.5. Then
desired scale is changed to existing scale it is already added tiling. It means
that we don't need to do new AddTiling.

[enne (OOO), 2014-04-30 01:12:37]

Maybe kSnapToExistingTileRatio should be 1.2 instead of 0.2?

https://codereview.chromium.org/81453002 added this code along with some unit
tests.  If this code is wrong, maybe you could add some demonstrative unit tests
that show how it should work?

[epennerAtGoogle, 2014-05-01 17:39:31]

On 2014/04/30 01:12:37, enne wrote:
> Maybe kSnapToExistingTileRatio should be 1.2 instead of 0.2?
> 
> https://codereview.chromium.org/81453002 added this code along with some unit
> tests.  If this code is wrong, maybe you could add some demonstrative unit
tests
> that show how it should work?

Nice catch! Looking at it, totally it looks like it should never be < 1 and this
will never take effect.

This was working before, but possibly I messed it up when I changed the other
logic to also snap better. Since the other logic snaps to multiples of 2, I was
debating whether this was necessary. But I left it in as it can't hurt. It looks
like subtracting '1' or using 1.2 is the right fix.

[yongha78.lee, 2014-05-02 04:16:07]

On 2014/05/01 17:39:31, epennerAtGoogle wrote:
> On 2014/04/30 01:12:37, enne wrote:
> > Maybe kSnapToExistingTileRatio should be 1.2 instead of 0.2?
> > 
> > https://codereview.chromium.org/81453002 added this code along with some
unit
> > tests.  If this code is wrong, maybe you could add some demonstrative unit
> tests
> > that show how it should work?
> 
> Nice catch! Looking at it, totally it looks like it should never be < 1 and
this
> will never take effect.
> 
> This was working before, but possibly I messed it up when I changed the other
> logic to also snap better. Since the other logic snaps to multiples of 2, I
was
> debating whether this was necessary. But I left it in as it can't hurt. It
looks
> like subtracting '1' or using 1.2 is the right fix.

In current logic, I could not find problem case with unit tests because raster
scale is always divide by 2 or multiple by 2. Initially, I just think that the
problem case(extra tiling) can be happened by the code. However, current code is
working. But if current logic regarding snapping change, the code might be make
tiling problem. I have changed 0.2 to 1.2.

[epennerAtGoogle, 2014-05-02 19:47:21]

On 2014/05/02 04:16:07, yongha78.lee wrote:
> On 2014/05/01 17:39:31, epennerAtGoogle wrote:
> > On 2014/04/30 01:12:37, enne wrote:
> > > Maybe kSnapToExistingTileRatio should be 1.2 instead of 0.2?
> > > 
> > > https://codereview.chromium.org/81453002 added this code along with some
> unit
> > > tests.  If this code is wrong, maybe you could add some demonstrative unit
> > tests
> > > that show how it should work?
> > 
> > Nice catch! Looking at it, totally it looks like it should never be < 1 and
> this
> > will never take effect.
> > 
> > This was working before, but possibly I messed it up when I changed the
other
> > logic to also snap better. Since the other logic snaps to multiples of 2, I
> was
> > debating whether this was necessary. But I left it in as it can't hurt. It
> looks
> > like subtracting '1' or using 1.2 is the right fix.
> 
> In current logic, I could not find problem case with unit tests because raster
> scale is always divide by 2 or multiple by 2. Initially, I just think that the
> problem case(extra tiling) can be happened by the code. However, current code
is
> working. But if current logic regarding snapping change, the code might be
make
> tiling problem. I have changed 0.2 to 1.2.

I think it could still be tested, by artificially setting up a situation in
which there is a tiling within 20% scale difference, and just making sure that
it snaps in that cast. It's okay to create that situation artificially in the
test IMO, since there is several different causes for different scales to exist,
and we just need to test that it does snap in those cases.

[yongha78.lee, 2014-05-08 05:23:05]

On 2014/05/02 19:47:21, epennerAtGoogle wrote:
> On 2014/05/02 04:16:07, yongha78.lee wrote:
> > On 2014/05/01 17:39:31, epennerAtGoogle wrote:
> > > On 2014/04/30 01:12:37, enne wrote:
> > > > Maybe kSnapToExistingTileRatio should be 1.2 instead of 0.2?
> > > > 
> > > > https://codereview.chromium.org/81453002 added this code along with some
> > unit
> > > > tests.  If this code is wrong, maybe you could add some demonstrative
unit
> > > tests
> > > > that show how it should work?
> > > 
> > > Nice catch! Looking at it, totally it looks like it should never be < 1
and
> > this
> > > will never take effect.
> > > 
> > > This was working before, but possibly I messed it up when I changed the
> other
> > > logic to also snap better. Since the other logic snaps to multiples of 2,
I
> > was
> > > debating whether this was necessary. But I left it in as it can't hurt. It
> > looks
> > > like subtracting '1' or using 1.2 is the right fix.
> > 
> > In current logic, I could not find problem case with unit tests because
raster
> > scale is always divide by 2 or multiple by 2. Initially, I just think that
the
> > problem case(extra tiling) can be happened by the code. However, current
code
> is
> > working. But if current logic regarding snapping change, the code might be
> make
> > tiling problem. I have changed 0.2 to 1.2.
> 
> I think it could still be tested, by artificially setting up a situation in
> which there is a tiling within 20% scale difference, and just making sure that
> it snaps in that cast. It's okay to create that situation artificially in the
> test IMO, since there is several different causes for different scales to
exist,
> and we just need to test that it does snap in those cases.

I have added unit test case. In test case, because desired scale is within 1.2
ratio of existing scale, desired scale should not add new tiling. please review
these.

[epenner, 2014-05-08 22:17:48]

On 2014/05/08 05:23:05, yongha78.lee wrote:
> On 2014/05/02 19:47:21, epennerAtGoogle wrote:
> > On 2014/05/02 04:16:07, yongha78.lee wrote:
> > > On 2014/05/01 17:39:31, epennerAtGoogle wrote:
> > > > On 2014/04/30 01:12:37, enne wrote:
> > > > > Maybe kSnapToExistingTileRatio should be 1.2 instead of 0.2?
> > > > > 
> > > > > https://codereview.chromium.org/81453002 added this code along with
some
> > > unit
> > > > > tests.  If this code is wrong, maybe you could add some demonstrative
> unit
> > > > tests
> > > > > that show how it should work?
> > > > 
> > > > Nice catch! Looking at it, totally it looks like it should never be < 1
> and
> > > this
> > > > will never take effect.
> > > > 
> > > > This was working before, but possibly I messed it up when I changed the
> > other
> > > > logic to also snap better. Since the other logic snaps to multiples of
2,
> I
> > > was
> > > > debating whether this was necessary. But I left it in as it can't hurt.
It
> > > looks
> > > > like subtracting '1' or using 1.2 is the right fix.
> > > 
> > > In current logic, I could not find problem case with unit tests because
> raster
> > > scale is always divide by 2 or multiple by 2. Initially, I just think that
> the
> > > problem case(extra tiling) can be happened by the code. However, current
> code
> > is
> > > working. But if current logic regarding snapping change, the code might be
> > make
> > > tiling problem. I have changed 0.2 to 1.2.
> > 
> > I think it could still be tested, by artificially setting up a situation in
> > which there is a tiling within 20% scale difference, and just making sure
that
> > it snaps in that cast. It's okay to create that situation artificially in
the
> > test IMO, since there is several different causes for different scales to
> exist,
> > and we just need to test that it does snap in those cases.
> 
> I have added unit test case. In test case, because desired scale is within 1.2
> ratio of existing scale, desired scale should not add new tiling. please
review
> these.

Thanks for the test! Lgtm.

[enne (OOO), 2014-05-08 22:26:18]

Could you update the description and the title of this bug to something
more...descriptive? The bug you're fixing is that snapping isn't working as
intended because of a bad constant and it's creating extra tilings during pinch.

[yongha78.lee, 2014-05-09 00:50:17]

The CQ bit was checked by yongha78.lee@samsung.com

[commit-bot: I haz the power, 2014-05-09 00:57:31]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/yongha78.lee@samsung.com/256343002/70001

[commit-bot: I haz the power, 2014-05-09 02:14:28]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-05-09 02:26:44]

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

[commit-bot: I haz the power, 2014-05-09 02:26:44]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[yongha78.lee, 2014-05-09 04:36:24]

The CQ bit was checked by yongha78.lee@samsung.com

[commit-bot: I haz the power, 2014-05-09 04:41:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/yongha78.lee@samsung.com/256343002/70001

[commit-bot: I haz the power, 2014-05-09 05:21:58]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-05-09 05:29:11]

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

[commit-bot: I haz the power, 2014-05-09 05:29:11]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[yongha78.lee, 2014-05-09 07:43:21]

The CQ bit was checked by yongha78.lee@samsung.com

[commit-bot: I haz the power, 2014-05-09 07:49:56]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/yongha78.lee@samsung.com/256343002/90001

[commit-bot: I haz the power, 2014-05-09 08:50:28]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-05-09 08:57:18]

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

[commit-bot: I haz the power, 2014-05-09 08:57:18]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[yongha78.lee, 2014-05-09 10:37:11]

The CQ bit was checked by yongha78.lee@samsung.com

[commit-bot: I haz the power, 2014-05-09 10:42:01]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/yongha78.lee@samsung.com/256343002/110001

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

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)

[commit-bot: I haz the power, 2014-05-09 12:55:59]

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

[commit-bot: I haz the power, 2014-05-09 12:56:00]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[yongha78.lee, 2014-05-09 23:40:07]

On 2014/05/08 22:26:18, enne wrote:
> Could you update the description and the title of this bug to something
> more...descriptive? The bug you're fixing is that snapping isn't working as
> intended because of a bad constant and it's creating extra tilings during
pinch.

Dear enne

I have updated bug description more specifically.
Could you review those?

[enne (OOO), 2014-05-09 23:49:01]

On 2014/05/09 23:40:07, yongha78.lee wrote:
> On 2014/05/08 22:26:18, enne wrote:
> > Could you update the description and the title of this bug to something
> > more...descriptive? The bug you're fixing is that snapping isn't working as
> > intended because of a bad constant and it's creating extra tilings during
> pinch.
> 
> Dear enne
> 
> I have updated bug description more specifically.
> Could you review those?

Sure.  Can you wrap it to 72 columns?

[yongha78.lee, 2014-05-10 00:14:23]

On 2014/05/09 23:49:01, enne wrote:
> On 2014/05/09 23:40:07, yongha78.lee wrote:
> > On 2014/05/08 22:26:18, enne wrote:
> > > Could you update the description and the title of this bug to something
> > > more...descriptive? The bug you're fixing is that snapping isn't working
as
> > > intended because of a bad constant and it's creating extra tilings during
> > pinch.
> > 
> > Dear enne
> > 
> > I have updated bug description more specifically.
> > Could you review those?
> 
> Sure.  Can you wrap it to 72 columns?

Dear enne

I have modified description like your guide.
Please review this

[enne (OOO), 2014-05-10 00:15:35]

Thanks, lgtm.

[yongha78.lee, 2014-05-10 00:27:20]

The CQ bit was checked by yongha78.lee@samsung.com

[commit-bot: I haz the power, 2014-05-10 00:32:21]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/yongha78.lee@samsung.com/256343002/110001

[commit-bot: I haz the power, 2014-05-10 11:48:54]

Change committed as 269563