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

Issue 755743 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: User zoom levels reset

Reported by d...@unabridgedsoftware.com, Aug 15 2017

Issue description

Chrome Version       : 61.0.3163.39 (Official Build) beta (64-bit)
OS                   : Linux (Ubuntu 16.04.03 LTS)
URLs (if applicable) : n/a
Other browsers tested: n/a

What steps will reproduce the problem?
(1) Upgrade Chrome beta from 60 to 61
(2) Restart browser

What is the expected result?
Previously set zoom levels per-website are persisted and automatically applied when re-visiting the sites.


What happens instead?
All website have the zoom set to the default of 100%. No previously set zoom level is used.


Please provide any additional information below. Attach a screenshot if
possible.
I think this is pretty self-explanatory and straightforward. This is obviously not a huge deal-breaker but it's a bit annoying to have happen.
 
Components: UI>Browser>Zoom
Labels: -Type-Bug -Pri-3 Hotlist-ConOps-Channel-Beta Hotlist-ConOps M-61 Hotlist-ConOps-Source-Forum OS-Linux Pri-1 Type-Bug-Regression

Comment 3 by gov...@chromium.org, Aug 16 2017

Cc: pbomm...@chromium.org ranjitkan@chromium.org nyerramilli@chromium.org
 ranjitkan@, pls try to repro and bisect if possible. Thank you.
Cc: gov...@chromium.org r...@opera.com
Labels: Needs-Triage-M61
Owner: patricia...@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce the issue using chrome version 61.0.3163.49 & Canary build 62.0.3187.0 on Ubuntu 14.04. Issue is broken in M61 and below are the bisect details for the same:

Bisect info:
============
61.0.3145.0 - Good build
61.0.3146.0 - Bad build

Change Log: https://chromium.googlesource.com/chromium/src/+log/61.0.3145.0..61.0.3146.0?pretty=fuller&n=10000

Unable to narrow down using bisect script as we need to update chrome over the existing saved settings.

Manually searching suspecting below changes could be a possible culprit:

https://chromium.googlesource.com/chromium/src/+/f3b6e00fe2826406218b15bd2f317cfebe835696 (or)
https://chromium.googlesource.com/chromium/src/+/7c44da721a59e6aa0b9fdcddb314175cb1e0123f

@patricialor: Assigning to you, kindly take a look into it. Please help us to find an owner if not with respect to your change. @rune: CC'ing you incase if it is related to your change. 

Issue is not observed on Windows and MAC OS.
Labels: ReleaseBlock-Stable
Adding blocker label, please remove if not the case.

Thanks.!

Comment 6 by r...@opera.com, Aug 17 2017

Almost certainly not mine.
Cc: tbansal@chromium.org
I'm not sure this is me, though I also don't think it's rune@'s change either.

It might have something to do with this patch?
https://chromium.googlesource.com/chromium/src/+/08a0e3ee7ad01c643155daf1c95a70954fe05c00
I could be wrong - tbansal@, any chance your change could have deleted some of the zoom prefs for whatever reason?
At first glance, it does not seem like it was because of my patch (linked in #7). My patch was not OS-specific or zoom-level specific.
Cc: thomasanderson@chromium.org
Labels: Needs-Bisect
+ pbommana, could you pls bisect this?
 thomasanderson@ ranjit from our team has already bisected the issue and posted the bisect results in comment#4.
[Bulk Edit]
URGENT - PTAL.
M61 Stable promotion is coming soon and your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP. 

Know that this issue shouldn't block the release?  Remove the ReleaseBlock-Stable label or move to M62. Thank you!

Note: We will only have 2 beta releases before Stable promotion. Plan is to cut M61 Stable RC on 08/31/17.

Cc: patricia...@chromium.org
Owner: thomasanderson@chromium.org
Status: Started (was: Assigned)
c#11: That bisect has many revisions.  Using tools/bisect_builds.py, I was able to bisect it down to just 9 revisions:
https://chromium.googlesource.com/chromium/src/+log/172d5dce95c34577fc45230fa8d5fd18f2143364..4c513d4ad2b4353ab41d3c7666c0bad7ae043379

Based on that, I'd guess this issue is caused by switching chrome to libc++ (my cl): 39fcd6af4d8c9ddcf909b8d489c6ba684d046157

I'll see if I can find a root cause
Labels: -Needs-Bisect
Thanks very much for doing a narrower bisect thomasanderson@!
Cc: wjmaclean@chromium.org thakis@chromium.org
Update:
Confirmed the symptom is caused by 39fcd6af4d8c9ddcf909b8d489c6ba684d046157

Details:
In our user preference file, the per-host-zoom-level is stored as a key-value pair.  The key is the hash of some string, calculated here:
https://cs.chromium.org/chromium/src/chrome/browser/ui/zoom/chrome_zoom_level_prefs.cc?rcl=e41aeaed051c8cc401e0bce2a6d57dcc330808e6&l=53

The hash function ends up plumbing into std::hash<std::string>().  Since the libstdc++ hash algorithm is different from the libc++ one, Chrome creates new zoom preferences for the new hash, effectively resetting the zoom preferences for all hosts.

There are some options:
(1) Change the key to be deterministic, which would fix the issue in the long run, but with a one-time reset of everyone's zoom preferences on all platforms.
(2) WontFix the issue, with this issue resurfacing whenever the standard library changes.
(3) Create a hack on each platform that copies the system behavior of std::hash<std::string>().

Unless anyone has any objections, I'm going to move forward with (1).
I agree that (1) seems the best ... but is it possible to make the fixed key the same as the old key, to perhaps minimize the number of people who see even one reset? Or has that boat already sailed ...
Cc: -tbansal@chromium.org
c#16 Now that you mention it, I think it is possible to preserve everyone's zoom settings.  On initialization, we could just check using both the new and the old keys, and if (# of elements for new key) < (# of elements for old key), then migrate the settings to the new key.

This should preserve Mac and Windows user's settings.  The Linux case would still be broken though, unless we try to mimic libstdc++'s std::hash<std::string>.

Lastly, we would remove the migration code after a few stable releases.  wdyt?
Sounds good. How hard would mimicing libstdc++'s thing for the migration code be?
The libstdc++ std::hash<std::string> impl uses their _Hash_bytes function, and the documentation for that says: 
  // The actual hash algorithm is not guaranteed to stay
  // the same from release to release -- it may be updated or tuned to
  // improve hash quality or speed.
https://cs.chromium.org/chromium/src/third_party/perl/c/lib/gcc/i686-w64-mingw32/4.6.3/include/c%2B%2B/bits/hash_bytes.h?q=%22hash+function+implementation+for+the+nontrivial%22&sq=package:chromium&l=41

So I probably can't just copy whatever the libstdc++ does on my machine into Chromium code.  Perhaps we could dlopen() libstdc++.so.6 and dlsym() _Hash_bytes, and if the library and symbol are available, try to use it?

Labels: Merge-Request-61
Requesting merge for https://crrev.com/b2be8c44f3383dc069d3b298b2d978d6592af08b
Project Member

Comment 22 by sheriffbot@chromium.org, Aug 24 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: We are only 11 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -ReleaseBlock-Stable
As per offline discussion with govind@, we agreed to pass on the M61 merge.  The change (https://crrev.com/b2be8c44f3383dc069d3b298b2d978d6592af08b) has not been tested in beta and the issue is not serious enough to warrant a merge of such a risky change.

Users will have to wait until M62 to see the fix.  There will not be a similar reset of zoom settings on the upgrade from M61 to M62, and per-site zoom settings from pre-M61 will even be retroactively restored when M62 is released.
Labels: -Merge-Review-61 Merge-Rejected-61
Rejecting merge to M61 based on comment #23.
Status: fie (was: Started)
Status: Fixed (was: fie)
Labels: TE-Verified-M62 TE-Verified-62.0.3196.0
Rechecked this issue on Chrome version 62.0.3196.0. Fix is working as intended. 

Installed first chrome version 62.0.3192.0 and from chrome://setting changed the page zoom level to 125%. Over installed Chrome version 62.0.3196.0 and navigated to chrome setting and observed that chrome retained the page zoom percentage.

Adding TE-Verified labels for M62.

Thanks.!

Sign in to add a comment