[Webkit-unassigned] [Bug 17679] Shared PluginView implementation
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Mar 6 23:37:01 PST 2008
http://bugs.webkit.org/show_bug.cgi?id=17679
jhoneycutt at apple.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #19570|review? |review-
Flag| |
------- Comment #3 from jhoneycutt at apple.com 2008-03-06 23:36 PDT -------
(From update of attachment 19570)
Please avoid these few uses of platform-specific code in cross-platform code,
even if they're meant to be temporary. I think you should use platform-defined
helper functions like platformForceRedraw and platformInvalidateRect.
r- for build breakage and platform-dependent code in cross-platform code; more
comments below.
><html><body><pre style="word-wrap: break-word; white-space: pre-wrap;">Index: WebCore/ChangeLog
>===================================================================
>+ Move PluginmessageThrottler to a shared class.
Typo 'm.'
>Index: WebCore/plugins/PluginView.cpp
>===================================================================
>
>+#if PLATFORM(WIN)
> // Unsubclass the window
> if (m_isWindowed) {
>- WNDPROC currentWndProc = (WNDPROC)GetWindowLongPtr(m_window, GWLP_WNDPROC);
>-
>- if (currentWndProc == PluginViewWndProc)
>- SetWindowLongPtr(m_window, GWLP_WNDPROC, (LONG)m_pluginWndProc);
>+ unsetWndProc();
> }
Can remove the braces now that this is one line.
>- KJS::JSLock::DropAllLocks;
>+ KJS::JSLock::DropAllLocks dropAllLocks;
Good catch! You should mention this fix in the ChangeLog.
>+#if PLATFORM(WIN)
> // Get file info
> WIN32_FILE_ATTRIBUTE_DATA attrs;
> if (GetFileAttributesExW(filename.charactersWithNullTermination(), GetFileExInfoStandard, &attrs) == 0)
>@@ -1188,6 +782,7 @@ NPError PluginView::handlePost(const cha
>
> if (retval == 0 || bytesRead != attrs.nFileSizeLow)
> return NPERR_FILE_NOT_FOUND;
>+#endif
Can use SharedBuffer::createWithContentsOfFile here.
>Index: WebCore/plugins/PluginView.h
>===================================================================
>+ class PluginMessageThrottler {
I'm not sure of the utility of making this cross-platform. On Windows, this is
used exclusively for the Flash plug-in, which sends Windows WM_USER + 1
messages to its own window at a high enough rate to cause the app to stutter.
We subclass its window, intercept these messages, and dispatch them when
WebCore timers run.
Do you know if this will be needed by plug-ins on other platforms, and if their
"message" flood can both be handled asynchronously and be stored as an NPEvent?
If not, you should leave it Windows-only for now, and make it cross-platform as
needed. If so, you should at least change the name from PluginMessage to
PluginEvent.
>+ bool isEventUserGesture(NPEvent&);
This could be a static method taking a const ref.
>
>+#if PLATFORM(WIN)
>+ void setWndProc();
>+ void unsetWndProc();
>+ static LRESULT CALLBACK PluginViewWndProc(HWND, UINT, WPARAM, LPARAM);
>+
Please remove these and any other tab characters.
Windows build is currently broken here; you need to prefix the definition of
PluginViewWndProc with the class name in PluginViewWin.cpp or move this back
out of PluginView. If you want it to remain in PluginView, you should rename it
from PluginView::PluginViewWndProc to remove the redundancy.
>+ NPEvent npEvent = { message, wParam, lParam };
On Windows, NPEvent.event is a unsigned short. Windows messages are unsigned
int.
--
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
More information about the webkit-unassigned
mailing list