|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
59 bytes, text/x-review-board-request
|Details | Review|
The code at http://searchfox.org/mozilla-central/rev/20963d7269b1b14d455f47bc0260d0653015bf84/toolkit/mozapps/extensions/internal/PluginProvider.jsm#31 uses NSS to compute MD5 hashes. See this profile: https://perfht.ml/2sf5TQ3 The JS call stack is: getIDHashForString getPluginList buildPluginList getAddonsByTypes callProviderAsync promiseCallProvider/< Promise promiseCallProvider getAddonsByTypes/< next
This sounds like perhaps bug 1358907?
(In reply to Andrew Swan [:aswan] from comment #1) > This sounds like perhaps bug 1358907? That's what triggers this now but the bug itself would arise with any other code invoking this. The real issue dates back to bug 617289. The code here is trying to compute a persisted identifier for the plugins that is preserved across restarts. Now that we only support Flash and a handful of other known plugins, can we have a whitelist of known plugin types here? Needinfoing qdot as he has worked on related stuff recently.
Er, I'm not sure what you mean by whitelist here. This hashes the plugin by its name and description, and the description will contain the version number, so we'll still need some id for differentiating plugins (since Flash can be updated in the background while Firefox is running). I don't think it needs to be an md5 though. We've got few enough plugins that we could possibly just use the name/description string itself?
Yeah, I don't see any reason to use a hash here versus the actual strings.
This passed try, after I realized the plugin test failures were already intermittents that weren't related to this: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a7cf9532e0f0f89356ad6a991e0ce96c8ea92aa0
Comment on attachment 8877317 [details] Bug 1370832 - Remove PluginProvider Hash calculation for plugin name/desc; https://reviewboard.mozilla.org/r/148652/#review153618
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/7f2e9866b29b Remove PluginProvider Hash calculation for plugin name/desc; r=bsmedberg
Since this is a fairly benign change that gets us a startup speed boost, is it worth uplifting?
I don't think an uplift is necessary. Keep focus on 57!