We should support redirect handling for NPAPI plugins as specified in NPAPI version 26: https://wiki.mozilla.org/NPAPI:HTTPRedirectHandling
Created attachment 241447 [details] Patch
Created attachment 241454 [details] Patch
Attachment 241454 [details] did not pass style-queue: ERROR: Source/WebCore/plugins/npapi.h:871: Extra space between NPBool and NP_LOADDS [whitespace/declaration] [3] ERROR: Source/WebCore/plugins/npapi.h:872: The parameter name "direction" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/plugins/npapi.h:872: Extra space between NPBool and NP_LOADDS [whitespace/declaration] [3] ERROR: Source/WebCore/plugins/npapi.h:873: Extra space between void and NP_LOADDS [whitespace/declaration] [3] Total errors found: 4 in 44 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 241456 [details] Patch
Attachment 241456 [details] did not pass style-queue: ERROR: Source/WebCore/plugins/npapi.h:871: Extra space between NPBool and NP_LOADDS [whitespace/declaration] [3] ERROR: Source/WebCore/plugins/npapi.h:872: The parameter name "direction" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/plugins/npapi.h:872: Extra space between NPBool and NP_LOADDS [whitespace/declaration] [3] ERROR: Source/WebCore/plugins/npapi.h:873: Extra space between void and NP_LOADDS [whitespace/declaration] [3] Total errors found: 4 in 45 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 241460 [details] Patch
Attachment 241460 [details] did not pass style-queue: ERROR: Source/WebCore/plugins/npapi.h:871: Extra space between NPBool and NP_LOADDS [whitespace/declaration] [3] ERROR: Source/WebCore/plugins/npapi.h:872: The parameter name "direction" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/plugins/npapi.h:872: Extra space between NPBool and NP_LOADDS [whitespace/declaration] [3] ERROR: Source/WebCore/plugins/npapi.h:873: Extra space between void and NP_LOADDS [whitespace/declaration] [3] Total errors found: 4 in 45 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 241460 [details] Patch Couple of things: * I think you should split up the patch and land the loader parts before the plug-in support. * Why are no changes to ResourceHandle needed? It seems like asynchronous willSendRequest would require that. * I think you should just change willSendRequest and have it take an std::function instead of adding "maybeSendRequest". * Having ResourceRequest and bool seems strange, maybe you can use a WTF::Optional<ResourceRequest> instead.
Comment on attachment 241460 [details] Patch Attachment 241460 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6530079978946560 New failing tests: http/tests/plugins/get-url-redirect.html
Created attachment 241543 [details] Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-14 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 241911 [details] Patch
Comment on attachment 241543 [details] Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 This should fix all of the build and test issues, but I should probably still split this into two patches.
Attachment 241911 [details] did not pass style-queue: ERROR: Source/WebCore/plugins/npapi.h:871: Extra space between NPBool and NP_LOADDS [whitespace/declaration] [3] ERROR: Source/WebCore/plugins/npapi.h:872: The parameter name "direction" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/plugins/npapi.h:872: Extra space between NPBool and NP_LOADDS [whitespace/declaration] [3] ERROR: Source/WebCore/plugins/npapi.h:873: Extra space between void and NP_LOADDS [whitespace/declaration] [3] Total errors found: 4 in 46 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 242469 [details] Patch
Attachment 242469 [details] did not pass style-queue: ERROR: Source/WebCore/plugins/npapi.h:871: Extra space between NPBool and NP_LOADDS [whitespace/declaration] [3] ERROR: Source/WebCore/plugins/npapi.h:872: The parameter name "direction" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/plugins/npapi.h:872: Extra space between NPBool and NP_LOADDS [whitespace/declaration] [3] ERROR: Source/WebCore/plugins/npapi.h:873: Extra space between void and NP_LOADDS [whitespace/declaration] [3] Total errors found: 4 in 43 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 242469 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242469&action=review > Source/WebCore/loader/NetscapePlugInStreamLoader.cpp:70 > +void NetscapePlugInStreamLoader::willSendRequest(ResourceRequest&& request, const ResourceResponse& redirectResponse, std::function<void(ResourceRequest&)> callback) function parameter should be ResourceRequest&&. > Source/WebCore/loader/NetscapePlugInStreamLoader.cpp:76 > + if (request.isNull() || !protect->m_client) { > + request = ResourceRequest(); This is weird. Just pass an empty ResourceRequest to callback() instead. > Source/WebCore/loader/NetscapePlugInStreamLoader.h:41 > + virtual void willSendRequest(NetscapePlugInStreamLoader*, ResourceRequest&&, const ResourceResponse& redirectResponse, std::function<void(ResourceRequest&)> callback) = 0; I think "completionHandler" is a better name than callback. > Source/WebKit/mac/Plugins/Hosted/HostedNetscapePluginStream.mm:201 > + // FIXME: Call NPP_URLRedirectNotify Is there a bug number for this FIXME? > Source/WebKit2/WebProcess/Plugins/Netscape/NetscapeBrowserFuncs.cpp:1052 > + netscapeFuncs.urlredirectresponse = NPN_URLRedirectResponse; This should only be set if we're on an OS where this is supported. > Source/WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.cpp:407 > +void NetscapePlugin::registerRedirect(NetscapePluginStream* stream, const URL& requestURL, int redirectResponseStatus, void* notificationData) "registerRedirect" is a weird name. > Source/WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.cpp:420 > + auto pair = m_redirects.take(notifyData); This needs to handle invalid hash keys. Also, pair is a bad variable name, especially since you're using auto here. > Source/WebKit2/WebProcess/Plugins/Netscape/NetscapePluginStream.cpp:352 > +void NetscapePluginStream::redirect(const String& newURLString) > +{ > + m_requestURLString = newURLString; > +} This is just a setter. It should have a setter name. > Source/WebKit2/WebProcess/Plugins/PluginView.cpp:141 > + std::function<void(ResourceRequest&)> m_callback; This should be named better.
Created attachment 242550 [details] Patch
Attachment 242550 [details] did not pass style-queue: ERROR: Source/WebCore/plugins/PluginStream.cpp:406: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/plugins/npapi.h:871: Extra space between NPBool and NP_LOADDS [whitespace/declaration] [3] ERROR: Source/WebCore/plugins/npapi.h:872: The parameter name "direction" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/plugins/npapi.h:872: Extra space between NPBool and NP_LOADDS [whitespace/declaration] [3] ERROR: Source/WebCore/plugins/npapi.h:873: Extra space between void and NP_LOADDS [whitespace/declaration] [3] ERROR: Source/WebKit/mac/Plugins/Hosted/HostedNetscapePluginStream.h:64: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/mac/Plugins/Hosted/HostedNetscapePluginStream.mm:199: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/WebProcess/Plugins/PluginView.cpp:132: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/WebProcess/Plugins/PluginView.cpp:141: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/WebProcess/Plugins/PluginView.cpp:227: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/mac/Plugins/WebNetscapePluginStream.h:82: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/mac/Plugins/WebNetscapePluginStream.mm:300: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/plugins/PluginStream.h:79: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/loader/ResourceLoader.cpp:289: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/loader/NetscapePlugInStreamLoader.cpp:70: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/loader/ResourceLoader.h:96: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/loader/NetscapePlugInStreamLoader.h:41: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/loader/NetscapePlugInStreamLoader.h:60: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 18 in 46 files If any of these errors are false positives, please file a bug against check-webkit-style.
<rdar://problem/15779101>
*** Bug 49480 has been marked as a duplicate of this bug. ***
Created attachment 258133 [details] proposed patch Worked through how this patch works by re-writing many parts, and ended up with the same result. A few differences: - used rvalue references more; - removed a "cancel loaded" clause in WebResourceLoader that was added in r176626 without explanation, and shouldn't be needed; - performed a small refactoring in ResourceHandle to clean up "willSendRequest" variants.
Attachment 258133 [details] did not pass style-queue: ERROR: Source/WebKit/win/Plugins/PluginStream.cpp:406: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/plugins/npapi.h:871: Extra space between NPBool and NP_LOADDS [whitespace/declaration] [3] ERROR: Source/WebCore/plugins/npapi.h:872: The parameter name "direction" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/plugins/npapi.h:872: Extra space between NPBool and NP_LOADDS [whitespace/declaration] [3] ERROR: Source/WebCore/plugins/npapi.h:873: Extra space between void and NP_LOADDS [whitespace/declaration] [3] ERROR: Source/WebKit/mac/Plugins/Hosted/HostedNetscapePluginStream.h:89: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/mac/Plugins/Hosted/HostedNetscapePluginStream.mm:132: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/win/Plugins/PluginStream.h:85: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/WebProcess/Plugins/PluginView.cpp:132: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/WebProcess/Plugins/PluginView.cpp:141: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/WebProcess/Plugins/PluginView.cpp:224: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/mac/Plugins/WebNetscapePluginStream.h:100: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/mac/Plugins/WebNetscapePluginStream.mm:300: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/loader/NetscapePlugInStreamLoader.h:41: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 14 in 48 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 258133 [details] proposed patch Clearing flags on attachment: 258133 Committed r187886: <http://trac.webkit.org/changeset/187886>
All reviewed patches have been landed. Closing bug.
This patch broke the Windows build: ..\..\win\Plugins\PluginView.cpp(800): error C2248: 'WebCore::PluginStream::didReceiveResponse' : cannot access private member declared in class 'WebCore::PluginStream' [C:\cygwin\home\buildbot\slave\win-release\build\Source\WebKit\WebKit.vcxproj\WebKit\WebKit.vcxproj] c:\cygwin\home\buildbot\slave\win-release\build\source\webkit\win\plugins\PluginStream.h(86) : see declaration of 'WebCore::PluginStream::didReceiveResponse' c:\cygwin\home\buildbot\slave\win-release\build\source\webkit\win\plugins\PluginStream.h(57) : see declaration of 'WebCore::PluginStream' ..\..\win\Plugins\PluginView.cpp(811): error C2248: 'WebCore::PluginStream::didReceiveData' : cannot access private member declared in class 'WebCore::PluginStream' [C:\cygwin\home\buildbot\slave\win-release\build\Source\WebKit\WebKit.vcxproj\WebKit\WebKit.vcxproj] c:\cygwin\home\buildbot\slave\win-release\build\source\webkit\win\plugins\PluginStream.h(87) : see declaration of 'WebCore::PluginStream::didReceiveData' c:\cygwin\home\buildbot\slave\win-release\build\source\webkit\win\plugins\PluginStream.h(57) : see declaration of 'WebCore::PluginStream' ..\..\win\Plugins\PluginView.cpp(822): error C2248: 'WebCore::PluginStream::didFinishLoading' : cannot access private member declared in class 'WebCore::PluginStream' [C:\cygwin\home\buildbot\slave\win-release\build\Source\WebKit\WebKit.vcxproj\WebKit\WebKit.vcxproj] c:\cygwin\home\buildbot\slave\win-release\build\source\webkit\win\plugins\PluginStream.h(89) : see declaration of 'WebCore::PluginStream::didFinishLoading' c:\cygwin\home\buildbot\slave\win-release\build\source\webkit\win\plugins\PluginStream.h(57) : see declaration of 'WebCore::PluginStream' ..\..\win\Plugins\PluginView.cpp(833): error C2248: 'WebCore::PluginStream::didFail' : cannot access private member declared in class 'WebCore::PluginStream' [C:\cygwin\home\buildbot\slave\win-release\build\Source\WebKit\WebKit.vcxproj\WebKit\WebKit.vcxproj] c:\cygwin\home\buildbot\slave\win-release\build\source\webkit\win\plugins\PluginStream.h(88) : see declaration of 'WebCore::PluginStream::didFail' c:\cygwin\home\buildbot\slave\win-release\build\source\webkit\win\plugins\PluginStream.h(57) : see declaration of 'WebCore::PluginStream'
Fixed in http://trac.webkit.org/changeset/187894