Bug 46825

Summary: build fails if NETSCAPE_PLUGIN_API is disabled
Product: WebKit Reporter: sccameron
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: andersca, joenotcharles
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Patch for Page.cpp andersca: review-, darin: commit-queue-

Description sccameron 2010-09-29 11:20:48 PDT
Created attachment 69228 [details]
Patch for Page.cpp

Calls to PluginView:: need to be wrapped in #if ENABLE(NETSCAPE_PLUGIN_API)]

This should identify the commit causing this error: 
   http://svn.webkit.org/repository/webkit/trunk@65707 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Comment 1 Anders Carlsson 2010-09-30 08:13:55 PDT
Comment on attachment 69228 [details]
Patch for Page.cpp

I think the #if ENABLE needs to go inside the function body, otherwise the code in Settings.cpp that tries to call it will fail.
Comment 2 Anders Carlsson 2010-09-30 08:42:13 PDT
Actually, I don't see how this would cause the build to fail. PluginView is still built even though NETSCAPE_PLUGIN_API is disabled.
Comment 3 sccameron 2010-10-01 06:52:11 PDT
(In reply to comment #2)
> Actually, I don't see how this would cause the build to fail. PluginView is still built even though NETSCAPE_PLUGIN_API is disabled.


The linker error is:
(Page.obj) : error LNK2001: unresolved external symbol "public: void __thiscall WebCore::PluginView::privateBrowsingStateChanged(bool)" (?privateBrowsingStateChanged@PluginView@WebCore@@QAEX_N@Z)

I'll post a new patch if you agree that it's a valid problem.

In addition, the following linker error occurs:
(ScriptDebugServer.obj) : error LNK2001: unresolved external symbol "public: void __thiscall WebCore::PluginView::setJavaScriptPaused(bool)"
(?setJavaScriptPaused@PluginView@WebCore@@QAEX_N@Z)


...which requires the following change:

--- a/WebCore/bindings/js/ScriptDebugServer.cpp
+++ b/WebCore/bindings/js/ScriptDebugServer.cpp
@@ -433,9 +433,7 @@ void ScriptDebugServer::setJavaScriptPaused(FrameView* view, bool paused)
         Widget* widget = (*it).get();
         if (!widget->isPluginView())
             continue;
+#if ENABLE(NETSCAPE_PLUGIN_API)
         static_cast<PluginView*>(widget)->setJavaScriptPaused(paused);
+#endif
     }
 }
Comment 4 Anders Carlsson 2010-10-01 09:14:04 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > Actually, I don't see how this would cause the build to fail. PluginView is still built even though NETSCAPE_PLUGIN_API is disabled.
> 
> 
> The linker error is:
> (Page.obj) : error LNK2001: unresolved external symbol "public: void __thiscall WebCore::PluginView::privateBrowsingStateChanged(bool)" (?privateBrowsingStateChanged@PluginView@WebCore@@QAEX_N@Z)
> 
> I'll post a new patch if you agree that it's a valid problem.
> 

Are you including the plugins/PluginViewNone.cpp file in your build? Not doing that would cause those two build errors.
Comment 5 sccameron 2010-10-01 09:26:26 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > Actually, I don't see how this would cause the build to fail. PluginView is still built even though NETSCAPE_PLUGIN_API is disabled.
> > 
> > 
> > The linker error is:
> > (Page.obj) : error LNK2001: unresolved external symbol "public: void __thiscall WebCore::PluginView::privateBrowsingStateChanged(bool)" (?privateBrowsingStateChanged@PluginView@WebCore@@QAEX_N@Z)
> > 
> > I'll post a new patch if you agree that it's a valid problem.
> > 
> 
> Are you including the plugins/PluginViewNone.cpp file in your build? Not doing that would cause those two build errors.

Yes it is.  WebCore.pro contains:

    contains(DEFINES, ENABLE_NETSCAPE_PLUGIN_API=1) {
    ...
    } else {
        SOURCES += \
            plugins/PluginPackageNone.cpp \
            plugins/PluginViewNone.cpp
    }

...and PluginViewNone.cpp is build in the logfile.
Comment 6 Joe Mason 2010-10-18 12:23:04 PDT
Aha, the problem is:

// The functions below are for platforms that do not use PluginView for plugins
// due to architectural differences. The plan is to eventually have all
// ports using PluginView, but until then, if new functions like this are
// added, please make sure they have the proper platform #ifs so that changes
// do not break ports who compile both this file and PluginView.cpp.
#if PLATFORM(MAC) || PLATFORM(CHROMIUM) || PLATFORM(EFL) || (OS(WINCE) && !PLATFORM(QT)) || (PLATFORM(QT) && !OS(WINCE)) || PLATFORM(BREWMP)
...
void PluginView::privateBrowsingStateChanged(bool)
{
}
...
#endif

We just need to add our port to that list.