Removing a DCHECK in OverscrollWindowAnimation::OnOverscrollModeChange

Removing a DCHECK in OverscrollWindowAnimation::OnOverscrollModeChange

Proper fix is in https://codereview.chromium.org/2983883002/, but I'd rather not
land that while on leave. I will land the proper fix when I come back if this is
still not replaced with the new UI.

BUG=668296
Review-Url: https://codereview.chromium.org/2985543002
Cr-Commit-Position: refs/heads/master@{#488328}
Committed: https://chromium.googlesource.com/chromium/src/+/736822c161cce5c582e0d57b8957c68d683ba23e

[mfomitchev, 2017-07-19 21:01:13]

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

[commit-bot: I haz the power, 2017-07-19 21:01:25]

Dry run: 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, 2017-07-19 22:27:05]

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

[commit-bot: I haz the power, 2017-07-19 22:27:06]

Dry run: This issue passed the CQ dry run.

[mfomitchev, 2017-07-20 02:06:32]

mfomitchev@chromium.org changed reviewers:
+ mohsen@chromium.org

[mfomitchev, 2017-07-20 02:06:32]

Mohsen, can you PTAL?

[mohsen, 2017-07-20 17:22:38]

https://codereview.chromium.org/2985543002/diff/1/content/browser/web_content...
File content/browser/web_contents/aura/overscroll_window_animation.cc (left):

https://codereview.chromium.org/2985543002/diff/1/content/browser/web_content...
content/browser/web_contents/aura/overscroll_window_animation.cc:124:
DCHECK_EQ(overscroll_source_, OverscrollSource::NONE);
Do you mean that this is not always true? If yes, when/why?

[mfomitchev, 2017-07-20 17:28:09]

https://codereview.chromium.org/2985543002/diff/1/content/browser/web_content...
File content/browser/web_contents/aura/overscroll_window_animation.cc (left):

https://codereview.chromium.org/2985543002/diff/1/content/browser/web_content...
content/browser/web_contents/aura/overscroll_window_animation.cc:124:
DCHECK_EQ(overscroll_source_, OverscrollSource::NONE);
On 2017/07/20 17:22:37, mohsen wrote:
> Do you mean that this is not always true? If yes, when/why?

See the CL that I linked in the description. Basically it's possible for the
overlay to send OnOvercrollModeChange(NONE, WEST) via OverscrollWindowDelegate
while the "cancel" animation is in progress, then for the overlay to be
destroyed, and then for the OverscrollController to send the same
OnOvercrollModeChange. Then this DCHECK will trigger.

Obscure scenarios like this is one of the reasons it would be great to replace
this with a simplified version :)

[mohsen, 2017-07-20 18:14:26]

lgtm

https://codereview.chromium.org/2985543002/diff/1/content/browser/web_content...
File content/browser/web_contents/aura/overscroll_window_animation.cc (left):

https://codereview.chromium.org/2985543002/diff/1/content/browser/web_content...
content/browser/web_contents/aura/overscroll_window_animation.cc:124:
DCHECK_EQ(overscroll_source_, OverscrollSource::NONE);
On 2017/07/20 17:28:09, mfomitchev wrote:
> On 2017/07/20 17:22:37, mohsen wrote:
> > Do you mean that this is not always true? If yes, when/why?
> 
> See the CL that I linked in the description. Basically it's possible for the
> overlay to send OnOvercrollModeChange(NONE, WEST) via OverscrollWindowDelegate
> while the "cancel" animation is in progress, then for the overlay to be
> destroyed, and then for the OverscrollController to send the same
> OnOvercrollModeChange. Then this DCHECK will trigger.
> 
> Obscure scenarios like this is one of the reasons it would be great to replace
> this with a simplified version :)

Opps! Sorry, I missed the CL in the description!

[mfomitchev, 2017-07-20 18:39:35]

The CQ bit was checked by mfomitchev@chromium.org

[commit-bot: I haz the power, 2017-07-20 18:39:53]

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, 2017-07-20 18:47:15]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1500575975498530, "parent_rev":
"8f9efe1f0d7e7d5c1c186f31a52956e8e5155c65", "commit_rev":
"736822c161cce5c582e0d57b8957c68d683ba23e"}

[commit-bot: I haz the power, 2017-07-20 18:47:34]

Description was changed from

==========
Removing a DCHECK in OverscrollWindowAnimation::OnOverscrollModeChange

Proper fix is in https://codereview.chromium.org/2983883002/, but I'd rather not
land that while on leave. I will land the proper fix when I come back if this is
still not replaced with the new UI.

BUG=668296
==========

to

==========
Removing a DCHECK in OverscrollWindowAnimation::OnOverscrollModeChange

Proper fix is in https://codereview.chromium.org/2983883002/, but I'd rather not
land that while on leave. I will land the proper fix when I come back if this is
still not replaced with the new UI.

BUG=668296

Review-Url: https://codereview.chromium.org/2985543002
Cr-Commit-Position: refs/heads/master@{#488328}
Committed:
https://chromium.googlesource.com/chromium/src/+/736822c161cce5c582e0d57b8957...
==========

[commit-bot: I haz the power, 2017-07-20 18:47:35]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/736822c161cce5c582e0d57b8957...