WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
37478
Update PluginViewNone and PluginPackageNone to the last Plugin API
https://bugs.webkit.org/show_bug.cgi?id=37478
Summary
Update PluginViewNone and PluginPackageNone to the last Plugin API
Gustavo Sverzut Barbieri
Reported
2010-04-12 17:18:05 PDT
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.
Attachments
Update WebCore/plugins/Plugin*None.cpp to new API
(1.34 KB, patch)
2010-04-12 17:18 PDT
,
Gustavo Sverzut Barbieri
no flags
Details
Formatted Diff
Diff
Update WebCore/plugins/Plugin*None.cpp to new API, isolated to XP_UNIX and NETSCAPE_PLUGIN_API
(1.40 KB, patch)
2010-04-12 17:52 PDT
,
Gustavo Sverzut Barbieri
no flags
Details
Formatted Diff
Diff
Update PluginViewNone and PluginPackageNone to the last Plugin API
(1.44 KB, patch)
2010-04-13 06:55 PDT
,
Leandro Pereira
no flags
Details
Formatted Diff
Diff
Update PluginViewNone and PluginPackageNone to the last Plugin API
(1.50 KB, patch)
2010-04-14 07:33 PDT
,
Leandro Pereira
no flags
Details
Formatted Diff
Diff
Update PluginViewNone and PluginPackageNone to the last Plugin API
(1.63 KB, patch)
2010-04-15 13:43 PDT
,
Leandro Pereira
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
WebKit Review Bot
Comment 1
2010-04-12 17:22:52 PDT
Attachment 53203
[details]
did not build on chromium: Build output:
http://webkit-commit-queue.appspot.com/results/1682241
Gustavo Sverzut Barbieri
Comment 2
2010-04-12 17:52:48 PDT
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).
Leandro Pereira
Comment 3
2010-04-13 06:55:13 PDT
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.
Dave Hyatt
Comment 4
2010-04-13 11:55:48 PDT
Comment on
attachment 53241
[details]
Update PluginViewNone and PluginPackageNone to the last Plugin API r=me
WebKit Commit Bot
Comment 5
2010-04-13 12:32:06 PDT
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).
Eric Seidel (no email)
Comment 6
2010-04-13 13:21:02 PDT
I'm not sure what went wrong for the commit-queue, I'll investigate.
Eric Seidel (no email)
Comment 7
2010-04-13 13:28:55 PDT
Comment on
attachment 53241
[details]
Update PluginViewNone and PluginPackageNone to the last Plugin API Trying again?
WebKit Commit Bot
Comment 8
2010-04-13 13:42:21 PDT
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).
Eric Seidel (no email)
Comment 9
2010-04-13 13:44:54 PDT
I'll figure out what's going wrong and fix it this afternoon. Sorry again!
Eric Seidel (no email)
Comment 10
2010-04-13 14:07:09 PDT
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.
Chris Jerdonek
Comment 11
2010-04-13 15:48:46 PDT
(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
)
Leandro Pereira
Comment 12
2010-04-14 07:33:33 PDT
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.
Eric Seidel (no email)
Comment 13
2010-04-14 09:07:34 PDT
Comment on
attachment 53330
[details]
Update PluginViewNone and PluginPackageNone to the last Plugin API OK.
WebKit Commit Bot
Comment 14
2010-04-14 09:41:02 PDT
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
Eric Seidel (no email)
Comment 15
2010-04-14 09:45:13 PDT
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
Joseph Pecoraro
Comment 16
2010-04-14 11:39:15 PDT
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.
Leandro Pereira
Comment 17
2010-04-15 13:43:20 PDT
Created
attachment 53467
[details]
Update PluginViewNone and PluginPackageNone to the last Plugin API This updated patch should build correctly under Mac.
Eric Seidel (no email)
Comment 18
2010-04-18 16:26:19 PDT
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
.
Eric Seidel (no email)
Comment 19
2010-04-18 16:26:31 PDT
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
.
WebKit Commit Bot
Comment 20
2010-04-19 23:30:10 PDT
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
>
WebKit Commit Bot
Comment 21
2010-04-19 23:30:18 PDT
All reviewed patches have been landed. Closing bug.
Gustavo Sverzut Barbieri
Comment 22
2010-04-20 07:39:03 PDT
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.
Adam Roben (:aroben)
Comment 23
2010-04-20 07:48:49 PDT
(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
Gustavo Sverzut Barbieri
Comment 24
2010-04-20 10:12:36 PDT
(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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug