New issue
Advanced search Search tips

Issue 860612 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Browser gets unresponsive and delay is observed after dragging a sample thumbnail in omnibox

Reported by shruti.j...@etouch.net, Jul 6

Issue description

Chrome Version :69.0.3483.0 (Official Build) (cohort: Stable)Revision	68bf9707c1026959630e89e90235cfcf0440c08c-refs/branch-heads/3483@{#1}(64-bit) 

OS: Windows(7,8,8.1,10)

Steps to reproduce:
1.Launch chrome and navigate to chrome://thumbnails.
2.Drag the thumbnail present on chrome://thumbnails to NTP.
2.Observe browser.
Actual Result:  Browser gets unresponsive and delay is observed in loading of thumbnail.
Expected Result: Browser should not get unresponsive and no delay should be seen while loading the thumbnail.

This is a regression issue, broken in 'M-69', and will soon provide bisect-info:
Good Build:69.0.3480.0 (Revision: 572082)
Bad Build: 69.0.3481.0 (Revision: 572437)

 
Actual_Resultt.mp4
598 KB View Download
Labels: RegressedIn-69 FoundIn-69 Target-69 hasbisect
Owner: msw@chromium.org
Status: Assigned (was: Unconfirmed)
Summary: Regression: Browser gets unresponsive and delay is observed after dragging a sample thumbnail in omnibox (was: Regression: Browser gets unresponsive and delay is observed in loading of thumbnail.)
Update:

Unable to bisect using per-revision bisect and unable to find the possible suspect in  narrow-bisect.

Narrow Bisect:

https://chromium.googlesource.com/chromium/src/+log/1fa3bfe11eee3a3f873153e95294636189954627..79c4dc9d0916a74430ce84c2d39d8360ca3a902a?pretty=fuller&n=10000

Suspecting: r572414 ?

Mike@: Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner

Note: 
1.Unable to bisect using per-revision bisect ,hence providing narrow-bisect.
2.Issue is not reproducible on MAC(10.12.6,10.13.1,10.13.6 and 10.14) and Linux(14.04 LTS)

Thank You!


Expected_Result.mp4
430 KB View Download
Cc: wangxianzhu@chromium.org
Owner: ccameron@chromium.org
My CL only affects ash_shell_with_content. Nothing in that range is too suspicious. Is the bisect accurate?
Assigning to ccameron for crrev.com/c/1123456
CC'ing wangxianzhu@ for crrev.com/c/1125300
This is indeed my patch -- we end up creating a cache entry for every character of a ~2k string.

Each cache entry has the full 2k string included. And, when looking up cache entries, we re-hash every entry every time.

Handful of fixes that will improve this
- store the result of hb_buffer_add_utf16 instead of the full buffer
- pre-compute the hash (and only hash the substr that is relevant)
- diff the hb_buffer_ts
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 11

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

commit 2228dce400e0743b6dd1f59205e03e0e2aaee145
Author: Christopher Cameron <ccameron@chromium.org>
Date: Wed Jul 11 06:50:34 2018

RenderTextHarfBuzz: Improve cache performance

Avoid checking the full input |text| string, rather, only examine the
part that sits inside |range| and its adjacent context. Note that we
still need the full string (or at least we need to know the offset
into it that we are using), because that will affect the output.

Pre-compute the input hash, to avoid re-computing a string's hash at
every comparison.

Add comments regarding key hash computation and thread use, and mark
members as const (requested in previous patch's review).

Bug:  860612 
Change-Id: I9dd73e0a2d0247a7139131edfb33b3d99138b103
Reviewed-on: https://chromium-review.googlesource.com/1130159
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574092}
[modify] https://crrev.com/2228dce400e0743b6dd1f59205e03e0e2aaee145/ui/gfx/render_text_harfbuzz.cc

Status: Fixed (was: Assigned)
This should be fixed now (can't verify on Windows myself due to lack of a machine).
Labels: TE-Verified-M69 TE-Verified-69.0.3489.0
Update:
Rechecked the above issue on Windows(7,8,8.1,10) using latest canary build #69.0.3489.0 and issue is fix.Browser does not get unresponsive after dragging sample thumbnails.
Please refer attached screencast for reference.

Thank You...
Canary_behaviour.mp4
392 KB View Download
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 13

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

commit 791f1c19acbf159aab8fb9652e95b77222075239
Author: Christopher Cameron <ccameron@chromium.org>
Date: Fri Jul 13 03:58:48 2018

MacViews: Only use context for text shape when using RTL languages

The call to hb_buffer_add_utf16 will read up to 5 characters adjacent
to the specified range. This context is only used in the function
arabic_joining.

We can get much better caching if we don't include this context in our
keys. Arabic is an RTL language, so we can omit this context in the key
for all non-RTL languages.

Bug:  860612 , 862773
Change-Id: Icc11059886ab835032141ad25b966d358f7177b3
Reviewed-on: https://chromium-review.googlesource.com/1135723
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574834}
[modify] https://crrev.com/791f1c19acbf159aab8fb9652e95b77222075239/ui/gfx/render_text_harfbuzz.cc

Labels: TE-Verified-69.0.3493.0
Update :
Rechecked the above issue on Windows(7,8,8.1,10)OS with latest Canary Chrome version : 69.0.3493.0 and the issue is Fixed.Kindly refer the attached screen cast for reference.
Fixed_video.mp4
312 KB View Download

Sign in to add a comment