New issue
Advanced search Search tips

Issue 836624 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Components:
EstimatedDays: ----
NextAction: 2018-05-03
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Crash from ExtensionPrefs when using --disable-extensions with unpacked extensions

Project Member Reported by falken@chromium.org, Apr 25 2018

Issue description

I think this is a DCHECK failure.

Chrome Version: ToT at 68.0.3404.0 (Developer Build) (64-bit)
OS: Linux

What steps will reproduce the problem?
(1) Start Chrome with a new profile (--user-data-dir=/tmp/blah).
(2) Go to chrome://extensions, load an unpacked extension like chrome/test/data/extensions/api_test/service_worker/background
(3) Restart Chrome with the same profile and command-line-flag --disable

What is the expected result?

Chrome starts up.

What happens instead?

Received signal 11 SEGV_MAPERR 000000000008
#0 0x7fecab3370ec base::debug::StackTrace::StackTrace()
#1 0x7fecab336c51 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#2 0x7fecab3a20c0 <unknown>
#3 0x7fecab331611 base::internal::flat_tree<>::equal_range<>()
#4 0x7fecab32d591 base::DictionaryValue::HasKey()
#5 0x56227d1c55ea extensions::Manifest::GetTypeFromManifestValue()
#6 0x56227c88074f extensions::ExtensionPrefs::InitPrefStore()
#7 0x56227c88156d extensions::ExtensionPrefs::ExtensionPrefs()
#8 0x56227c87a539 extensions::ExtensionPrefs::Create()
#9 0x56227c882e5f extensions::ExtensionPrefsFactory::BuildServiceInstanceFor()
#10 0x7feca6f52bd5 BrowserContextKeyedServiceFactory::BuildServiceInstanceFor()
#11 0x7feca74e3cb5 KeyedServiceFactory::GetServiceForContext()
#12 0x56227c86739f extensions::EventRouterFactory::BuildServiceInstanceFor()
#13 0x7feca6f52bd5 BrowserContextKeyedServiceFactory::BuildServiceInstanceFor()
#14 0x7feca74e3cb5 KeyedServiceFactory::GetServiceForContext()
#15 0x56227c91e406 extensions::CastChannelAPI::CastChannelAPI()
#16 0x56227c920a62 extensions::BrowserContextKeyedAPIFactory<>::BuildServiceInstanceFor()
#17 0x7feca6f52bd5 BrowserContextKeyedServiceFactory::BuildServiceInstanceFor()
#18 0x7feca74e3cb5 KeyedServiceFactory::GetServiceForContext()
#19 0x7feca74e3159 DependencyManager::CreateContextServices()
#20 0x7feca6f522ed BrowserContextDependencyManager::DoCreateBrowserContextServices()
#21 0x56227cc9239d ProfileImpl::OnLocaleReady()
#22 0x56227cc8f659 ProfileImpl::OnPrefsLoaded()
#23 0x56227cc8f44e ProfileImpl::ProfileImpl()
#24 0x56227cc8d8f9 Profile::CreateProfile()
#25 0x56227ccae655 ProfileManager::CreateProfileHelper()
#26 0x56227cca8897 ProfileManager::CreateAndInitializeProfile()
#27 0x56227cca85dc ProfileManager::GetProfile()
#28 0x56227d965e65 GetStartupProfile()
#29 0x56227cb926e0 ChromeBrowserMainParts::PreMainMessageLoopRunImpl()
#30 0x56227cb91e3a ChromeBrowserMainParts::PreMainMessageLoopRun()
#31 0x7feca8e57891 content::BrowserMainLoop::PreMainMessageLoopRun()
#32 0x7feca91ac6f7 content::StartupTaskRunner::RunAllTasksNow()
#33 0x7feca8e5645a content::BrowserMainLoop::CreateStartupTasks()
#34 0x7feca8e5a1a3 content::BrowserMainRunnerImpl::Initialize()
#35 0x7feca8e54044 content::BrowserMain()
#36 0x7feca95ebc48 content::ContentMainRunnerImpl::Run()
#37 0x7fecab76f494 service_manager::Main()
#38 0x7feca95e9d94 content::ContentMain()
#39 0x56227c7681b3 ChromeMain
#40 0x7feca05b92b1 __libc_start_main
#41 0x56227c76802a _start




Please use labels and text to provide additional information.

If this is a regression (i.e., worked before), please consider using the
bisect tool (https://www.chromium.org/developers/bisect-builds-py) to help
us identify the root cause and more rapidly triage the issue.

For graphics-related bugs, please copy/paste the contents of the about:gpu
page at the end of this report.


 

Comment 1 by falken@chromium.org, Apr 25 2018

Summary: Crash from ExtensionPrefs when using --disable-extensions with unpacked extensions (was: DCHECK failure from ExtensionPrefs when using --disable-extensions with unpacked extensions)
Hm, maybe it's not a DCHECK. It still crashes on my DCHECK-disabled build.
Owner: rdevlin....@chromium.org
Status: Assigned (was: Untriaged)
I added this, so probably my fault.  I'll investigate.
NextAction: 2018-04-30
(Setting a reminder to handle this on Monday)
The NextAction date has arrived: 2018-04-30
Project Member

Comment 5 by bugdroid1@chromium.org, May 1 2018

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

commit 8c23eeccfa70cb0735ca78e66d5cc74d83feb5fe
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Tue May 01 18:42:28 2018

[Extensions] Fix a crash in ExtensionPrefs

Revision efe0c88679a699f8876fbdff4face5b222d65790 added a check to
verify that extensions that would be loaded in all cases (in order
to fix a crash in ExtensionPrefValueMap). However, this depended on
knowing the type of extension, which was determined by the manifest
(stored in the extension preferences). Unpacked extensions do not
store the manifest, so this caused a crash.

As a workaround, gracefully handle the case of the extension having
no manifest. This is only safe because the only type of extension
that we would still load with extensions disabled is a theme, and
themes do not need entries in ExtensionPrefValueMap. This is a
pretty ugly/hacky solution, but should work to fix the crash.

Also add a regression browser test for the same.

Bug:  836624 

Change-Id: Ie7c766ceee261504b1d3f2cc570b7072e71e0f71
Reviewed-on: https://chromium-review.googlesource.com/1035448
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555123}
[modify] https://crrev.com/8c23eeccfa70cb0735ca78e66d5cc74d83feb5fe/chrome/browser/extensions/extension_service.cc
[add] https://crrev.com/8c23eeccfa70cb0735ca78e66d5cc74d83feb5fe/chrome/browser/extensions/extensions_disabled_browsertest.cc
[modify] https://crrev.com/8c23eeccfa70cb0735ca78e66d5cc74d83feb5fe/chrome/test/BUILD.gn
[add] https://crrev.com/8c23eeccfa70cb0735ca78e66d5cc74d83feb5fe/chrome/test/data/extensions/simple_with_key/manifest.json
[modify] https://crrev.com/8c23eeccfa70cb0735ca78e66d5cc74d83feb5fe/extensions/browser/extension_prefs.cc
[modify] https://crrev.com/8c23eeccfa70cb0735ca78e66d5cc74d83feb5fe/extensions/common/extension.h
[modify] https://crrev.com/8c23eeccfa70cb0735ca78e66d5cc74d83feb5fe/extensions/common/manifest.cc
[modify] https://crrev.com/8c23eeccfa70cb0735ca78e66d5cc74d83feb5fe/extensions/common/manifest.h

Labels: -Type-Bug -Pri-3 Merge-Request-67 M-67 OS-Chrome OS-Linux OS-Mac OS-Windows Pri-1 Type-Bug-Regression
NextAction: 2018-05-03
Status: Fixed (was: Assigned)
This should be fixed with #5.

Given this is a crash and a regression in M67, requesting merge (assuming no issues come up from bake time in Canary after 24 hours).
Pls update the bug with canary result on 05/03.Thank you.
Project Member

Comment 8 by sheriffbot@chromium.org, May 2 2018

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

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
The NextAction date has arrived: 2018-05-03
Labels: -Merge-Review-67 Merge-Approved-67
Approving merge to M67 branch 3396 based on comment #10. Please merge ASAP. Thank you.
Project Member

Comment 12 by bugdroid1@chromium.org, May 3 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/94c8b7a188e17c3181b15cba4d66c9479114832f

commit 94c8b7a188e17c3181b15cba4d66c9479114832f
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Thu May 03 19:21:56 2018

[Merge M67][Extensions] Fix a crash in ExtensionPrefs

Revision efe0c88679a699f8876fbdff4face5b222d65790 added a check to
verify that extensions that would be loaded in all cases (in order
to fix a crash in ExtensionPrefValueMap). However, this depended on
knowing the type of extension, which was determined by the manifest
(stored in the extension preferences). Unpacked extensions do not
store the manifest, so this caused a crash.

As a workaround, gracefully handle the case of the extension having
no manifest. This is only safe because the only type of extension
that we would still load with extensions disabled is a theme, and
themes do not need entries in ExtensionPrefValueMap. This is a
pretty ugly/hacky solution, but should work to fix the crash.

Also add a regression browser test for the same.

Bug:  836624 

TBR=rdevlin.cronin@chromium.org

(cherry picked from commit 8c23eeccfa70cb0735ca78e66d5cc74d83feb5fe)

Change-Id: Ie7c766ceee261504b1d3f2cc570b7072e71e0f71
Reviewed-on: https://chromium-review.googlesource.com/1035448
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#555123}
Reviewed-on: https://chromium-review.googlesource.com/1042870
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#464}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/94c8b7a188e17c3181b15cba4d66c9479114832f/chrome/browser/extensions/extension_service.cc
[add] https://crrev.com/94c8b7a188e17c3181b15cba4d66c9479114832f/chrome/browser/extensions/extensions_disabled_browsertest.cc
[modify] https://crrev.com/94c8b7a188e17c3181b15cba4d66c9479114832f/chrome/test/BUILD.gn
[add] https://crrev.com/94c8b7a188e17c3181b15cba4d66c9479114832f/chrome/test/data/extensions/simple_with_key/manifest.json
[modify] https://crrev.com/94c8b7a188e17c3181b15cba4d66c9479114832f/extensions/browser/extension_prefs.cc
[modify] https://crrev.com/94c8b7a188e17c3181b15cba4d66c9479114832f/extensions/common/extension.h
[modify] https://crrev.com/94c8b7a188e17c3181b15cba4d66c9479114832f/extensions/common/manifest.cc
[modify] https://crrev.com/94c8b7a188e17c3181b15cba4d66c9479114832f/extensions/common/manifest.h

Sign in to add a comment