Handle NP_ASFILE and NP_ASFILEONLY transfer modes
Created attachment 61984 [details] Patch
Attachment 61984 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.h:71: NPP_StreamAsFile is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.cpp:193: NetscapePlugin::NPP_StreamAsFile is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 2 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 61984 [details] Patch > + return CString(temporaryFilePath.data()); Does the CString constructor really require a vector that already has the '\0' in it? Or does this code end up adding two of them? > + void NPP_StreamAsFile(NPStream* stream, const char* filename); Leave out the "stream" argument name? > + RefPtr<PluginView::Stream> stream = m_streams.get(streamID).get(); Here inside the class we can just call it Stream instead of PluginView::Stream.
(In reply to comment #3) > (From update of attachment 61984 [details]) > > + return CString(temporaryFilePath.data()); > > Does the CString constructor really require a vector that already has the '\0' in it? Or does this code end up adding two of them? > The constructor that takes a const char* requires it to be null terminated. The constructor that takes a length still adds a null terminator.
Comment on attachment 61984 [details] Patch > + // NPP_StreamAsFile could call NPN_DestroyStream and destroy the stream. > + if (!m_isStarted) > + return; > + return; > + } Does this really do what you want?
(In reply to comment #5) > (From update of attachment 61984 [details]) > > + // NPP_StreamAsFile could call NPN_DestroyStream and destroy the stream. > > + if (!m_isStarted) > > + return; > > + return; > > + } > > Does this really do what you want? Uh oh! Iām sure that at least one test will fail because of this!!!
Comment on attachment 61984 [details] Patch > --- a/WebCore/WebCore.xcodeproj/project.pbxproj > +++ b/WebCore/WebCore.xcodeproj/project.pbxproj Not mentioned in the change log. > +#import <wtf/text/CString.h> > #import "PlatformString.h" Not in ASCII order. > + // We failed to open the file, stop the stream. > + if (m_fileHandle == invalidPlatformFileHandle) > + stop(NPRES_NETWORK_ERR); Is it safer to return here? > + if (!m_isStarted) > + return; > + return; Too many of something.
Comment on attachment 61984 [details] Patch What everyone else said. r=me too!
Committed r63704: <http://trac.webkit.org/changeset/63704>