Remove DeleteThemeDoNotUse |
||||||||||
Issue description(updated) After https://crrev.com/572255, the following crash happened. The change was reverted and crashes are no longer happening. But DeleteThemDoNotUse still can't be remvoed safely. reporter:emaxx@google.com Magic Signature: `anonymous namespace'::ShouldAllowInstall Crash link: https://crash.corp.google.com/browse?q=product_name%3D%27Chrome%27+AND+product.version%3D%2769.0.3488.0%27+AND+expanded_custom_data.ChromeCrashProto.ptype%3D%27browser%27+AND+expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27%60anonymous+namespace%5C%27%3A%3AShouldAllowInstall%27&stbtiq=&reportid=&index=0 ------------------------------------------------------------------------------- Sample Report ------------------------------------------------------------------------------- Product name: Chrome Magic Signature : `anonymous namespace'::ShouldAllowInstall Product Version: 69.0.3488.0 Process type: browser Report ID: 22e2aab9a352f4ee Report Url: https://crash.corp.google.com/22e2aab9a352f4ee Report Time: 2018-07-11T09:37:03-07:00 Upload Time: 2018-07-11T09:37:22.293-07:00 Uptime: 90000 ms OS Name: Windows NT OS Version: 10.0.16299 431 CPU Architecture: amd64 CPU Info: family 6 model 42 stepping 7 ------------------------------------------------------------------------------- Crashing thread: Thread index: 0. Stack Quality: 100%. Thread id: 11284. ------------------------------------------------------------------------------- 0x00007ff830f4f5f2 (chrome.dll - extension_sync_service.cc: 63) `anonymous namespace'::ShouldAllowInstall 0x00007ff830b30a55 (chrome.dll - extension_service.cc: 1400) extensions::ExtensionService::OnExtensionInstalled(extensions::Extension const *,syncer::Ordinal<syncer::StringOrdinalTraits> const &,int,base::Optional<int> const &) 0x00007ff830f49b0c (chrome.dll - crx_installer.cc: 948) extensions::CrxInstaller::ReportSuccessFromUIThread() 0x00007ff82f5a5c8b (chrome.dll - task_annotator.cc: 101) base::debug::TaskAnnotator::RunTask(char const *,base::PendingTask *) 0x00007ff82f5a564b (chrome.dll - message_loop.cc: 357) base::MessageLoop::RunTask(base::PendingTask *) 0x00007ff82f59d801 (chrome.dll - message_loop.cc: 425) base::MessageLoop::DoWork() 0x00007ff82f69dac8 (chrome.dll - message_pump_win.cc: 177) base::MessagePumpForUI::DoRunLoop() 0x00007ff82f59d437 (chrome.dll - message_pump_win.cc: 56) base::MessagePumpWin::Run(base::MessagePump::Delegate *) 0x00007ff82f59d180 (chrome.dll - run_loop.cc: 102) base::RunLoop::Run() 0x00007ff82f98f6ad (chrome.dll - chrome_browser_main.cc: 2063) ChromeBrowserMainParts::MainMessageLoopRun(int *) 0x00007ff82f98f4b1 (chrome.dll - browser_main_loop.cc: 1015) content::BrowserMainLoop::RunMainMessageLoopParts() 0x00007ff82f98f45c (chrome.dll - browser_main_runner_impl.cc: 169) content::BrowserMainRunnerImpl::Run() 0x00007ff82f59e444 (chrome.dll - browser_main.cc: 51) content::BrowserMain(content::MainFunctionParams const &,std::unique_ptr<content::BrowserProcessSubThread,std::default_delete<content::BrowserProcessSubThread> >) 0x00007ff82f59e2da (chrome.dll - content_main_runner_impl.cc: 600) content::RunBrowserProcessMain(content::MainFunctionParams const &,content::ContentMainDelegate *,std::unique_ptr<content::BrowserProcessSubThread,std::default_delete<content::BrowserProcessSubThread> >) 0x00007ff82f599925 (chrome.dll - content_main_runner_impl.cc: 947) content::ContentMainRunnerImpl::Run() 0x00007ff82f585bbf (chrome.dll - main.cc: 459) service_manager::Main(service_manager::MainParams const &) 0x00007ff82f585647 (chrome.dll - content_main.cc: 19) content::ContentMain(content::ContentMainParams const &) 0x00007ff82f581e59 (chrome.dll - chrome_main.cc: 101) ChromeMain 0x00007ff70f97372b (chrome.exe - main_dll_loader_win.cc: 201) MainDllLoader::Launch(HINSTANCE__ *,base::TimeTicks) 0x00007ff70f971698 (chrome.exe - chrome_exe_main_win.cc: 230) wWinMain 0x00007ff70fa34891 (chrome.exe - exe_common.inl: 283) __scrt_common_main_seh 0x00007ff8775f1fe3 (KERNEL32.DLL + 0x00011fe3) BaseThreadInitThunk 0x00007ff8779ef060 (ntdll.dll + 0x0006f060) RtlUserThreadStart ------------------------------------------------------------------------------- Manual regression range finder link ------------------------------------------------------------------------------- https://crash.corp.google.com/browse?q=expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27%60anonymous+namespace%5C%27%3A%3AShouldAllowInstall%27+AND+expanded_custom_data.ChromeCrashProto.ptype%3D%27browser%27#-property-selector,-samplereports,+productname,+productversion:1000,+directory,-clientid,+operatingsystem,+url,+simplifiedurl,+extensions
,
Jul 12
treib@ seems to be OOO. Reassigning to Devlin - should we revert r572255, or you'd suspect the issue rooting somewhere else?
,
Jul 12
Just to note, this is currently a top-#1 crasher on Windows Canary 69.0.3489.0 (4 reports from 4 different clients). There are also crashes observed on Mac.
,
Jul 13
[stability sheriff] This is the #2 crasher on 69.0.3489.0 canary. The crash is indeed at // Note: In the past, there was a bug where some themes incorrectly ended up // here, see https://crbug.com/558299. Make sure that doesn't happen anymore. CHECK(!extension->is_theme()); Which was added in r572255 in 69.0.3481.0. devlin or treib: Can you revert or fix this before the M69 branch cut next week? Crashes per version: 69.0.3489.1 7.14% 2 69.0.3489.0 57.14% 16 69.0.3488.0 28.57% 8 69.0.3486.0 7.14% 2
,
Jul 13
Yep, we should revert and investigate further, since clearly the issue isn't resolved. I'll revert now.
,
Jul 13
Reverted in revision ec30d368ed2cbcbcb55171b5d36918bd79e1cee8. Over to Marc for more investigation when he comes back.
,
Jul 16
Thanks Devlin for handling the revert in my absence! Relevant context: bug 558299 I'll update the "clean this up" comment to refer to this bug. Throwing back into the available pool, since I don't know when I'll find more time to burn on this.
,
Jul 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dfbb6cbe090f5adfea6537b5a22f1cb88f8cb74c commit dfbb6cbe090f5adfea6537b5a22f1cb88f8cb74c Author: Marc Treib <treib@chromium.org> Date: Mon Jul 16 14:55:06 2018 Update comment on ExtensionSyncService::DeleteThemeDoNotUse Bug: 862665 Change-Id: I5116783c22c7dc02ef057aa654528dfeaa3366fb Reviewed-on: https://chromium-review.googlesource.com/1138153 Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Commit-Queue: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#575262} [modify] https://crrev.com/dfbb6cbe090f5adfea6537b5a22f1cb88f8cb74c/chrome/browser/extensions/extension_sync_service.h
,
Jul 17
[stability sheriff] Thanks for the revert. It landed in 69.0.3491.0. There are (unsurprisingly) no crashes since then. Retitling/relabeling.
,
Jul 17
,
Jan 7
Crash analysis has not encountered any reports for this bug for the past 90 days. We have added the label 'crash-BugNoRepro' Crash analysis will be automatically closing the bug in 6 days. If you do not want Crash analysis to automatically close the bug, please remove the label 'crash-BugNoRepro'. If you have any feedback on this feature, please contact pranavk@
,
Jan 7
Removing Crash labels but keeping this open because there is still the following code[1]: // TODO(crbug.com/862665): This *should* be safe to remove now, but it's not. void DeleteThemeDoNotUse(const extensions::Extension& theme); [1] https://cs.chromium.org/chromium/src/chrome/browser/extensions/extension_sync_service.h?l=73&rcl=f4608cc6cd9815a1628febe28ef18e2f67871488 |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by emaxx@chromium.org
, Jul 11Owner: treib@chromium.org
Status: Assigned (was: Untriaged)