Bug 27940

Summary: Build fix if Netscape plugin support is turned off
Product: WebKit Reporter: Laszlo Gombos <laszlo.gombos>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, kenneth, vestbo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
proposed patch. eric: review+

Description Laszlo Gombos 2009-08-03 07:11:36 PDT
After r46649 and r46697 turning off Netscape plugin support makes the build fail.
Comment 1 Laszlo Gombos 2009-08-03 07:27:21 PDT
Created attachment 33974 [details]
proposed patch.

PluginView.cpp - Do not call NPN_MemFree if NPAPI is disabled
PluginViewNone.cpp - Add empty stub for platformStart()
Comment 2 Eric Seidel (no email) 2009-08-03 09:30:34 PDT
Comment on attachment 33974 [details]
proposed patch.

LGTM.
Comment 3 Kenneth Rohde Christiansen 2009-08-03 09:58:11 PDT
The patch contains trailing spaces. I will fix it when I land it
Comment 4 Kenneth Rohde Christiansen 2009-08-03 10:01:26 PDT
Landed in 46719.
Comment 5 Darin Adler 2009-08-03 10:02:39 PDT
Why do the functions in PluginViewNone.cpp have notImplemented() in them? I'd think they could just be empty function. The notImplemented() function is intended to alert the person working on the port that a particular non-implemented stub was hit. But in the case of "none" we don't ever plan to implement these. They should either be empty, or have ASSERT_NOT_REACHED() in them, or not exist at all, depending on our needs.
Comment 6 Kenneth Rohde Christiansen 2009-08-03 10:14:20 PDT
I think the reasoning was that ports use PluginViewNone when they still have not added support for plugins and thus, will run into a not implemented. If that is not the purpose of PluginViewNone, this ofcourse should be fixed.
Comment 7 Darin Adler 2009-08-03 10:16:27 PDT
(In reply to comment #6)
> I think the reasoning was that ports use PluginViewNone when they still have
> not added support for plugins and thus, will run into a not implemented. If
> that is not the purpose of PluginViewNone, this of course should be fixed.

OK. That makes sense. On the other hand, I could imagine a port that doesn't want plug-in support at all.
Comment 8 Kenneth Rohde Christiansen 2009-08-03 10:19:09 PDT
In that case I believe they should be able to disable the plugin support via the settings API.
Comment 9 Laszlo Gombos 2009-08-03 10:21:54 PDT
(In reply to comment #7)
> (In reply to comment #6)
> > I think the reasoning was that ports use PluginViewNone when they still have
> > not added support for plugins and thus, will run into a not implemented. If
> > that is not the purpose of PluginViewNone, this of course should be fixed.
> 
> OK. That makes sense. On the other hand, I could imagine a port that doesn't
> want plug-in support at all.

I think Darin has a valid point. i just used notImplemented() to stay
consistent with the rest of the file.

PluginViewNone.cpp was initially developed for the wx port only and I proposed
a patch to promote it to all ports to enable turning off NPAPI support.

I think notImplemented should go. I will create a patch.
Comment 10 Kenneth Rohde Christiansen 2009-08-03 10:32:51 PDT
If that is the case, I agree with you :-)

I talked with Kevin Ollivier on irc and he agrees with you Laszlo