Implement HTML5 by Default behind a feature flag |
||||
Issue descriptionImplement HTML5 by Default behind a feature flag
,
Jul 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a5a2bb3171707315ef1e2737edc4677985ca453c commit a5a2bb3171707315ef1e2737edc4677985ca453c Author: trizzofo <trizzofo@google.com> Date: Mon Jul 25 18:10:18 2016 [HBD] Don't advertise Flash if Content Setting is on CONTENT_SETTING_BLOCK When PreferHtmlOverPlugins feature is enabled, fetch Content Settings and don't advertise Flash if it's on CONTENT_SETTING_BLOCK. This will only work properly when plugin caches are disabled in future CLs. Here is more information about HTML5 by Default: https://docs.google.com/document/d/1iHhg7wxYTgBHsTzcfaZy36UJ33JmjRLro8TZR6lb0ss BUG= 626728 Review-Url: https://codereview.chromium.org/2168453002 Cr-Commit-Position: refs/heads/master@{#407514} [modify] https://crrev.com/a5a2bb3171707315ef1e2737edc4677985ca453c/chrome/browser/plugins/chrome_plugin_service_filter.cc [modify] https://crrev.com/a5a2bb3171707315ef1e2737edc4677985ca453c/chrome/browser/plugins/chrome_plugin_service_filter.h [modify] https://crrev.com/a5a2bb3171707315ef1e2737edc4677985ca453c/chrome/browser/printing/print_preview_dialog_controller_browsertest.cc [modify] https://crrev.com/a5a2bb3171707315ef1e2737edc4677985ca453c/chrome/browser/profiles/off_the_record_profile_impl.cc [modify] https://crrev.com/a5a2bb3171707315ef1e2737edc4677985ca453c/chrome/browser/profiles/profile_impl.cc
,
Aug 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c56e1750274cbd7efb5cba947421ec7c36379f3a commit c56e1750274cbd7efb5cba947421ec7c36379f3a Author: trizzofo <trizzofo@google.com> Date: Tue Aug 23 21:30:43 2016 [HBD] Implement site-origin exceptions If PreferHtmlOverPlugins feature is enabled, use "Plugins" Content Settings exceptions to decide if Flash should be advertised or not. Here is more information about HTML5 by Default: go/hbd-design-doc BUG= 626728 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2208463002 Cr-Commit-Position: refs/heads/master@{#413842} [modify] https://crrev.com/c56e1750274cbd7efb5cba947421ec7c36379f3a/chrome/browser/plugins/chrome_plugin_service_filter.cc [add] https://crrev.com/c56e1750274cbd7efb5cba947421ec7c36379f3a/chrome/browser/plugins/plugin_filter_utils.cc [add] https://crrev.com/c56e1750274cbd7efb5cba947421ec7c36379f3a/chrome/browser/plugins/plugin_filter_utils.h [modify] https://crrev.com/c56e1750274cbd7efb5cba947421ec7c36379f3a/chrome/browser/plugins/plugin_info_message_filter.cc [modify] https://crrev.com/c56e1750274cbd7efb5cba947421ec7c36379f3a/chrome/browser/plugins/plugin_info_message_filter.h [modify] https://crrev.com/c56e1750274cbd7efb5cba947421ec7c36379f3a/chrome/browser/plugins/plugin_info_message_filter_unittest.cc [modify] https://crrev.com/c56e1750274cbd7efb5cba947421ec7c36379f3a/chrome/chrome_browser.gypi
,
Aug 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3e5d33572d42429457897450228f773d30e7af17 commit 3e5d33572d42429457897450228f773d30e7af17 Author: trizzofo <trizzofo@google.com> Date: Tue Aug 23 22:14:40 2016 [HBD] Call PurgePluginListCache() whenever plugin content settings change Call PluginService::PurgePluginListCache() whenever plugin content settings change. This is made to ensure that Page's PluginData instance is updated for the next navigation. BUG= 626728 Review-Url: https://codereview.chromium.org/2234133003 Cr-Commit-Position: refs/heads/master@{#413856} [modify] https://crrev.com/3e5d33572d42429457897450228f773d30e7af17/chrome/browser/plugins/chrome_plugin_service_filter.cc [modify] https://crrev.com/3e5d33572d42429457897450228f773d30e7af17/chrome/browser/plugins/chrome_plugin_service_filter.h [modify] https://crrev.com/3e5d33572d42429457897450228f773d30e7af17/chrome/browser/profiles/off_the_record_profile_impl.cc [modify] https://crrev.com/3e5d33572d42429457897450228f773d30e7af17/chrome/browser/profiles/profile_impl.cc
,
Aug 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a1b97045fc29fd8e045cd2b9bbdbbe1d1104d6dc commit a1b97045fc29fd8e045cd2b9bbdbbe1d1104d6dc Author: trizzofo <trizzofo@google.com> Date: Fri Aug 26 02:33:25 2016 [HBD] Map ASK to DETECT_IMPORTANT_CONTENT when PreferHtmlOverPlugins is enabled If PreferHtmlOverPlugins feature is enabled, map the obsolete ASK setting to DETECT_IMPORTANT_CONTENT. More info about Html5 by Default here: go/hbd-design-doc BUG= 626728 Review-Url: https://codereview.chromium.org/2264583003 Cr-Commit-Position: refs/heads/master@{#414634} [modify] https://crrev.com/a1b97045fc29fd8e045cd2b9bbdbbe1d1104d6dc/chrome/browser/plugins/chrome_plugin_service_filter.cc [modify] https://crrev.com/a1b97045fc29fd8e045cd2b9bbdbbe1d1104d6dc/chrome/browser/plugins/plugin_info_message_filter.cc [add] https://crrev.com/a1b97045fc29fd8e045cd2b9bbdbbe1d1104d6dc/chrome/browser/plugins/plugins_field_trial.cc [rename] https://crrev.com/a1b97045fc29fd8e045cd2b9bbdbbe1d1104d6dc/chrome/browser/plugins/plugins_field_trial.h [modify] https://crrev.com/a1b97045fc29fd8e045cd2b9bbdbbe1d1104d6dc/chrome/browser/ui/website_settings/permission_menu_model.cc [modify] https://crrev.com/a1b97045fc29fd8e045cd2b9bbdbbe1d1104d6dc/chrome/browser/ui/website_settings/website_settings_ui.cc [modify] https://crrev.com/a1b97045fc29fd8e045cd2b9bbdbbe1d1104d6dc/chrome/browser/ui/webui/options/content_settings_handler.cc [modify] https://crrev.com/a1b97045fc29fd8e045cd2b9bbdbbe1d1104d6dc/chrome/chrome_browser.gypi [modify] https://crrev.com/a1b97045fc29fd8e045cd2b9bbdbbe1d1104d6dc/components/content_settings.gypi [modify] https://crrev.com/a1b97045fc29fd8e045cd2b9bbdbbe1d1104d6dc/components/content_settings/core/browser/BUILD.gn [delete] https://crrev.com/6c2cdfb89375fe0734615bd8081283d25fb0aad5/components/content_settings/core/browser/plugins_field_trial.cc
,
Aug 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1c684eba8afc4c730b3ca2e68814e8412020552c commit 1c684eba8afc4c730b3ca2e68814e8412020552c Author: trizzofo <trizzofo@google.com> Date: Tue Aug 30 05:00:24 2016 Removes PluginCache and reload PluginData's plugin list whenever the origin changes. Use origin url to retrieve new plugin list. This is needed because, after HTML5 by Default is implemented, the plugin list will depend on the current web page url (settings url exceptions). Here is more information about HTML5 by Default: go/hbd-design-doc BUG= 626728 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2156803002 Cr-Commit-Position: refs/heads/master@{#415128} [modify] https://crrev.com/1c684eba8afc4c730b3ca2e68814e8412020552c/content/browser/frame_host/render_frame_message_filter.cc [modify] https://crrev.com/1c684eba8afc4c730b3ca2e68814e8412020552c/content/browser/frame_host/render_frame_message_filter.h [modify] https://crrev.com/1c684eba8afc4c730b3ca2e68814e8412020552c/content/common/frame_messages.h [modify] https://crrev.com/1c684eba8afc4c730b3ca2e68814e8412020552c/content/ppapi_plugin/ppapi_blink_platform_impl.cc [modify] https://crrev.com/1c684eba8afc4c730b3ca2e68814e8412020552c/content/ppapi_plugin/ppapi_blink_platform_impl.h [modify] https://crrev.com/1c684eba8afc4c730b3ca2e68814e8412020552c/content/renderer/renderer_blink_platform_impl.cc [modify] https://crrev.com/1c684eba8afc4c730b3ca2e68814e8412020552c/content/renderer/renderer_blink_platform_impl.h [modify] https://crrev.com/1c684eba8afc4c730b3ca2e68814e8412020552c/content/test/test_blink_web_unit_test_support.cc [modify] https://crrev.com/1c684eba8afc4c730b3ca2e68814e8412020552c/content/test/test_blink_web_unit_test_support.h [modify] https://crrev.com/1c684eba8afc4c730b3ca2e68814e8412020552c/third_party/WebKit/Source/core/dom/DOMImplementation.cpp [modify] https://crrev.com/1c684eba8afc4c730b3ca2e68814e8412020552c/third_party/WebKit/Source/core/frame/LocalFrame.cpp [modify] https://crrev.com/1c684eba8afc4c730b3ca2e68814e8412020552c/third_party/WebKit/Source/core/page/Page.cpp [modify] https://crrev.com/1c684eba8afc4c730b3ca2e68814e8412020552c/third_party/WebKit/Source/core/page/Page.h [modify] https://crrev.com/1c684eba8afc4c730b3ca2e68814e8412020552c/third_party/WebKit/Source/platform/plugins/PluginData.cpp [modify] https://crrev.com/1c684eba8afc4c730b3ca2e68814e8412020552c/third_party/WebKit/Source/platform/plugins/PluginData.h [modify] https://crrev.com/1c684eba8afc4c730b3ca2e68814e8412020552c/third_party/WebKit/public/platform/Platform.h
,
Sep 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8ddaeb7e8317409c88cbb9f1bc65e4351f539239 commit 8ddaeb7e8317409c88cbb9f1bc65e4351f539239 Author: trizzofo <trizzofo@google.com> Date: Thu Sep 01 23:18:27 2016 [HBD] Intercept navigation to Flash download page Intercepts navigation to get.adobe.com/flashplayer Here is more information about HTML5 by Default: go/hbd-design-doc BUG= 626728 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2272123002 Cr-Commit-Position: refs/heads/master@{#416113} [modify] https://crrev.com/8ddaeb7e8317409c88cbb9f1bc65e4351f539239/chrome/browser/BUILD.gn [modify] https://crrev.com/8ddaeb7e8317409c88cbb9f1bc65e4351f539239/chrome/browser/chrome_content_browser_client.cc [add] https://crrev.com/8ddaeb7e8317409c88cbb9f1bc65e4351f539239/chrome/browser/plugins/flash_download_interception.cc [add] https://crrev.com/8ddaeb7e8317409c88cbb9f1bc65e4351f539239/chrome/browser/plugins/flash_download_interception.h [add] https://crrev.com/8ddaeb7e8317409c88cbb9f1bc65e4351f539239/chrome/browser/plugins/flash_download_interception_unittest.cc [modify] https://crrev.com/8ddaeb7e8317409c88cbb9f1bc65e4351f539239/chrome/chrome_tests_unit.gypi
,
Sep 2 2016
I'm assigning this to tommycli@ because my internship is coming to an end.
,
Sep 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8583adb8ee4711d11421d53579567bc68eaf4d54 commit 8583adb8ee4711d11421d53579567bc68eaf4d54 Author: dominickn <dominickn@chromium.org> Date: Tue Sep 20 01:22:22 2016 [HBD] Gate the advertising of Flash on Site Engagement. When the PreferHtmlOverPlugins feature is active, and the plugins content setting isn't one of ALLOW or BLOCK, each site's engagement score is now consulted to determine whether or not Flash is added to the list of available plugins. The score must be above a certain threshold for Flash to be advertised. The amount of engagement required to activate Flash defaults to 1, which is currently the equivalent of navigating directly to a site via the omnibox, or spending ~1 minute interacting with the site after visiting via a link. This value is controllable via a variations parameter under the PreferHtmlOverPlugins feature. This CL adds a thread-agnostic interface to the site engagement service, which is necessary as ChromePluginServiceFilter::IsPluginAvailable runs exclusively on the IO thread, from which is it not possible to create a SiteEngagementService object. This new interface relies on the fact that the underlying HostContentSettingsMap class methods can be called from any thread. This CL additionally removes the fallback logic from the site engagement service for incognito support, as this is already baked into the underlying HostContentSettingsMap. A minor cleanup of ChromePluginServiceFilter is also undertaken. BUG= 626728 ,641627,641630 Review-Url: https://codereview.chromium.org/2285553002 Cr-Commit-Position: refs/heads/master@{#419623} [modify] https://crrev.com/8583adb8ee4711d11421d53579567bc68eaf4d54/chrome/browser/engagement/site_engagement_score.cc [modify] https://crrev.com/8583adb8ee4711d11421d53579567bc68eaf4d54/chrome/browser/engagement/site_engagement_score.h [modify] https://crrev.com/8583adb8ee4711d11421d53579567bc68eaf4d54/chrome/browser/engagement/site_engagement_score_unittest.cc [modify] https://crrev.com/8583adb8ee4711d11421d53579567bc68eaf4d54/chrome/browser/engagement/site_engagement_service.cc [modify] https://crrev.com/8583adb8ee4711d11421d53579567bc68eaf4d54/chrome/browser/engagement/site_engagement_service.h [modify] https://crrev.com/8583adb8ee4711d11421d53579567bc68eaf4d54/chrome/browser/engagement/site_engagement_service_unittest.cc [modify] https://crrev.com/8583adb8ee4711d11421d53579567bc68eaf4d54/chrome/browser/plugins/chrome_plugin_service_filter.cc [modify] https://crrev.com/8583adb8ee4711d11421d53579567bc68eaf4d54/chrome/browser/plugins/chrome_plugin_service_filter.h [add] https://crrev.com/8583adb8ee4711d11421d53579567bc68eaf4d54/chrome/browser/plugins/chrome_plugin_service_filter_unittest.cc [modify] https://crrev.com/8583adb8ee4711d11421d53579567bc68eaf4d54/chrome/browser/plugins/plugins_field_trial.cc [modify] https://crrev.com/8583adb8ee4711d11421d53579567bc68eaf4d54/chrome/browser/plugins/plugins_field_trial.h [modify] https://crrev.com/8583adb8ee4711d11421d53579567bc68eaf4d54/chrome/test/BUILD.gn
,
Sep 21 2016
,
Sep 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/41667aa90a95103ac486a938b1b513bf33a72613 commit 41667aa90a95103ac486a938b1b513bf33a72613 Author: tommycli <tommycli@chromium.org> Date: Mon Sep 26 16:41:35 2016 [HBD] Add Field Trial testing config for PreferHtmlOverPlugins BUG= 626728 Review-Url: https://codereview.chromium.org/2360083003 Cr-Commit-Position: refs/heads/master@{#420907} [modify] https://crrev.com/41667aa90a95103ac486a938b1b513bf33a72613/testing/variations/fieldtrial_testing_config.json
,
Sep 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/41667aa90a95103ac486a938b1b513bf33a72613 commit 41667aa90a95103ac486a938b1b513bf33a72613 Author: tommycli <tommycli@chromium.org> Date: Mon Sep 26 16:41:35 2016 [HBD] Add Field Trial testing config for PreferHtmlOverPlugins BUG= 626728 Review-Url: https://codereview.chromium.org/2360083003 Cr-Commit-Position: refs/heads/master@{#420907} [modify] https://crrev.com/41667aa90a95103ac486a938b1b513bf33a72613/testing/variations/fieldtrial_testing_config.json
,
Oct 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/03f4f7abbeee8660189dc9e1a3504790b2d74adc commit 03f4f7abbeee8660189dc9e1a3504790b2d74adc Author: robliao <robliao@chromium.org> Date: Sat Oct 01 02:16:40 2016 Revert of [HBD] Add Field Trial testing config for PreferHtmlOverPlugins (patchset #5 id:80001 of https://codereview.chromium.org/2360083003/ ) Reason for revert: Speculative revert to see if smoothness.scrolling_tough_ad_cases improves Original issue's description: > [HBD] Add Field Trial testing config for PreferHtmlOverPlugins > > BUG= 626728 > > Committed: https://crrev.com/41667aa90a95103ac486a938b1b513bf33a72613 > Cr-Commit-Position: refs/heads/master@{#420907} TBR=engedy@chromium.org,rkaplow@chromium.org,tommycli@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 626728 , 651547 Review-Url: https://codereview.chromium.org/2375153008 Cr-Commit-Position: refs/heads/master@{#422288} [modify] https://crrev.com/03f4f7abbeee8660189dc9e1a3504790b2d74adc/testing/variations/fieldtrial_testing_config.json
,
Oct 3 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0952e991d96c311fc55a4cf26506d4f5357f2f7d commit 0952e991d96c311fc55a4cf26506d4f5357f2f7d Author: tommycli <tommycli@chromium.org> Date: Mon Oct 03 18:12:32 2016 [HBD] Add Field Trial testing config for PreferHtmlOverPlugins BUG= 626728 Committed: https://crrev.com/41667aa90a95103ac486a938b1b513bf33a72613 Review-Url: https://codereview.chromium.org/2360083003 Cr-Original-Commit-Position: refs/heads/master@{#420907} Cr-Commit-Position: refs/heads/master@{#422461} [modify] https://crrev.com/0952e991d96c311fc55a4cf26506d4f5357f2f7d/testing/variations/fieldtrial_testing_config.json
,
Oct 19 2016
Hi, I'm working on updating my Chromium-based application past the changes in comment #6. The changes in #6 added a new caller of IsPluginAvailable (via DOMImplementation::createDocument and RendererBlinkPlatformImpl::getPluginList). If I block the plugin load for this caller then the PluginDocument is not created in DOMImplementation::createDocument and the client is left in a generally broken state (it displays the view_id in an HTMLDocument instead of triggering plugin creation via ContentRendererClient::CreatePlugin as expected). To work around this problem I'm now caching the plugin load decision on a per-render-frame basis when IsPluginAvailable is called for the first time (via MimeSniffingResourceHandler::CheckForPluginHandler). This leads to another problem. When RenderFrameMessageFilter::GetPluginsCallback is called via RendererBlinkPlatformImpl::getPluginList it's always passing (-1, MSG_ROUTING_NONE) to IsPluginAvailable instead of using the actual routing IDs. Is this intentional, or can we instead pass (render_process_id_, reply_msg->routing_id()) from RenderFrameMessageFilter::GetPluginsCallback? Thanks, Marshall
,
Oct 19 2016
@comment#15: Looks like reply_msg->routing_id() is always MSG_ROUTING_NONE in RenderFrameMessageFilter::GetPluginsCallback. To make this work we would also need to plumb the frame ID to RendererBlinkPlatformImpl::getPluginList and explicitly send it via FrameHostMsg_GetPlugins.
,
Oct 24 2016
marshall: By "block the plugin load", you mean the plugin is blocked within Content Settings and |pluginData| is null right? How did it behave before when the plugin was disabled via chrome://plugins?
,
Oct 24 2016
@tommycli: I use a custom PluginServiceFilter implementation in my application. The PluginServiceFilter::IsPluginAvailable method will be called multiple times (call stacks shown in https://bitbucket.org/chromiumembedded/cef/issues/2015/). The goal is to return the same value consistently for the calls originating from Blink or network internals (e.g. call stacks 1, 2, 3 and 5). For a given frame: - If IsPluginAvailable consistently returns true then Blink will list the plugin in `navigator.plugins` and the plugin will load/display. - If IsPluginAvailable consistently returns false then Blink will not list the plugin in `navigator.plugins` and the plugin will be disabled (e.g. the same behavior as disabling the plugin via chrome://plugins). - If IsPluginAvailable returns mixed values then Blink will exhibit the breakage described in comment #15. In order to enable/disable plugin loading on a per-frame basis it's necessary to identify all IsPluginAvailable calls for that frame. This is not possible currently without modifying RenderFrameMessageFilter, because RenderFrameMessageFilter::GetPluginsCallback does not pass accurate routing IDs to IsPluginAvailable. If Blink is provided with consistent enabled/disabled state via IsPluginAvailable then the application can additionally intercept plugin loads on a per-instance basis in ContentRendererClient::OverrideCreatePlugin and show the plugin placeholder for that particular instance. Reasons to return false from IsPluginAvailable instead of showing the plugin placeholder: - The plugin is disabled globally. - The plugin is disabled for a particular frame (e.g. based on application-specific logic). - You would prefer to use the plugin placeholder in general but a particular website behaves better if the plugin is not listed in `navigator.plugins`. For example, some websites will use HTML5 instead of Flash for video iff Flash is not listed in `navigator.plugins`. So, in summary, I would like to enable/disable plugin loading on a per-frame basis using IsPluginAvailable, and also enable/disable plugin loading on a per-instance basis using the plugin placeholder.
,
Oct 24 2016
marshall: Whoa, you made Chromium Embedded Framework? That's awesome. I'm trying to figure out why the changes in #6 broke the old behavior. It looks like in the past, DOMImplementation::createDocument still accessed the plugin list: https://codereview.chromium.org/2156803002/diff/680001/third_party/WebKit/Source/core/dom/DOMImplementation.cpp Another question: Why does IsPluginAvailable not return a consistent answer? I realize the answer depends on the origin of the top level frame - but I thought you were implementing a custom IsPluginAvailable method?
,
Oct 24 2016
@tommycli:
> I'm trying to figure out why the changes in #6 broke the old behavior.
My understanding is that, prior to this change, PluginCache stored the value and RendererBlinkPlatformImpl::getPluginList (and consequently IsPluginAvailable) was called less frequently. I've verified that I don't see a call to IsPluginAvailable from DOMImplementation::createDocument in older branches.
> Why does IsPluginAvailable not return a consistent answer?
I would like it to return a consistent answer :) Previously, Blink cached the result of calling IsPluginAvailable and I didn't need to track that state myself. Now that I need to implement my own cache in IsPluginAvailable I need a bit more state information from RenderFrameMessageFilter.
I can match PluginCache's functionality with a very minor change to RenderFrameMessageFilter (pass the correct render process ID):
diff --git content/browser/frame_host/render_frame_message_filter.cc content/browser/frame_host/render_frame_message_filter.cc
index 4558e89..6e9a472 100644
--- content/browser/frame_host/render_frame_message_filter.cc
+++ content/browser/frame_host/render_frame_message_filter.cc
@@ -484,7 +484,7 @@ void RenderFrameMessageFilter::GetPluginsCallback(
PluginServiceFilter* filter = PluginServiceImpl::GetInstance()->GetFilter();
std::vector<WebPluginInfo> plugins;
- int child_process_id = -1;
+ int child_process_id = render_process_id_;
int routing_id = MSG_ROUTING_NONE;
GURL policy_url =
main_frame_origin.unique() ? GURL() : GURL(main_frame_origin.Serialize());
However, that only allows me to implement render-process-based caching instead of the frame-based caching that I would prefer (because the frame ID is still missing).
,
Oct 24 2016
Why is a cache needed to make the answer consistent? If you have a custom implementation of IsPluginAvailable, it can be consistent per origin right? When does the answer of IsPluginAvailable change in CEF? Also - we already cache the plugin data on the Blink side per-origin here: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/page/Page.cpp?sq=package:chromium&rcl=1477308281&l=246
,
Oct 24 2016
> Why is a cache needed to make the answer consistent? If you have a custom implementation of IsPluginAvailable, it can be consistent per origin right? CEF used to query the client directly each time IsPluginAvailable was called. Now when IsPluginAvailable returns inconsistent results it causes Blink to break as described above (previously Blink didn't break -- maybe we were just lucky). So CEF needs to add a cache in IsPluginAvailable to keep the client from answering inconsistently and breaking Blink. From CEF's perspective we want the client to answer the following questions: 1. Should the plugin be listed in `navigator.plugins`? 2. Should this particular instance of the plugin be allowed to load directly, or should the plugin placeholder be displayed? Each call to IsPluginAvailable is effectively trying to answer #1, so we want to ask the client only a single time and cache the result. The question is how to key the cache, and how to make sure all necessary information is delivered to IsPluginAvailable so that it can maintain the cache. From the client's perspective a frame has a lot of useful metadata beyond just the origin. Say, for example, that an application shows ads and it wants to block those ads from loading specific plugins. In many cases ads are loaded from third-party ad providers in an iframe. If IsPluginAvailable has frame information then the application can check if the requesting frame is parented to that iframe and thereby block the request. If only origin information is available then the application has less options -- it would need to guarantee that it never loaded its own resources from the same origin as ads, or else it risks breaking itself. So, from the client's perspective, it seems that (frame + origin) could be the most useful key. > Also - we already cache the plugin data on the Blink side per-origin here: Thanks for pointing that out. Now I realize that this might be more complicated than I originally thought. The |main_frame_origin| passed to IsPluginAvailable may also be empty. I'm not sure how to maintain an origin-based cache in that case.
,
Oct 24 2016
Ah crap. I didn't realize that the user of CEF implemented IsPluginAvailable (i.e. you don't control that). Previously in Blink there was a global cache, and that patch changed it to a per-origin cache. I think since the cache is hit a lot less often, this is why there are breakages now. i.e. - we were just getting lucky before... This might be tricky to fix if IsPluginAvailable is not a reliable source of answers.
,
Nov 18 2016
Seems to be done... |
||||
►
Sign in to add a comment |
||||
Comment 1 by bugdroid1@chromium.org
, Jul 16 2016