Date: Mon, 24 Feb 2014 22:10:23 -0500 From: John Carr <jfc@MIT.EDU> To: chromium@FreeBSD.org Subject: Patch for Chromium settings crash Message-ID: <201402250310.s1P3ANZt027016@outgoing.mit.edu>
next in thread | raw e-mail | index | archive | help
------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <5887.1393297823.1@contents-vnder-pressvre.MIT.EDU>
If I select "Settings" then "Exit" from the menu at top right,
chrome crashes every time. Same with "History" instead of "Settings".
I am running FreeBSD 10.0 on amd64, chromium package 32.0.1700.107.
I started X with xinit, basically just starting a few xterms. Maybe
some service that chrome assumes is always running isn't running.
I assume this crash would have been noticed if it affected a standard
desktop environment.
I attached a patch, plus a second patch to make the build work in debug
mode so I could find the problem.
I'm sending this to the FreeBSD address because I can't tell if the
bug is in a porting change or the original code.
The debug build failure may be caused by a clang bug. An inline
function is not being found at final link. I made it not inline.
The stack trace below may explain the crash better than I can.
Note the null "this" pointer.
When the browser shuts down it calls a bunch of destructors.
The MediaGalleriesHandler destructor calls media_file_system_registry(),
There is no media file system registry, so it creates one.
The new MediaFileSystemRegistry tries to hook itself onto the global
StorageMonitor(), which has already been destroyed.
I don't understand the media gallery code, so I just added a test for
null pointer near the point of the crash.
(gdb) bt
#0 StorageMonitor::AddObserver (this=0x0, obs=0x81a335e50)
at ref_counted.h:260
#1 0x0000000000a058b6 in MediaFileSystemRegistry (this=0x81a335e50)
at ../../chrome/browser/media_galleries/media_file_system_registry.cc:594
#2 0x00000000009c7e1b in BrowserProcessImpl::media_file_system_registry (
this=0x81380d000) at ../../chrome/browser/browser_process_impl.cc:657
#3 0x0000000002d8c5fa in ~MediaGalleriesHandler (this=0x81a254040)
at ../../chrome/browser/ui/webui/options/media_galleries_handler.cc:34
#4 0x0000000002d8c4ee in ~MediaGalleriesHandler (this=0x81a254040)
at ../../chrome/browser/ui/webui/options/media_galleries_handler.cc:28
#5 0x0000000002f35e02 in ~ScopedVector (this=0x8165ae470) at stl_util.h:44
#6 0x0000000002f34528 in ~WebUIImpl (this=0x8165ae420) at scoped_vector.h:36
#7 0x0000000002f3448e in ~WebUIImpl (this=0x8165ae420)
at ../../content/browser/webui/web_ui_impl.cc:53
#8 0x0000000002cf4cdf in ~UberUI (this=0x81a03a130) at stl_util.h:152
#9 0x0000000002cf4c8e in ~UberUI (this=0x81a03a130)
at ../../chrome/browser/ui/webui/uber/uber_ui.cc:140
#10 0x0000000002f3450b in ~WebUIImpl (this=0x8165aeb00) at scoped_ptr.h:137
#11 0x0000000002f3448e in ~WebUIImpl (this=0x8165aeb00)
at ../../content/browser/webui/web_ui_impl.cc:53
#12 0x0000000002f1aa64 in ~WebContentsImpl (this=<value optimized out>)
at ../../content/browser/web_contents/web_contents_impl.cc:429
#13 0x0000000002f1a35e in ~WebContentsImpl (this=0x8139d8200)
------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="chrome-diff"; charset="us-ascii"
Content-ID: <5887.1393297823.2@contents-vnder-pressvre.MIT.EDU>
Content-Description: Check for null StorageMonitor
Content-Transfer-Encoding: quoted-printable
--- chrome/browser/media_galleries/media_file_system_registry.cc.orig 2014=
-02-03 15:15:11.000000000 -0500
+++ chrome/browser/media_galleries/media_file_system_registry.cc 2014-02-2=
4 20:57:03.060309366 -0500
@@ -591,7 +591,10 @@
// Constructor in 'private' section because depends on private class defi=
nition.
MediaFileSystemRegistry::MediaFileSystemRegistry()
: file_system_context_(new MediaFileSystemContextImpl(this)) {
- StorageMonitor::GetInstance()->AddObserver(this);
+ /* This conditional is needed for shutdown. Destructors
+ try to get the media file system registry. */
+ if (StorageMonitor::GetInstance())
+ StorageMonitor::GetInstance()->AddObserver(this);
}
=
MediaFileSystemRegistry::~MediaFileSystemRegistry() {
------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="chrome-webkit-diff"; charset="us-ascii"
Content-ID: <5887.1393297823.3@contents-vnder-pressvre.MIT.EDU>
Content-Description: Work around missing inline function
Content-Transfer-Encoding: quoted-printable
*** third_party/WebKit/Source/core/dom/Node.cpp.orig Mon Feb 3 15:19:16 2=
014
--- third_party/WebKit/Source/core/dom/Node.cpp Mon Feb 24 20:03:13 2014
***************
*** 120,125 ****
--- 120,130 ----
return DOMImplementation::hasFeature(feature, version);
}
=
+ bool Node::hasTagName(const QualifiedName& name) const
+ {
+ return isElementNode() && toElement(this)->hasTagName(name);
+ }
+ =
#if DUMP_NODE_STATISTICS
static HashSet<Node*> liveNodeSet;
#endif
*** third_party/WebKit/Source/core/dom/Element.h.orig Mon Feb 3 15:19:16 =
2014
--- third_party/WebKit/Source/core/dom/Element.h Mon Feb 24 20:03:15 2014
***************
*** 664,673 ****
--- 664,676 ----
return node->isElementNode() && toElement(node)->isDisabledFormContr=
ol();
}
=
+ /* This was inline, but being inline causes trouble with a debug build.
+ =
inline bool Node::hasTagName(const QualifiedName& name) const
{
return isElementNode() && toElement(this)->hasTagName(name);
}
+ */
=
inline Element* Node::parentElement() const
{
------- =_aaaaaaaaaa0--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201402250310.s1P3ANZt027016>