Bug 93913 - Removing a plug-in element from a page opened in a background tab in Safari crashes
Summary: Removing a plug-in element from a page opened in a background tab in Safari c...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on: 95007
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-13 17:26 PDT by Brady Eidson
Modified: 2012-08-25 13:37 PDT (History)
6 users (show)

See Also:


Attachments
Patch v1 - Fix + regression test (9.28 KB, patch)
2012-08-13 17:31 PDT, Brady Eidson
buildbot: commit-queue-
Details | Formatted Diff | Diff
Patch v2 - Shot at fixing GTK (10.79 KB, patch)
2012-08-15 09:20 PDT, Brady Eidson
buildbot: commit-queue-
Details | Formatted Diff | Diff
Patch v3 - All the right exports in all the right places (11.60 KB, patch)
2012-08-15 10:59 PDT, Brady Eidson
bdakin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2012-08-13 17:26:01 PDT
Removing a plug-in element from a page opened in a background tab in Safari crashes

In Safari background-tab opened plug-ins aren't initialized until the tab is brought to the foreground.  When such a page removes a plug-in element, we crash:

#0	0x0000000101a1587c in WTF::RefPtr<CoreIPC::Connection>::get at /Volumes/SSD-Data/ZinUser/build/Debug/usr/local/include/wtf/RefPtr.h:58
#1	0x0000000101b18f4c in WebKit::PluginProcessConnection::connection at /Volumes/SSD-Data/git/OpenSource/Source/WebKit2/WebProcess/Plugins/PluginProcessConnection.h:54
#2	0x0000000101b2ccdd in WebKit::PluginProxy::destroy at /Volumes/SSD-Data/git/OpenSource/Source/WebKit2/WebProcess/Plugins/PluginProxy.cpp:202
#3	0x0000000101b012aa in WebKit::Plugin::destroyPlugin() ()
#4	0x0000000101b37c24 in WebKit::PluginView::~PluginView() ()

In this case the PluginProxy has a null m_connection.  A simple null check should suffice.

In radar as <rdar://problem/12057991>
Comment 1 Brady Eidson 2012-08-13 17:31:02 PDT
Created attachment 158157 [details]
Patch v1 - Fix + regression test
Comment 2 Brady Eidson 2012-08-13 18:00:13 PDT
Hmmmm I see there's a GTK failure already...

./.libs/libWebCoreInternals.a(./.libs/../Source/WebCore/testing/.libs/libWebCoreInternals_la-InternalSettings.o):InternalSettings.cpp:function WebCore::InternalSettings::Backup::restoreTo(WebCore::Page*, WebCore::Settings*): error: undefined reference to 'WebCore::Page::setCanStartMedia(bool)'
./.libs/libWebCoreInternals.a(./.libs/../Source/WebCore/testing/.libs/libWebCoreInternals_la-InternalSettings.o):InternalSettings.cpp:function WebCore::InternalSettings::setCanStartMedia(bool, int&): error: undefined reference to 'WebCore::Page::setCanStartMedia(bool)'
collect2: error: ld returned 1 exit status

As far as I can tell Page::setCanStartMedia() is defined in all ports.  Dunno what this is about yet.
Comment 3 Build Bot 2012-08-13 20:18:58 PDT
Comment on attachment 158157 [details]
Patch v1 - Fix + regression test

Attachment 158157 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13486729
Comment 4 Tim Horton 2012-08-13 20:20:15 PDT
(In reply to comment #2)
> As far as I can tell Page::setCanStartMedia() is defined in all ports.  Dunno what this is about yet.

The Windows error makes me wonder if it's not something to do with exports or something?
Comment 5 Brady Eidson 2012-08-14 08:32:00 PDT
(In reply to comment #4)
> (In reply to comment #2)
> > As far as I can tell Page::setCanStartMedia() is defined in all ports.  Dunno what this is about yet.
> 
> The Windows error makes me wonder if it's not something to do with exports or something?

Indeed the win and gtk failures are uncannily the same error...  I was under the (perhaps quite mistaken) impression that Mac was the only platform with explicit symbol exporting (WebCore.exp.in).

Grumble!
Comment 6 Brady Eidson 2012-08-15 09:20:59 PDT
Created attachment 158578 [details]
Patch v2 - Shot at fixing GTK
Comment 7 Build Bot 2012-08-15 09:51:47 PDT
Comment on attachment 158578 [details]
Patch v2 - Shot at fixing GTK

Attachment 158578 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13516028
Comment 8 Brady Eidson 2012-08-15 09:56:01 PDT
(In reply to comment #7)
> (From update of attachment 158578 [details])
> Attachment 158578 [details] did not pass win-ews (win):
> Output: http://queues.webkit.org/results/13516028

I think I have a response to the Win failure (WebKit2.def for exports) but will let the gtk ews play out on patch v2 first.
Comment 9 Brady Eidson 2012-08-15 10:59:11 PDT
Created attachment 158593 [details]
Patch v3 - All the right exports in all the right places
Comment 10 Brady Eidson 2012-08-15 11:12:48 PDT
Will land after win ews confirms it builds.
Comment 11 Brady Eidson 2012-08-15 12:34:01 PDT
http://trac.webkit.org/changeset/125695