Bug 182748 - Allow specifying which plug-ins are supported
Summary: Allow specifying which plug-ins are supported
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-02-13 14:47 PST by youenn fablet
Modified: 2018-02-16 17:30 PST (History)
10 users (show)

See Also:


Attachments
Patch (75.80 KB, patch)
2018-02-13 16:07 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (78.29 KB, patch)
2018-02-13 16:53 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (78.90 KB, patch)
2018-02-13 18:08 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (78.81 KB, patch)
2018-02-13 18:25 PST, youenn fablet
no flags Details | Formatted Diff | Diff
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 Details
Patch (75.82 KB, patch)
2018-02-14 13:52 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (76.07 KB, patch)
2018-02-14 14:08 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (76.20 KB, patch)
2018-02-14 14:24 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (79.02 KB, patch)
2018-02-14 15:28 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (79.39 KB, patch)
2018-02-14 16:37 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (79.41 KB, patch)
2018-02-14 18:35 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (79.40 KB, patch)
2018-02-15 09:59 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (79.75 KB, patch)
2018-02-16 13:25 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (79.75 KB, patch)
2018-02-16 14:13 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (79.55 KB, patch)
2018-02-16 14:24 PST, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2018-02-13 14:47:44 PST
Allow specifying which plug-ins are supported
Comment 1 youenn fablet 2018-02-13 16:07:07 PST
Created attachment 333739 [details]
Patch
Comment 2 youenn fablet 2018-02-13 16:53:15 PST
Created attachment 333747 [details]
Patch
Comment 3 youenn fablet 2018-02-13 18:08:34 PST
Created attachment 333755 [details]
Patch
Comment 4 youenn fablet 2018-02-13 18:25:08 PST
Created attachment 333757 [details]
Patch
Comment 5 Chris Dumez 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()?
Comment 6 EWS Watchlist 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
Comment 7 EWS Watchlist 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
Comment 8 youenn fablet 2018-02-14 13:52:25 PST
Created attachment 333836 [details]
Patch
Comment 9 youenn fablet 2018-02-14 14:08:25 PST
Created attachment 333839 [details]
Patch
Comment 10 youenn fablet 2018-02-14 14:24:47 PST
Created attachment 333843 [details]
Patch
Comment 11 youenn fablet 2018-02-14 15:28:46 PST
Created attachment 333852 [details]
Patch
Comment 12 youenn fablet 2018-02-14 16:37:15 PST
Created attachment 333859 [details]
Patch
Comment 13 youenn fablet 2018-02-14 18:35:11 PST
Created attachment 333865 [details]
Patch
Comment 14 youenn fablet 2018-02-15 09:59:21 PST
Created attachment 333911 [details]
Patch
Comment 15 mitz 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?
Comment 16 Chris Dumez 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.
Comment 17 youenn fablet 2018-02-16 13:25:22 PST
Created attachment 334063 [details]
Patch for landing
Comment 18 youenn fablet 2018-02-16 14:13:46 PST
Created attachment 334069 [details]
Patch for landing
Comment 19 WebKit Commit Bot 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
Comment 20 youenn fablet 2018-02-16 14:24:44 PST
Created attachment 334071 [details]
Patch for landing
Comment 21 WebKit Commit Bot 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>
Comment 22 WebKit Commit Bot 2018-02-16 15:31:39 PST
All reviewed patches have been landed.  Closing bug.
Comment 23 Radar WebKit Bug Importer 2018-02-16 15:32:23 PST
<rdar://problem/37621938>
Comment 24 Chris Dumez 2018-02-16 17:00:53 PST
WebKit.UnavailablePlugIn API test seems to be failing after this.
Comment 25 youenn fablet 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.