Bug 182748

Summary: Allow specifying which plug-ins are supported
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebCore Misc.Assignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, aestes, cdumez, commit-queue, dbates, ews-watchlist, mitz, rniwa, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Archive of layout-test-results from ews104 for mac-sierra-wk2
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch for landing
none
Patch for landing
none
Patch for landing none

youenn fablet
Reported 2018-02-13 14:47:44 PST
Allow specifying which plug-ins are supported
Attachments
Patch (75.80 KB, patch)
2018-02-13 16:07 PST, youenn fablet
no flags
Patch (78.29 KB, patch)
2018-02-13 16:53 PST, youenn fablet
no flags
Patch (78.90 KB, patch)
2018-02-13 18:08 PST, youenn fablet
no flags
Patch (78.81 KB, patch)
2018-02-13 18:25 PST, youenn fablet
no flags
Archive of layout-test-results from ews104 for mac-sierra-wk2 (3.40 MB, application/zip)
2018-02-13 21:49 PST, EWS Watchlist
no flags
Patch (75.82 KB, patch)
2018-02-14 13:52 PST, youenn fablet
no flags
Patch (76.07 KB, patch)
2018-02-14 14:08 PST, youenn fablet
no flags
Patch (76.20 KB, patch)
2018-02-14 14:24 PST, youenn fablet
no flags
Patch (79.02 KB, patch)
2018-02-14 15:28 PST, youenn fablet
no flags
Patch (79.39 KB, patch)
2018-02-14 16:37 PST, youenn fablet
no flags
Patch (79.41 KB, patch)
2018-02-14 18:35 PST, youenn fablet
no flags
Patch (79.40 KB, patch)
2018-02-15 09:59 PST, youenn fablet
no flags
Patch for landing (79.75 KB, patch)
2018-02-16 13:25 PST, youenn fablet
no flags
Patch for landing (79.75 KB, patch)
2018-02-16 14:13 PST, youenn fablet
no flags
Patch for landing (79.55 KB, patch)
2018-02-16 14:24 PST, youenn fablet
no flags
youenn fablet
Comment 1 2018-02-13 16:07:07 PST
youenn fablet
Comment 2 2018-02-13 16:53:15 PST
youenn fablet
Comment 3 2018-02-13 18:08:34 PST
youenn fablet
Comment 4 2018-02-13 18:25:08 PST
Chris Dumez
Comment 5 2018-02-13 18:26:16 PST
Comment on attachment 333755 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333755&action=review > Source/WebCore/html/HTMLPlugInElement.cpp:408 > + ASCIILiteral consoleMessage("Tried to use an unsupported plugin."); I do not think this local variable is needed. > Source/WebCore/plugins/SupportedPluginNames.h:37 > +struct SupportedPluginNames { Given the API, it seems like ignorance could be a class with the data members private. > Source/WebCore/plugins/SupportedPluginNames.h:50 > + auto specific = originSpecificPlugins.get(&origin); This may construct an empty HashSet unnecessarily. We may want to use find() here. > Source/WebCore/plugins/SupportedPluginNames.h:66 > +template<class Decoder> inline std::optional<HashSet<String>> decodePluginNames(Decoder& decoder) Doesn’t HashSet already have encoding / decoding functions? > Source/WebCore/testing/Internals.cpp:3277 > + if (!renderer || !is<RenderEmbeddedObject>(*renderer)) I believe we can omit the first check and pass a pointer to the is() function. > Source/WebCore/testing/Internals.cpp:3280 > + auto result = downcast<RenderEmbeddedObject>(*renderer).pluginReplacementTextIfUnavailable(); I do not think this local variable is useful. > Source/WebKit/WebProcess/Plugins/WebPluginInfoProvider.cpp:130 > + for (int32_t i = plugins.size() - 1; i >= 0; --i) { Cannot this use removeAllMatching()?
EWS Watchlist
Comment 6 2018-02-13 21:49:39 PST
Comment on attachment 333757 [details] Patch Attachment 333757 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/6494850 New failing tests: http/tests/plugins/supported-plugin-origin-specific-visibility.html http/tests/plugins/nounsupported-plugin.html
EWS Watchlist
Comment 7 2018-02-13 21:49:40 PST
Created attachment 333765 [details] Archive of layout-test-results from ews104 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
youenn fablet
Comment 8 2018-02-14 13:52:25 PST
youenn fablet
Comment 9 2018-02-14 14:08:25 PST
youenn fablet
Comment 10 2018-02-14 14:24:47 PST
youenn fablet
Comment 11 2018-02-14 15:28:46 PST
youenn fablet
Comment 12 2018-02-14 16:37:15 PST
youenn fablet
Comment 13 2018-02-14 18:35:11 PST
youenn fablet
Comment 14 2018-02-15 09:59:21 PST
mitz
Comment 15 2018-02-16 10:43:47 PST
Comment on attachment 333911 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333911&action=review > Source/WebCore/platform/LocalizedStrings.cpp:665 > + return WEB_UI_STRING_KEY("Unsupported Plug-in", "Unsupported Plug-In", "Label text to be used when an unsupported plug-in was blocked from loading"); Why does the key need to be different from the English here?
Chris Dumez
Comment 16 2018-02-16 11:32:51 PST
Comment on attachment 333911 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333911&action=review r=me > Source/WebCore/PAL/pal/SessionID.h:98 > +std::optional<SessionID> SessionID::decode(Decoder& decoder) Please update the legacy coder to call this one if possible. This would avoid code duplication. > Source/WebCore/html/HTMLPlugInElement.cpp:408 > + document().addConsoleMessage(MessageSource::JS, MessageLevel::Warning, ASCIILiteral("Tried to use an unsupported plugin.")); plug-in ? >> Source/WebCore/platform/LocalizedStrings.cpp:665 >> + return WEB_UI_STRING_KEY("Unsupported Plug-in", "Unsupported Plug-In", "Label text to be used when an unsupported plug-in was blocked from loading"); > > Why does the key need to be different from the English here? At least it is consistent with Blocked Plug-in above. > Source/WebKit/Platform/IPC/ArgumentCoders.h:429 > + static std::optional<HashSetType> decode(Decoder& decoder) Please update the legacy coder to call this one if possible. This would avoid code duplication.
youenn fablet
Comment 17 2018-02-16 13:25:22 PST
Created attachment 334063 [details] Patch for landing
youenn fablet
Comment 18 2018-02-16 14:13:46 PST
Created attachment 334069 [details] Patch for landing
WebKit Commit Bot
Comment 19 2018-02-16 14:17:39 PST
Comment on attachment 334069 [details] Patch for landing Rejecting attachment 334069 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 334069, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: in.html patching file LayoutTests/platform/ios-wk1/TestExpectations patching file LayoutTests/platform/mac-wk1/TestExpectations Hunk #2 FAILED at 504. 1 out of 2 hunks FAILED -- saving rejects to file LayoutTests/platform/mac-wk1/TestExpectations.rej patching file LayoutTests/plugins/unsupported-plugin-expected.txt patching file LayoutTests/plugins/unsupported-plugin.html Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: http://webkit-queues.webkit.org/results/6542471
youenn fablet
Comment 20 2018-02-16 14:24:44 PST
Created attachment 334071 [details] Patch for landing
WebKit Commit Bot
Comment 21 2018-02-16 15:31:37 PST
Comment on attachment 334071 [details] Patch for landing Clearing flags on attachment: 334071 Committed r228587: <https://trac.webkit.org/changeset/228587>
WebKit Commit Bot
Comment 22 2018-02-16 15:31:39 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 23 2018-02-16 15:32:23 PST
Chris Dumez
Comment 24 2018-02-16 17:00:53 PST
WebKit.UnavailablePlugIn API test seems to be failing after this.
youenn fablet
Comment 25 2018-02-16 17:30:36 PST
(In reply to Chris Dumez from comment #24) > WebKit.UnavailablePlugIn API test seems to be failing after this. Not reproduced on bot AFAICS. Locally tested, timing out if flash is installed, passing otherwise. I guess the test could be made more robust.
Note You need to log in before you can comment on or make changes to this bug.