New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 735774 link

Starred by 8 users
Status: Fixed
Owner: ----
Closed: Oct 2017
Cc:
ifratric@google.com
yosin@chromium.org
█ msrchandra@chromium.org
editing-dev@chromium.org
Components:
EstimatedDays: ----
NextAction: 2017-07-10
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocking:
issue 749321
issue 773220



Sign in to add a comment

VIsibleSelection ctor with word granularity should not yield start > end VisibleSelection

Project Member Reported by yosin@chromium.org, Jun 22 2017 
When we add CHECK_LE(start, end) in VisibleSelection::Validate(), we have
lots of crash report after expanding granularity.

When we don't find samples and fix it, we'll add swap start and end.

[1] http://crrev.com/2947613002: Add CHECK to VisbileSelection::Validate() to find where we get wrong positions

Comment 1 by yosin@chromium.org, Jun 22 2017

Cc: msrchandra@chromium.org yosin@chromium.org ifratric@google.com
 Issue 735437  has been merged into this issue.

Comment 2 by yosin@chromium.org, Jun 22 2017 

 Issue 734948  has been merged into this issue.
 Issue 734964  has been merged into this issue.
 Issue 735004  has been merged into this issue.
Issue 735042 has been merged into this issue.
 Issue 735064  has been merged into this issue.
 Issue 735326  has been merged into this issue.

Comment 3 by yosin@chromium.org, Jun 22 2017 

The  issue 735326  is caused by paragraph expansion yields same Text node, "xBxC\n"
where start at 1 and end is 0.
Project Member

Comment 4 by sheriffbot@chromium.org, Jun 22 2017

Labels: Fracas FoundIn-M-61
Users experienced this crash on the following builds:

Android Dev 61.0.3129.3 -  1.20 CPM, 50 reports, 26 clients (signature TextIteratorAlgorithm)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas
Project Member

Comment 5 by sheriffbot@chromium.org, Jun 22 2017

Labels: Fracas FoundIn-M-61
Users experienced this crash on the following builds:

Win Canary 61.0.3136.0 -  2.70 CPM, 29 reports, 28 clients (signature blink::VisibleSelectionTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> >::Validate)
Mac Canary 61.0.3136.0 -  1.89 CPM, 12 reports, 12 clients (signature blink::VisibleSelectionTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> >::Validate)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas
Project Member

Comment 6 by ClusterFuzz, Jun 22 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Available)
ClusterFuzz testcase 4657490305482752 is verified as fixed, so closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Comment 7 by yosin@chromium.org, Jun 30 2017

Status: Available (was: Verified)

Comment 8 by xiaoche...@chromium.org, Jun 30 2017 

Issue 734296 has been merged into this issue.

Comment 9 by monor...@bugs.chromium.org, Jul 10 2017 

The NextAction date has arrived: 2017-07-10

Comment 11 by gov...@chromium.org, Jul 11 2017 

A friendly reminder that M61 branch is coming soon on 07/20! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix ASAP to trunk. This way we branch M61 from a high quality trunk. Thank you.

Comment 12 by w...@chromium.org, Jul 15 2017

Labels: -OS-Fuchsia

Comment 15 by gov...@chromium.org, Jul 26 2017 

URGENT - PTAL.
Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the M61 branch #3163 ASAP to have enough baking time in Beta before Stable promotion. Thank you!

Know that this issue shouldn't block the release?  Remove the ReleaseBlock-Stable label.

Comment 17 by yoichio@chromium.org, Aug 2 2017

Owner: yosin@chromium.org

Comment 18 by yosin@chromium.org, Aug 2 2017

Labels: -ReleaseBlock-Stable
Owner: ----
Remove "ReleaseBlock-Stable" since crash is instances are small and "ReleaseBlock-Stable" is marked when we use CHECK() in VisibleSelection::Validate()

Comment 19 by yosin@chromium.org, Aug 2 2017 

To fix this bug, we need to revise StartOfWord()/EndOfWord()
Project Member

Comment 20 by ClusterFuzz, Sep 1 2017

Status: WontFix (was: Available)
ClusterFuzz testcase 6200372480966656 is flaky and no longer reproduces, so closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Comment 22 by rlanday@chromium.org, Sep 14 2017

Blocking: 749321

Comment 23 by aluo@chromium.org, Sep 14 2017

Labels: ReleaseBlock-Stable
This is #5 renderer crash on stable, marking as RBS

Comment 24 by amineer@chromium.org, Sep 15 2017

Labels: -M-61
Owner: yosin@chromium.org
Probably too late for M61, so will punt to M62.  I'm disappointed we untagged as RB even though aluo@ pointed out this was a #5 crash in dev, yosin@ why did that happen?

Comment 25 by infe...@chromium.org, Sep 18 2017

Labels: -ClusterFuzz-Wrong
We have made a bunch of changes on ClusterFuzz side, so resetting ClusterFuzz-Wrong label.
Project Member

Comment 26 by sheriffbot@chromium.org, Sep 21 2017

Labels: FoundIn-M-63
Users experienced this crash on the following builds:

Android Dev 63.0.3214.0 -  0.94 CPM, 32 reports, 17 clients (signature blink::TextIteratorAlgorithm<blink::EditingAlgorithm<blink::FlatTreeTraversal> >::TextIteratorAlgorithm)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

Comment 27 by yosin@chromium.org, Sep 22 2017

Owner: ----
Status: Available (was: Assigned)
Mark Available to move this issue into team working queue.
Project Member

Comment 28 by sheriffbot@chromium.org, Sep 22 2017

Labels: FoundIn-M-62
Users experienced this crash on the following builds:

Android Beta 62.0.3202.29 -  0.77 CPM, 13 reports, 12 clients (signature blink::TextIteratorAlgorithm<blink::EditingAlgorithm<blink::FlatTreeTraversal> >::TextIteratorAlgorithm)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

Comment 29 by rlanday@chromium.org, Sep 26 2017

Owner: rlanday@chromium.org

Comment 30 by rlanday@chromium.org, Sep 26 2017

Status: Assigned (was: Available)

Comment 31 by rlanday@chromium.org, Sep 28 2017 

Repro steps:

1. Go to omegle.com
2. Type in some interests people are unlikely to have in common with you
3. Tap "Start a chat"
4. While waiting to match with someone, long press "Type your message..."
Project Member

Comment 32 by bugdroid1@chromium.org, Sep 29 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/115ef964ad20ac081252d434b5f5219a3b64f892

commit 115ef964ad20ac081252d434b5f5219a3b64f892
Author: Ryan Landay <rlanday@chromium.org>
Date: Fri Sep 29 03:36:25 2017

Fix Android renderer crash when long-pressing certain markup

Long-pressing on a webpage in Android calls
SelectionController::SelectClosestWordFromHitTestResult(), which calls
CreateVisibleSelectionWithGranularity() to attempt to select the closest word.
In some cases, this creates an invalid range (which ends before it starts),
which causes a crash. This is currently the #3 top renderer crash in Chrome 61
for Android. This CL adds a check for an invalid selection range to avoid a
crash that we can merge into the M62 release. We should properly fix
CreateVisibleSelectionWithGranularity() at a later point to not return invalid
ranges.

Bug:  735774 
Change-Id: If035606403df9f3d13961e49dce80f0129b96318
Reviewed-on: https://chromium-review.googlesource.com/691060
Commit-Queue: Ryan Landay <rlanday@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505283}
[modify] https://crrev.com/115ef964ad20ac081252d434b5f5219a3b64f892/third_party/WebKit/Source/core/editing/SelectionController.cpp

Comment 33 by rlanday@chromium.org, Sep 29 2017

Labels: Merge-Request-62
Project Member

Comment 34 by sheriffbot@chromium.org, Sep 29 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: M62 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 35 by cma...@chromium.org, Sep 29 2017 

Has this issue been verified in M63?

Comment 36 by rlanday@chromium.org, Sep 29 2017 

The policy is it's supposed to be in Canary for 24 hours before a merge, right? I just installed the latest Chrome Canary, 63.0.3226.0, and it's not fixed there yet (checking the revision log, it looks like I missed that release by a few hours). So I think we need to wait for 24 hours after the next Canary release before doing a merge.

Comment 37 by cma...@chromium.org, Sep 29 2017 

Correct! It is always safe to verify the fix in canary before merging it into beta branch. There will be a new Canary build tonight so you should be able to merge this on Monday.

Comment 38 by rlanday@chromium.org, Oct 2 2017 

OmahaProxy says 63.0.3226.0 is still the latest Android Canary release, so I think we need to wait a little longer on this.

Comment 39 by abdulsyed@chromium.org, Oct 2 2017 

Can you verify the fix on Windows or Mac? Canary's version is 63.0.3230.0 for Desktop.

Comment 40 by rlanday@chromium.org, Oct 2 2017 

I don't think so; this is not nearly as high-firing on Mac or Windows as it is on Android. I can't even find it being reported on Windows in an M62 or M63 build.

Comment 41 by candr...@chromium.org, Oct 3 2017 

63.0.3231.0 was pushed to Canary this morning.  Please check.  Thanks.

Comment 42 by rlanday@chromium.org, Oct 3 2017 

We're going to need to wait for more crash reports. There are only 15 renderer crashes reported on that version so far.

Comment 44 by abdulsyed@chromium.org, Oct 4 2017

Labels: -Merge-Review-62 Merge-Approved-62
Approving merge for M62. Branch:3202
Project Member

Comment 45 by bugdroid1@chromium.org, Oct 4 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c102fec36bba291a97de912e4028ac4e03e6eb36

commit c102fec36bba291a97de912e4028ac4e03e6eb36
Author: Ryan Landay <rlanday@chromium.org>
Date: Wed Oct 04 22:32:51 2017

Fix Android renderer crash when long-pressing certain markup

Long-pressing on a webpage in Android calls
SelectionController::SelectClosestWordFromHitTestResult(), which calls
CreateVisibleSelectionWithGranularity() to attempt to select the closest word.
In some cases, this creates an invalid range (which ends before it starts),
which causes a crash. This is currently the #3 top renderer crash in Chrome 61
for Android. This CL adds a check for an invalid selection range to avoid a
crash that we can merge into the M62 release. We should properly fix
CreateVisibleSelectionWithGranularity() at a later point to not return invalid
ranges.

Bug:  735774 
Change-Id: If035606403df9f3d13961e49dce80f0129b96318
Reviewed-on: https://chromium-review.googlesource.com/691060
Commit-Queue: Ryan Landay <rlanday@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#505283}(cherry picked from commit 115ef964ad20ac081252d434b5f5219a3b64f892)
Reviewed-on: https://chromium-review.googlesource.com/701614
Reviewed-by: Ryan Landay <rlanday@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#581}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/c102fec36bba291a97de912e4028ac4e03e6eb36/third_party/WebKit/Source/core/editing/SelectionController.cpp

Comment 46 by rlanday@chromium.org, Oct 5 2017

Owner: yosin@chromium.org
The crash has been fixed for M62. Passing back to yosin@ to determine what to do about the underlying issue.

Comment 47 by candr...@chromium.org, Oct 9 2017 

Since the crash is fixed, can we close this one and file a separate bug for the other issue?

Comment 48 by cma...@chromium.org, Oct 9 2017 

 candrada: sounds good!

Comment 49 by yosin@chromium.org, Oct 10 2017

Blocking: 773220
Owner: ----
Status: Fixed (was: Assigned)
Mark Fixed per #c48.
We use the issue 773220 for tracking, since this bug isn't fixed yet.
►

Sign in to add a comment