Issue metadata
Sign in to add a comment
|
ChromeOS's ProxyResolutionServiceProvider::ResolveProxy() needs to be converted for Network Service |
||||||||||||||||||||||||
Issue descriptionRepro: - Build simple chrome workflow with dcheck_always_on = true (https://chromium.googlesource.com/chromiumos/docs/+/master/simple_chrome_workflow.md#Debug-builds) - Deploy to chrome OS device - Log in. Browser will crash and you will be returned to the login screen. Check the logs in /home/chronos/user/log/chrome [28171:28171:0115/120629.693423:FATAL:storage_partition_impl.cc(772)] Check failed: false. Code in question: net::URLRequestContextGetter* StoragePartitionImpl::GetURLRequestContext() { // TODO(jam): enable for all, still used on WebView. // See copy of this ifdef in: // StoragePartitionImplMap::Get #if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_LINUX) if (base::FeatureList::IsEnabled(network::features::kNetworkService)) NOTREACHED(); #endif return url_request_context_.get(); } Making a P1 as we cannot run with dchecks on as is.
,
Jan 15
I don't know if you can repro it any other way. Here's the callstack: #0 0x55a3033e3dfc base::debug::StackTrace::StackTrace() #1 0x55a303148cc5 base::debug::StackTrace::StackTrace() #2 0x55a3033e381f base::debug::(anonymous namespace)::StackDumpSignalHandler() #3 0x7e5049996ab0 <unknown> #4 0x7e50484d8fb2 gsignal #5 0x7e50484dada1 abort #6 0x55a3033e2f96 base::debug::(anonymous namespace)::DebugBreak() #7 0x55a3033e2f78 base::debug::BreakDebugger() #8 0x55a3031a6491 logging::LogMessage::~LogMessage() #9 0x55a2fcea7e0c content::StoragePartitionImpl::GetURLRequestContext() #10 0x55a3028c5986 ProfileImpl::GetRequestContext() #11 0x55a2fef7e945 chromeos::ProxyResolutionServiceProvider::ResolveProxy() #12 0x55a2fef779df base::internal::FunctorTraits<>::Invoke<>() #13 0x55a2fef778f5 base::internal::InvokeHelper<>::MakeItSo<>() #14 0x55a2fef77860 _ZN4base8internal7InvokerINS0_9BindStateIMN8chromeos29ChromeFeaturesServiceProviderEFvPN4dbus10MethodCallENS_17RepeatingCallbackIFvNSt3__110unique_ptrINS5_8ResponseENS9_14default_deleteISB_EEEEEEEEJNS_7WeakPtrIS4_EEEEEFvS7_SG_EE7RunImplIRKSI_RKNS9_5tupleIJSK_EEEJLm0EEEEvOT_OT0_NS9_16integer_sequenceImJXspT1_EEEEOS7_OSG_ #15 0x55a2fef777c3 base::internal::Invoker<>::Run() #16 0x55a2fba8b43a _ZNKR4base17RepeatingCallbackIFvPN7content15RenderFrameHostEN4mojo16InterfaceRequestIN6device5mojom9VRServiceEEEEE3RunES3_S9_ #17 0x55a305e543da dbus::ExportedObject::RunMethod() #18 0x55a305e57d56 base::internal::FunctorTraits<>::Invoke<>() #19 0x55a305e57aea base::internal::InvokeHelper<>::MakeItSo<>() #20 0x55a305e57a52 _ZN4base8internal7InvokerINS0_9BindStateIMN4dbus14ExportedObjectEFvNS_17RepeatingCallbackIFvPNS3_10MethodCallENS5_IFvNSt3__110unique_ptrINS3_8ResponseENS8_14default_deleteISA_EEEEEEEEEENS9_IS6_NSB_IS6_EEEENS_9TimeTicksEEJ13scoped_refptrIS4_ESH_SJ_SK_EEEFvvEE7RunImplISM_NS8_5tupleIJSO_SH_SJ_SK_EEEJLm0ELm1ELm2ELm3EEEEvOT_OT0_NS8_16integer_sequenceImJXspT1_EEEE #21 0x55a305e578a9 base::internal::Invoker<>::RunOnce() #22 0x55a2f8b0101c _ZNO4base12OnceCallbackIFvvEE3RunEv #23 0x55a3031be858 base::debug::TaskAnnotator::RunTask() #24 0x55a3031bb993 base::MessageLoopImpl::RunTask() #25 0x55a3031bbfbe base::MessageLoopImpl::DeferOrRunPendingTask() #26 0x55a3031bca69 base::MessageLoopImpl::DoWork() #27 0x55a303430703 base::MessagePumpLibevent::Run() #28 0x55a3031bb0b3 base::MessageLoopImpl::Run() #29 0x55a3032482b2 base::RunLoop::Run() #30 0x55a302336385 ChromeBrowserMainParts::MainMessageLoopRun() #31 0x55a2fc025c01 content::BrowserMainLoop::RunMainMessageLoopParts() #32 0x55a2fc02df90 content::BrowserMainRunnerImpl::Run() #33 0x55a2fc018ae9 content::BrowserMain() #34 0x55a30230aa83 content::RunBrowserProcessMain() #35 0x55a30230e162 content::ContentMainRunnerImpl::RunServiceManager() #36 0x55a30230cc96 content::ContentMainRunnerImpl::Run() #37 0x55a302301f3c content::ContentServiceManagerMainDelegate::RunEmbedderProcess() #38 0x55a30232174a service_manager::Main() #39 0x55a302308433 content::ContentMain() #40 0x55a2f89db298 ChromeMain #41 0x55a2f89db182 main #42 0x7e50484c4ad4 __libc_start_main #43 0x55a2f89db02a _start
,
Jan 15
What commit are you building at? crrev.com/c/1359330 updated that check to not run on ChromeOS, but that landed over a month ago. Could you also please put the full stack trace here if there was one?
,
Jan 15
I'm building from ToT. Call stack is in comment #2.
,
Jan 15
Thanks for the stack trace! Please pull and try again. I just reverted a change that might be causing it.
,
Jan 15
crrev.com/c/1367088 took out the !defined(OS_CHROMEOS) added in http://crrev.com/c/1359330 fwiw
,
Jan 15
Yeh works building from ToT now - thanks for the fix!
,
Jan 15
Thanks Robbie. So the issue is that we weren't tracking the conversion of ProxyResolutionServiceProvider::ResolveProxy on ChromeOS right?
,
Jan 15
Yeah, I missed that call. I'll work on that this week.
,
Jan 15
,
Jan 15
Issue 922119 has been merged into this issue.
,
Jan 15
,
Jan 15
I am looking at this code as part of Issue 921479, so can take this conversion. Unless if you have already started, in which feel free to take back!
,
Jan 15
No, you can take this one.
,
Jan 15
,
Jan 15
Additional failures that are likely the same root cause here: issue 921479
,
Jan 15
Issue 922114 too?
,
Jan 16
922114 - The same root cause. Let's leave that as a separate bug though since it is about how to update the PFQ to pick up the fix (revert) which is already in. 921479 - This is also a separate bug AFAICT, as the initial bug report predates the Network Service having been enabled. The symptom is the same (a null pointer), however the cause appears to be different.
,
Jan 18
(5 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e5faa6d1e55641d69b75d6f6326cd242fd9b7f91 commit e5faa6d1e55641d69b75d6f6326cd242fd9b7f91 Author: Eric Roman <eroman@chromium.org> Date: Fri Jan 18 01:37:44 2019 Add browsertests for ProxyResolutionServiceProvider. Bug: 921814 Change-Id: Ia2ae35bb7bf4601318c672e1d1652bbd75e7713d Reviewed-on: https://chromium-review.googlesource.com/c/1418499 Commit-Queue: Eric Roman <eroman@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Cr-Commit-Position: refs/heads/master@{#623945} [modify] https://crrev.com/e5faa6d1e55641d69b75d6f6326cd242fd9b7f91/chrome/browser/chromeos/dbus/proxy_resolution_service_provider.cc [modify] https://crrev.com/e5faa6d1e55641d69b75d6f6326cd242fd9b7f91/chrome/browser/chromeos/dbus/proxy_resolution_service_provider.h [add] https://crrev.com/e5faa6d1e55641d69b75d6f6326cd242fd9b7f91/chrome/browser/chromeos/dbus/proxy_resolution_service_provider_browsertest.cc [modify] https://crrev.com/e5faa6d1e55641d69b75d6f6326cd242fd9b7f91/chrome/test/BUILD.gn [modify] https://crrev.com/e5faa6d1e55641d69b75d6f6326cd242fd9b7f91/testing/buildbot/filters/mojo.fyi.chromeos.network_browser_tests.filter
,
Jan 18
(4 days ago)
Issue 921479 has been merged into this issue.
,
Jan 19
(4 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1c82b994fe0e6796bed58384e287e293418e02f6 commit 1c82b994fe0e6796bed58384e287e293418e02f6 Author: Eric Roman <eroman@chromium.org> Date: Sat Jan 19 00:09:40 2019 Convert ProxyResolutionServiceProvider to use Network Service. This also updates the mojo ProxyLookupClient interface to include the network error code on failure, as the D-Bus services expects to have this error information. Bug: 921814 Change-Id: Ic1a240e72cffc7d8d708d150d139f32cfb7d1816 Reviewed-on: https://chromium-review.googlesource.com/c/1419264 Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Reviewed-by: Robbie McElrath <rmcelrath@chromium.org> Commit-Queue: Eric Roman <eroman@chromium.org> Cr-Commit-Position: refs/heads/master@{#624371} [modify] https://crrev.com/1c82b994fe0e6796bed58384e287e293418e02f6/chrome/browser/chromeos/dbus/proxy_resolution_service_provider.cc [modify] https://crrev.com/1c82b994fe0e6796bed58384e287e293418e02f6/chrome/browser/chromeos/dbus/proxy_resolution_service_provider.h [modify] https://crrev.com/1c82b994fe0e6796bed58384e287e293418e02f6/chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc [modify] https://crrev.com/1c82b994fe0e6796bed58384e287e293418e02f6/chrome/browser/predictors/preconnect_manager_unittest.cc [modify] https://crrev.com/1c82b994fe0e6796bed58384e287e293418e02f6/chrome/browser/predictors/proxy_lookup_client_impl.cc [modify] https://crrev.com/1c82b994fe0e6796bed58384e287e293418e02f6/chrome/browser/predictors/proxy_lookup_client_impl.h [modify] https://crrev.com/1c82b994fe0e6796bed58384e287e293418e02f6/content/browser/renderer_host/pepper/pepper_proxy_lookup_helper.cc [modify] https://crrev.com/1c82b994fe0e6796bed58384e287e293418e02f6/content/browser/renderer_host/pepper/pepper_proxy_lookup_helper_unittest.cc [modify] https://crrev.com/1c82b994fe0e6796bed58384e287e293418e02f6/content/browser/resolve_proxy_msg_helper.cc [modify] https://crrev.com/1c82b994fe0e6796bed58384e287e293418e02f6/content/browser/resolve_proxy_msg_helper.h [modify] https://crrev.com/1c82b994fe0e6796bed58384e287e293418e02f6/content/browser/resolve_proxy_msg_helper_unittest.cc [modify] https://crrev.com/1c82b994fe0e6796bed58384e287e293418e02f6/services/network/network_context_unittest.cc [modify] https://crrev.com/1c82b994fe0e6796bed58384e287e293418e02f6/services/network/proxy_lookup_request.cc [modify] https://crrev.com/1c82b994fe0e6796bed58384e287e293418e02f6/services/network/public/mojom/proxy_lookup_client.mojom [modify] https://crrev.com/1c82b994fe0e6796bed58384e287e293418e02f6/testing/buildbot/filters/mojo.fyi.chromeos.network_browser_tests.filter
,
Today
(11 hours ago)
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by jam@google.com
, Jan 15