Created attachment 53203 [details] Update WebCore/plugins/Plugin*None.cpp to new API Plugin API was updated but Plugin*None were not, leading to users of it to not link due unknown symbols. The following patch by my co-worker Leandro Pereira fixes it. We noticed so while changing bugs #35761 and #35762 to use Plugin*None as requested by Zecke. No runtime tests are requested.
Attachment 53203 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/1682241
Created attachment 53206 [details] Update WebCore/plugins/Plugin*None.cpp to new API, isolated to XP_UNIX and NETSCAPE_PLUGIN_API Isolates handleFocusInEvent and handleFocusOutEvent inside defined(XP_UNIX) && ENABLE(NETSCAPE_PLUGIN_API).
Created attachment 53241 [details] Update PluginViewNone and PluginPackageNone to the last Plugin API Updated patch adds a reference to this bug report on the ChangeLog entry.
Comment on attachment 53241 [details] Update PluginViewNone and PluginPackageNone to the last Plugin API r=me
Comment on attachment 53241 [details] Update PluginViewNone and PluginPackageNone to the last Plugin API Rejecting patch 53241 from commit-queue. Unexpected failure when landing patch! Please file a bug against webkit-patch. Failed to run "['WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', '53241', '--parent-command=commit-queue', '--no-update']" exit_code: 1 Logging in as eseidel@chromium.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=53241&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=37478&ctype=xml Processing 1 patch from 1 bug. Cleaning working directory Processing patch 53241 from bug 37478. WARNING: Creating invalid ChangeLogEntry: ERROR: /Users/eseidel/Projects/CommitQueue/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).
I'm not sure what went wrong for the commit-queue, I'll investigate.
Comment on attachment 53241 [details] Update PluginViewNone and PluginPackageNone to the last Plugin API Trying again?
I'll figure out what's going wrong and fix it this afternoon. Sorry again!
The patch is failing to apply correctly: diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog index 2a5dc84..1d287cb 100644 --- a/WebCore/ChangeLog +++ b/WebCore/ChangeLog @@ -1,4 +1,15 @@ 2010-04-13 Mikhail Naganov <mnaganov@chromium.org> +2010-04-13 Leandro Pereira <leandro@profusion.mobi> + + Reviewed by David Hyatt. + + Update WebCore/plugins/Plugin*None.cpp to conform to the + respective Plugin*.cpp API. + http://webkit.org/b/37478 + + * plugins/PluginPackageNone.cpp: Changed. + * plugins/PluginViewNone.cpp: Changed. + Reviewed by Pavel Feldman. This may be a svn-apply bug, or this may be a problem with the patch.
(In reply to comment #10) > The patch is failing to apply correctly: > > diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog > index 2a5dc84..1d287cb 100644 > --- a/WebCore/ChangeLog > +++ b/WebCore/ChangeLog > @@ -1,4 +1,15 @@ > 2010-04-13 Mikhail Naganov <mnaganov@chromium.org> > +2010-04-13 Leandro Pereira <leandro@profusion.mobi> > + It could have to do with the fact that the patch attachment's ChangeLog has no leading or ending context (the -1,0 means that the "before" chunk has 0 lines): diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog index 1de8c1e..e9a8738 100644 --- a/WebCore/ChangeLog +++ b/WebCore/ChangeLog @@ -1,0 +1,11 @@ +2010-04-12 Leandro Pereira <leandro@profusion.mobi> + + Reviewed by NOBODY (OOPS!). (from https://bugs.webkit.org/attachment.cgi?id=53241 )
Created attachment 53330 [details] Update PluginViewNone and PluginPackageNone to the last Plugin API This patch has been already r+'d by Hyatt, but was not created with the WebKit tools, so it didn't apply correctly. Here's an updated version that applies cleanly with svn-apply.
Comment on attachment 53330 [details] Update PluginViewNone and PluginPackageNone to the last Plugin API OK.
Comment on attachment 53330 [details] Update PluginViewNone and PluginPackageNone to the last Plugin API Rejecting patch 53330 from commit-queue. Failed to run "['WebKitTools/Scripts/build-webkit', '--debug']" exit_code: 1 Last 500 characters of output: ORT_DIR /Developer/Library/Xcode setenv XCODE_VERSION_ACTUAL 0313 setenv XCODE_VERSION_MAJOR 0300 setenv XCODE_VERSION_MINOR 0310 setenv YACC /Developer/usr/bin/yacc /bin/sh -c /Users/eseidel/Projects/CommitQueue/WebKitBuild/WebCore.build/Debug/WebCore.build/Script-5DF50887116F3077005202AB.sh ** BUILD FAILED ** The following build commands failed: WebCore: Ld /Users/eseidel/Projects/CommitQueue/WebKitBuild/Debug/WebCore.framework/Versions/A/WebCore normal i386 (1 failure) Full output: http://webkit-commit-queue.appspot.com/results/1602469
Undefined symbols: "__ZN7WebCore10PluginView19setJavaScriptPausedEb", referenced from: __ZN7WebCore17ScriptDebugServer19setJavaScriptPausedEPNS_9FrameViewEb in ScriptDebugServer.o "__ZN7WebCore10PluginView27privateBrowsingStateChangedEb", referenced from: __ZN7WebCore4Page27privateBrowsingStateChangedEv in Page.o "__ZN7WebCore10PluginView9keepAliveEP4_NPP", referenced from: __NPN_Evaluate in NP_jsobject.o ld: symbol(s) not found collect2: ld returned 1 exit status
You asked on IRC, so I took a brief look. For the first one issue It looks like PluginView's setJavaScriptPaused, which you change in this patch is actually used elsewhere: http://trac.webkit.org/browser/trunk/WebCore/bindings/js/ScriptDebugServer.cpp#L395 > -void PluginView::setJavaScriptPaused(bool) > +void PluginView::handleFocusOutEvent() Also, you can reverse the symbol using the c++filt command. Even though its normally pretty obvious sometimes this helps: > shell> c++filt __ZN7WebCore10PluginView19setJavaScriptPausedEb > WebCore::PluginView::setJavaScriptPaused(bool) > shell> c++filt __ZN7WebCore17ScriptDebugServer19setJavaScriptPausedEPNS_9FrameViewEb > WebCore::ScriptDebugServer::setJavaScriptPaused(WebCore::FrameView*, bool) Hopefully that gives you some leads.
Created attachment 53467 [details] Update PluginViewNone and PluginPackageNone to the last Plugin API This updated patch should build correctly under Mac.
Comment on attachment 53241 [details] Update PluginViewNone and PluginPackageNone to the last Plugin API Cleared David Hyatt's review+ from obsolete attachment 53241 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment on attachment 53330 [details] Update PluginViewNone and PluginPackageNone to the last Plugin API Cleared Eric Seidel's review+ from obsolete attachment 53330 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment on attachment 53467 [details] Update PluginViewNone and PluginPackageNone to the last Plugin API Clearing flags on attachment: 53467 Committed r57873: <http://trac.webkit.org/changeset/57873>
All reviewed patches have been landed. Closing bug.
Hi all, The applied patch still does not fix all the issues, if one links PluginView.cpp with PluginViewNone.cpp, it will not work since the following methods are defined in both: * void PluginPackage::determineQuirks(const String& mimeType); * void PluginView::keepAlive(NPP instance); * void PluginView::setJavaScriptPaused(bool paused); * void PluginView::privateBrowsingStateChanged(bool privateBrowsingEnabled); If we remove these from Plugin*None.cpp as we suggested before, it will not work on Mac/XCode as it will not link PluginView.cpp, just PluginViewNone.cpp and thus the error. I see two options: 1. Link Mac with PluginView.cpp, but needs someone from Mac to review; 2. ifdef these functions in PluginViewNone.cpp to just provide them if platform is MAC, or not XP_UNIX... I guess the first is more correct, but I'm not aware of the side effects.
(In reply to comment #22) > The applied patch still does not fix all the issues, if one links > PluginView.cpp with PluginViewNone.cpp, it will not work since the following > methods are defined in both: What port is trying to link both PluginView.cpp and PluginViewNone.cpp? That doesn't seem right. I think all ports should either: 1) Link PluginViewNone.cpp, or 2) Link PluginView.cpp plus PluginViewYourPort.cpp
(In reply to comment #23) > (In reply to comment #22) > > The applied patch still does not fix all the issues, if one links > > PluginView.cpp with PluginViewNone.cpp, it will not work since the following > > methods are defined in both: > > What port is trying to link both PluginView.cpp and PluginViewNone.cpp? That > doesn't seem right. I think all ports should either: > > 1) Link PluginViewNone.cpp, or > 2) Link PluginView.cpp plus PluginViewYourPort.cpp Well, after your help at IRC, I finally managed to fix it by using just PluginViewNone.cpp, not requiring PluginView.cpp. However I had to manually disable some code paths that would call Plugin* code, like the PluginDatabase and so on. Given this, I'm closing this bug. But it would be really good to have PluginView.cpp to be able to link with PluginViewNone.cpp and the methods would disable automatically.