Issue metadata
Sign in to add a comment
|
Regression:Extension icon is seen blank for added extension.
Reported by
shruti.j...@etouch.net,
Jan 5 2018
|
||||||||||||||||||||||
Issue descriptionChrome Version: 65.0.3311.4 (Official Build) (cohort: 63_win_132) 76b6ee9763eaa0e64d5f8a879956e7e93f17b75a-refs/branch-heads/3311@{#6} (32/64-bit) OS: Win(7,8.1,10) Steps to reproduce: 1.Launch Chrome, go to chrome://apps. 2.Add extension 'grammarly for chrome' and observe. Actual Result: Extension icon is seen blank for added extension. Expected Result: Extension icon should be seen for added extension. This is regression issue broken in ‘M-65’ and below per-revision bisect result: Using the per-revision bisect providing the bisect results, Good Build: 65.0.3304.0(Revision:459686) Bad Build: 65.0.3305.0(Revision:460255) You are probably looking for a change made after 526175 (known good), but no later than 526176 (first known bad). CHANGELOG URL: The script might not always return single CL as suspect as some perf builds might get missing due to failure. https://chromium.googlesource.com/chromium/src/+log/ade0637ecfd1cb390e26c06d55a5480ad3bf6ac5..21c6dc80fae7f2767d73438883bab77f5afda583 Suspect: https://chromium.googlesource.com/chromium/src/+/21c6dc80fae7f2767d73438883bab77f5afda583 Note:Issue is not seen in MAC and Linux. Note:@ivafanas is not displayed in the owner list and hence assigning to the reviewer. Thank You!
,
Jan 5 2018
ivafanas, can you take a look? I don't have time to look into this (and haven't looked at extensions code in years, so my gut reaction would be to revert the change).
,
Jan 9 2018
finnur, sorry for the late response. Today is the first workday in Russia in 2018. I will try to fix the issue.
,
Jan 9 2018
Found the problem. Here: https://cs.chromium.org/chromium/src/extensions/browser/sandboxed_unpacker.cc?dr&l=866 path_suffix contains forwarding slashes, while install_icon_path contains backslashes. So equal paths are considered different. Will try to fix.
,
Jan 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/926bf293faabd120115fa5a105eb8dcc6eeadf0a commit 926bf293faabd120115fa5a105eb8dcc6eeadf0a Author: Ivan Afanasyev <ivafanas@yandex-team.ru> Date: Fri Jan 12 14:52:43 2018 Fix unpack the same icon twice on unpackers layer Previous fix tried to return unqiue normalized paths from ExtensionIconSet::GetPaths method. But external code uses assumption that ExtensionIconSet::GetPaths returns icons paths as they are written in extension manifest. This patchset reverts the previous one and tries to fix the issue on unpackers layer: 1. Unpacker::Run normalizes paths from ExtensionIconSet and removes duplicates. It writes normalized paths to kDecodedImagesFilename. 2. SandboxedUnpacker::RewriteImageFiles knows about normalization and does checks for extensions paths accordingly. This reverts commit 21c6dc80fae7f2767d73438883bab77f5afda583. Bug: 799418 Change-Id: I309cd8e54489cfde54c7676d34e15c829291e9cc Reviewed-on: https://chromium-review.googlesource.com/856579 Reviewed-by: Finnur Thorarinsson <finnur@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Commit-Queue: Finnur Thorarinsson <finnur@chromium.org> Cr-Commit-Position: refs/heads/master@{#528942} [modify] https://crrev.com/926bf293faabd120115fa5a105eb8dcc6eeadf0a/extensions/browser/sandboxed_unpacker.cc [modify] https://crrev.com/926bf293faabd120115fa5a105eb8dcc6eeadf0a/extensions/common/BUILD.gn [modify] https://crrev.com/926bf293faabd120115fa5a105eb8dcc6eeadf0a/extensions/common/extension_icon_set.cc [modify] https://crrev.com/926bf293faabd120115fa5a105eb8dcc6eeadf0a/extensions/common/extension_icon_set_unittest.cc [add] https://crrev.com/926bf293faabd120115fa5a105eb8dcc6eeadf0a/extensions/common/extension_resource_path_normalizer.cc [add] https://crrev.com/926bf293faabd120115fa5a105eb8dcc6eeadf0a/extensions/common/extension_resource_path_normalizer.h [add] https://crrev.com/926bf293faabd120115fa5a105eb8dcc6eeadf0a/extensions/common/extension_resource_path_normalizer_unittest.cc [modify] https://crrev.com/926bf293faabd120115fa5a105eb8dcc6eeadf0a/extensions/utility/unpacker.cc [modify] https://crrev.com/926bf293faabd120115fa5a105eb8dcc6eeadf0a/extensions/utility/unpacker.h
,
Jan 19 2018
Update : Verified this issue on Windows (7,8,8.1,10) OS with latest canary version #65.0.3325.0(Official build) and the issue works as intended. Kindly refer attached screen-cast. Thank You!
,
Jan 23 2018
finnur@, please mark it as 'Fixed' if there is no other pending CL exists. Thanks..!
,
Jan 23 2018
,
Jan 23 2018
[Auto-generated comment by a script] We noticed that this issue is targeted for M-65; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-65 label, otherwise remove Merge-TBD label. Thanks.
,
Jan 23 2018
The issue is assigned to me, but I have no idea whether a merge is required (I didn't work on this).
,
Jan 23 2018
Is my help required? I have no idea what is a merge and why it may be required. Ticket author marked the issue as found in 65 and verified in 65, so I'm not clear what to do.
,
Jan 23 2018
CL listed at #5 is already in M65 branch. No merge is needed. Hence, removing "Merge-TBD" label.
,
Feb 2 2018
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by ranjitkan@chromium.org
, Jan 5 2018