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

Issue 719804 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression

Blocking:
issue 716110



Sign in to add a comment

Crash when changing orientation display

Project Member Reported by afakhry@chromium.org, May 9 2017

Issue description

Connect external display and from display settings attempt to change orientation to 270.

extensions::SystemDisplayFunction::ShouldRestrictToKioskAndWebUI() ()
    at /work2/chromium/.cros_cache/chrome-sdk/tarballs/kevin+9510.0.0+target_toolchain/usr/bin/../lib/gcc/armv7a-cros-linux-gnueabi/4.9.x/include/g++-v4/bits/unique_ptr.h:305
#1  0xace159c4 in extensions::SystemDisplayFunction::PreRunValidation(std::string*) () at ../../extensions/browser/api/system_display/system_display_api.cc:166
#2  0xacd342d0 in ExtensionFunction::RunWithValidation() () at ../../extensions/browser/extension_function.cc:453
#3  0xacd35c32 in extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal(ExtensionHostMsg_Request_Params const&, content::RenderFrameHost*, int, base::Callback<void (ExtensionFunction::ResponseType, base::ListValue const&, std::string const&, extensions::functions::HistogramValue), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) ()
    at ../../extensions/browser/extension_function_dispatcher.cc:454
#4  0xacd3592e in extensions::ExtensionFunctionDispatcher::Dispatch(ExtensionHostMsg_Request_Params const&, content::RenderFrameHost*, int) () at ../../extensions/browser/extension_function_dispatcher.cc:375
#5  0xacd4d85e in bool IPC::MessageT<ExtensionHostMsg_Request_Meta, std::tuple<ExtensionHostMsg_Request_Params>, void>::Dispatch<extensions::ExtensionWebContentsObserver, extensions::ExtensionWebContentsObserver, content::RenderFrameHost, void (extensions::ExtensionWebContentsObserver::*)(content::RenderFrameHost*, ExtensionHostMsg_Request_Params const&)>(IPC::Message const*, extensions::ExtensionWebContentsObserver*, extensions::ExtensionWebContentsObserver*, content::RenderFrameHost*, void (extensions::ExtensionWebContentsObserver::*)(content::RenderFrameHost*, ExtensionHostMsg_Request_Params const&)) ()
    at ../../ipc/ipc_message_templates.h:40
#6  0xacd4d72a in extensions::ExtensionWebContentsObserver::OnMessageReceived(IPC::Message const&, content::RenderFrameHost*) () at ../../extensions/browser/extension_web_contents_observer.cc:170
#7  0xaee39e88 in extensions::ChromeExtensionWebContentsObserver::OnMessageReceived(IPC::Message const&, content::RenderFrameHost*) ()
    at ../../chrome/browser/extensions/chrome_extension_web_contents_observer.cc:90
#8  0xacc251b8 in content::WebContentsImpl::OnMessageReceived(content::RenderFrameHostImpl*, IPC::Message const&) () at ../../content/browser/web_contents/web_contents_impl.cc:789
#9  0xaca7923e in content::RenderFrameHostImpl::OnMessageReceived(IPC::Message const&) () at ../../content/browser/frame_host/render_frame_host_impl.cc:762
#10 0xacb84c64 in content::RenderProcessHostImpl::OnMessageReceived(IPC::Message const&) () at ../../content/browser/renderer_host/render_process_host_impl.cc:2111
#11 0xadef9ae4 in IPC::ChannelProxy::Context::OnDispatchMessage(IPC::Message const&) () at ../../ipc/ipc_channel_proxy.cc:329
#12 0xadbb04ac in base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) () at ../../base/callback.h:91
#13 0xadb591b6 in base::MessageLoop::RunTask(base::PendingTask*) () at ../../base/message_loop/message_loop.cc:404
#14 0xadb5946e in base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) () at ../../base/message_loop/message_loop.cc:415
#15 0xadb59720 in base::MessageLoop::DoWork() () at ../../base/message_loop/message_loop.cc:503
#16 0xadb5a930 in base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) () at ../../base/message_loop/message_pump_libevent.cc:219
#17 0xadb71c72 in base::RunLoop::Run() () at ../../base/run_loop.cc:105
#18 0xad8982c6 in ChromeBrowserMainParts::MainMessageLoopRun(int*) () at ../../chrome/browser/chrome_browser_main.cc:1972
#19 0xac9ba678 in content::BrowserMainLoop::RunMainMessageLoopParts() () at ../../content/browser/browser_main_loop.cc:1182
#20 0xac9bc590 in content::BrowserMainRunnerImpl::Run() () at ../../content/browser/browser_main_runner.cc:141
#21 0xac9b710a in content::BrowserMain(content::MainFunctionParams const&) () at ../../content/browser/browser_main.cc:46
#22 0xad87ab10 in content::ContentMainRunnerImpl::Run() () at ../../content/app/content_main_runner.cc:705
#23 0xad890872 in service_manager::Main(service_manager::MainParams const&) () at ../../services/service_manager/embedder/main.cc:455
#24 0xad879eb0 in content::ContentMain(content::ContentMainParams const&) () at ../../content/app/content_main.cc:19
#25 0xac613784 in ChromeMain () at ../../chrome/app/chrome_main.cc:111
#26 0xf3d4f8b8 in __libc_start_main (main=0x0, argc=-745656, argv=0x0, init=<optimized out>, fini=0xb0f31239 <__libc_csu_fini>, rtld_fini=0xf43bff3d <_dl_fini>, stack_end=0xfff4a0a4) at libc-start.c:289
#27 0xac61362c in _start ()
 
Labels: ReleaseBlock-Beta
Status: Started (was: Assigned)
Cc: rdevlin....@chromium.org
Labels: -Type-Bug Type-Bug-Regression
My analysis shows that ExtensionFunctionDispatcher::CreateExtensionFunction() creates an ExtensionFunction with a nullptr extension! 

https://cs.chromium.org/chromium/src/extensions/browser/extension_function_dispatcher.cc?type=cs&q=ExtensionFunctionDispatcher::CreateExtensionFunction&l=588

This happened when ExtensionFunctionDispatcher::DispatchWithCallbackInternal() received params where 
params.extension_id is empty!

https://cs.chromium.org/chromium/src/extensions/browser/extension_function_dispatcher.cc?type=cs&q=ExtensionFunctionDispatcher::DispatchWithCallbackInternal&l=413

+rdevlin.cronin

This is a regression. I'm bisecting now.
By "display settings", do you mean the chrome://settings page?  If so, a null extension is expected and valid.  The ExtensionFunction::extension() member is only set when the request comes from an extension, and won't be present in the cases of coming from a website (including webui).  Extension functions that can be called from websites/webui should handle this case gracefully.
Cc: afakhry@chromium.org steve...@chromium.org
Owner: victorhsieh@chromium.org
Status: Assigned (was: Started)
Summary: Crash when changing orientation display (was: Crash when changing orientation of external display)
Yes, this CL: https://codereview.chromium.org/2862223003 caused the issue. 

The CL is being reverted.
Project Member

Comment 6 by bugdroid1@chromium.org, May 10 2017

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

commit 04b4874993cea9341a2d3446bcc486f52916477c
Author: afakhry <afakhry@chromium.org>
Date: Wed May 10 04:50:07 2017

Revert of Allow autotest extension to access system.display (patchset #1 id:1 of https://codereview.chromium.org/2862223003/ )

Reason for revert:
This causes a Chrome crash whenever you attempt to change anything in display settings. It assumes this is used from an extension.

Original issue's description:
> Allow autotest extension to access system.display
>
> system.display is currently only allowed for a few cases.  Grant
> autotest extension to use the API for test purpose, too.
>
> TEST=autotest works
> BUG= 712705 
>
> Review-Url: https://codereview.chromium.org/2862223003
> Cr-Commit-Position: refs/heads/master@{#470124}
> Committed: https://chromium.googlesource.com/chromium/src/+/84b76aa34b7ee89ea7b44af59239f6729aa53ab5

TBR=stevenjb@chromium.org,victorhsieh@google.com,victorhsieh@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 712705 , 719804 

Review-Url: https://codereview.chromium.org/2874633002
Cr-Commit-Position: refs/heads/master@{#470461}

[modify] https://crrev.com/04b4874993cea9341a2d3446bcc486f52916477c/extensions/browser/api/system_display/system_display_api.cc

Project Member

Comment 7 by bugdroid1@chromium.org, May 10 2017

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

commit 90e6006c41c0013b8f48c02263e3a44c32da0c9c
Author: victorhsieh <victorhsieh@chromium.org>
Date: Wed May 10 22:38:24 2017

Reland: Allow autotest extension to access system.display

system.display is currently only allowed for a few cases. Grant
autotest extension to use the API for test purpose, too.

TEST=Autotest works
TEST=Change display setting.  Did not crash with null check.
BUG= 719804 
BUG= 712705 

Review-Url: https://codereview.chromium.org/2874823002
Cr-Commit-Position: refs/heads/master@{#470737}

[modify] https://crrev.com/90e6006c41c0013b8f48c02263e3a44c32da0c9c/extensions/browser/api/system_display/system_display_api.cc

Status: Fixed (was: Assigned)
Should be fixed as the crash is not reproducible after the extra check.
Status: Verified (was: Fixed)
9563.0.0, 60.0.3102.0

Sign in to add a comment