Bug 39700 - Clean up MimeClassInfo and PluginInfo
Summary: Clean up MimeClassInfo and PluginInfo
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords:
Depends on: 39819
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-25 17:09 PDT by Anders Carlsson
Modified: 2010-05-27 02:29 PDT (History)
10 users (show)

See Also:


Attachments
Patch (30.27 KB, patch)
2010-05-25 17:15 PDT, Anders Carlsson
no flags Details | Formatted Diff | Diff
Patch (33.58 KB, patch)
2010-05-26 10:07 PDT, Anders Carlsson
no flags Details | Formatted Diff | Diff
Patch (35.25 KB, patch)
2010-05-26 11:32 PDT, Anders Carlsson
no flags Details | Formatted Diff | Diff
Patch (35.21 KB, patch)
2010-05-26 13:09 PDT, Anders Carlsson
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2010-05-25 17:09:33 PDT
Clean up MimeClassInfo and PluginInfo.
Comment 1 Anders Carlsson 2010-05-25 17:15:57 PDT
Created attachment 57055 [details]
Patch
Comment 2 WebKit Review Bot 2010-05-25 18:16:55 PDT
Attachment 57055 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2457005
Comment 3 Anders Carlsson 2010-05-26 10:07:47 PDT
Created attachment 57110 [details]
Patch
Comment 4 Anders Carlsson 2010-05-26 10:08:13 PDT
The new patch should fix the Chromium build.
Comment 5 WebKit Review Bot 2010-05-26 11:24:25 PDT
Attachment 57110 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2529033
Comment 6 Anders Carlsson 2010-05-26 11:32:39 PDT
Created attachment 57120 [details]
Patch
Comment 7 WebKit Review Bot 2010-05-26 12:59:15 PDT
Attachment 57120 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2500036
Comment 8 Anders Carlsson 2010-05-26 13:09:20 PDT
Created attachment 57129 [details]
Patch
Comment 9 Darin Adler 2010-05-26 13:50:38 PDT
Comment on attachment 57129 [details]
Patch

>  PluginData::~PluginData()
>  {
> -    deleteAllValues(m_plugins);
> -    deleteAllValues(m_mimes);
>  }

It'd be nice to just let the compiler generate this and not define it explicitly at all.
Comment 10 Anders Carlsson 2010-05-26 14:01:10 PDT
(In reply to comment #9)
> (From update of attachment 57129 [details])
> >  PluginData::~PluginData()
> >  {
> > -    deleteAllValues(m_plugins);
> > -    deleteAllValues(m_mimes);
> >  }
> 
> It'd be nice to just let the compiler generate this and not define it explicitly at all.

The destructor? Sure!
Comment 11 Anders Carlsson 2010-05-26 16:21:12 PDT
Committed r60258: <http://trac.webkit.org/changeset/60258>
Comment 12 WebKit Review Bot 2010-05-26 16:40:52 PDT
http://trac.webkit.org/changeset/60258 might have broken GTK Linux 32-bit Release and Qt Linux Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/60257
http://trac.webkit.org/changeset/60258
Comment 13 Julie Parent 2010-05-26 17:14:35 PDT
This looks to have introduced a crash in fast/dom/prototype-inheritance-2.html.

I see the crash on the following bots:
* Chromium (linux, win, mac)
* Leopard Intel Debug (Tests)
* Snow Leopard Intel Leaks
Comment 14 Darin Adler 2010-05-26 17:15:13 PDT
Is there a URL for the backtrace from the crash?
Comment 15 Anders Carlsson 2010-05-26 17:20:33 PDT
I think that the crash was caused by another change - looks like Geoff fixed it in http://trac.webkit.org/changeset/60261
Comment 16 Julie Parent 2010-05-26 17:30:31 PDT
Here is the Chromium linux bot stack:
	StackTrace::StackTrace() [0x80eeacf]
	base::(anonymous namespace)::StackDumpSignalHandler() [0x80de9da]
	0x4001c420
	WebCore::collectionIndexedPropertyEnumerator<>() [0x89648a8]
	v8::internal::GetKeysForIndexedInterceptor() [0x81ba92e]

Geoff's change was fixing a different crash.
Comment 17 Eric Seidel (no email) 2010-05-27 00:30:06 PDT
Looks like there may be new failures on Qt after this change?
http://build.webkit.org/results/Qt%20Linux%20Release/r60258%20(12428)/results.html
Comment 18 Csaba Osztrogonác 2010-05-27 00:40:41 PDT
(In reply to comment #17)
> Looks like there may be new failures on Qt after this change?
> http://build.webkit.org/results/Qt%20Linux%20Release/r60258%20(12428)/results.html

It seems this patch broke tests. I try to roll-out it locally now to confirm it.
Comment 19 Roland Steiner 2010-05-27 02:29:33 PDT
The crashing of fast/dom/prototype-inheritance-2.html also occurred during my WebKit gardening. I submitted and committed a quick fix in WebKit r60276.

See https://bugs.webkit.org/show_bug.cgi?id=39811