Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(131)

Issue 2157883002: Cache the mimeTypes and plugins DOM objects. (Closed)

Created:
4 years, 5 months ago by Anton Obzhirov
Modified:
4 years, 2 months ago
CC:
blink-reviews, chromium-reviews, haraken
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Cache the mimeTypes and plugins DOM objects. At the moment the mimeTypes and plugins objects on navigator return new objects when indexed by the same ordinal. This patch adds caching of the mimeTypes and plugins script wrappable DOM objects DOMPlugin/DOMMimeType in DOMPluginArray/DOMMimeTypeArray. BUG=616662

Patch Set 1 #

Patch Set 2 : Added new test and clear cache on refresh. #

Total comments: 4

Patch Set 3 : Added new layout test, both tests use testharness.js. #

Total comments: 11

Patch Set 4 : The new tests cleanup. #

Patch Set 5 : Updated after dry run. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+119 lines, -7 lines) Patch
A third_party/WebKit/LayoutTests/plugins/enabledPlugin-after-iframe-detached.html View 1 2 3 1 chunk +21 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/plugins/navigator-plugins-mimeTypes-comparison.html View 1 2 3 1 chunk +39 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/plugins/resources/x-blink-test-plugin-iframe.html View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/plugins/DOMMimeType.cpp View 1 2 3 2 chunks +4 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/plugins/DOMMimeTypeArray.h View 1 2 3 4 2 chunks +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/plugins/DOMMimeTypeArray.cpp View 1 2 3 4 4 chunks +18 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/plugins/DOMPlugin.cpp View 1 2 3 3 chunks +6 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/plugins/DOMPluginArray.h View 1 2 3 4 2 chunks +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/plugins/DOMPluginArray.cpp View 1 2 3 4 4 chunks +18 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/plugins/PluginData.h View 1 2 3 2 chunks +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/plugins/PluginData.cpp View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 39 (12 generated)
PhistucK
Does disabling a plugin using about:plugins without a page refresh behave as expected with this ...
4 years, 5 months ago (2016-07-18 05:28:26 UTC) #4
Anton Obzhirov
On 2016/07/18 05:28:26, PhistucK wrote: > Does disabling a plugin using about:plugins without a page ...
4 years, 5 months ago (2016-07-18 14:44:37 UTC) #5
dominicc (has gone to gerrit)
As always, please add tests.
4 years, 5 months ago (2016-07-19 01:24:17 UTC) #6
Anton Obzhirov
On 2016/07/19 01:24:17, dominicc wrote: > As always, please add tests. Will do.
4 years, 5 months ago (2016-07-19 10:43:25 UTC) #7
Anton Obzhirov
On 2016/07/19 10:43:25, Anton Obzhirov wrote: > On 2016/07/19 01:24:17, dominicc wrote: > > As ...
4 years, 5 months ago (2016-07-24 15:39:19 UTC) #9
dominicc (has gone to gerrit)
Thank you for working on this. A couple of comments inline. https://codereview.chromium.org/2157883002/diff/20001/third_party/WebKit/LayoutTests/plugins/script-tests/navigator-plugins-mimeTypes-comparison.js File third_party/WebKit/LayoutTests/plugins/script-tests/navigator-plugins-mimeTypes-comparison.js (right): ...
4 years, 5 months ago (2016-07-25 08:02:37 UTC) #10
Anton Obzhirov
On 2016/07/25 08:02:37, dominicc wrote: > Thank you for working on this. A couple of ...
4 years, 4 months ago (2016-07-27 19:41:56 UTC) #11
Anton Obzhirov
https://codereview.chromium.org/2157883002/diff/20001/third_party/WebKit/LayoutTests/plugins/script-tests/navigator-plugins-mimeTypes-comparison.js File third_party/WebKit/LayoutTests/plugins/script-tests/navigator-plugins-mimeTypes-comparison.js (right): https://codereview.chromium.org/2157883002/diff/20001/third_party/WebKit/LayoutTests/plugins/script-tests/navigator-plugins-mimeTypes-comparison.js#newcode1 third_party/WebKit/LayoutTests/plugins/script-tests/navigator-plugins-mimeTypes-comparison.js:1: description( On 2016/07/25 08:02:37, dominicc wrote: > Could you ...
4 years, 4 months ago (2016-07-27 19:43:04 UTC) #12
Anton Obzhirov
On 2016/07/27 19:43:04, Anton Obzhirov wrote: > https://codereview.chromium.org/2157883002/diff/20001/third_party/WebKit/LayoutTests/plugins/script-tests/navigator-plugins-mimeTypes-comparison.js > File > third_party/WebKit/LayoutTests/plugins/script-tests/navigator-plugins-mimeTypes-comparison.js > (right): > ...
4 years, 4 months ago (2016-08-14 20:41:52 UTC) #13
dominicc (has gone to gerrit)
Sorry for taking so long to get back to you on this! Thank you for ...
4 years, 3 months ago (2016-08-30 02:46:29 UTC) #14
Anton Obzhirov
On 2016/08/30 02:46:29, dominicc wrote: > Sorry for taking so long to get back to ...
4 years, 3 months ago (2016-08-30 20:00:29 UTC) #15
Anton Obzhirov
https://codereview.chromium.org/2157883002/diff/40001/third_party/WebKit/LayoutTests/plugins/enabledPlugin-after-iframe-detached.html File third_party/WebKit/LayoutTests/plugins/enabledPlugin-after-iframe-detached.html (right): https://codereview.chromium.org/2157883002/diff/40001/third_party/WebKit/LayoutTests/plugins/enabledPlugin-after-iframe-detached.html#newcode19 third_party/WebKit/LayoutTests/plugins/enabledPlugin-after-iframe-detached.html:19: }, 50)); On 2016/08/30 02:46:28, dominicc wrote: > Is ...
4 years, 3 months ago (2016-09-01 10:46:37 UTC) #16
Anton Obzhirov
On 2016/09/01 10:46:37, Anton Obzhirov wrote: > https://codereview.chromium.org/2157883002/diff/40001/third_party/WebKit/LayoutTests/plugins/enabledPlugin-after-iframe-detached.html > File > third_party/WebKit/LayoutTests/plugins/enabledPlugin-after-iframe-detached.html > (right): > ...
4 years, 3 months ago (2016-09-01 17:30:42 UTC) #17
dominicc (has gone to gerrit)
lgtm
4 years, 3 months ago (2016-09-02 01:24:36 UTC) #20
Anton Obzhirov
Will check the failing tests, seems to be the real one.
4 years, 3 months ago (2016-09-02 08:34:38 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2157883002/80001
4 years, 3 months ago (2016-09-05 21:14:20 UTC) #26
Anton Obzhirov
On 2016/09/02 08:34:38, Anton Obzhirov wrote: > Will check the failing tests, seems to be ...
4 years, 3 months ago (2016-09-05 21:16:33 UTC) #27
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/253167)
4 years, 3 months ago (2016-09-05 21:17:09 UTC) #29
Anton Obzhirov
On 2016/09/05 21:17:09, commit-bot: I haz the power wrote: > Try jobs failed on following ...
4 years, 3 months ago (2016-09-06 07:40:45 UTC) #31
tkent
This doesn't look a right approach. Can we merge PluginData into DOMPlugin, and merge MimeClassInfo ...
4 years, 3 months ago (2016-09-06 08:00:00 UTC) #32
Anton Obzhirov
On 2016/09/06 08:00:00, tkent wrote: > This doesn't look a right approach. > Can we ...
4 years, 3 months ago (2016-09-07 14:14:26 UTC) #33
tkent
On 2016/09/07 at 14:14:26, a.obzhirov wrote: > You mean merge PluginInfo into DOMPlugin? Oh, it's ...
4 years, 3 months ago (2016-09-08 05:41:48 UTC) #34
Anton Obzhirov
On 2016/09/08 05:41:48, tkent wrote: > On 2016/09/07 at 14:14:26, a.obzhirov wrote: > > You ...
4 years, 3 months ago (2016-09-08 12:29:58 UTC) #35
tkent
not lgtm. We should not introduce HashSets to resolve the issue. We should fix the ...
4 years, 3 months ago (2016-09-09 02:57:17 UTC) #36
Anton Obzhirov
On 2016/09/09 02:57:17, tkent wrote: > not lgtm. We should not introduce HashSets to resolve ...
4 years, 3 months ago (2016-09-09 10:24:53 UTC) #37
dominicc (has gone to gerrit)
On 2016/09/09 at 10:24:53, a.obzhirov wrote: > On 2016/09/09 02:57:17, tkent wrote: > > not ...
4 years, 2 months ago (2016-10-14 08:23:05 UTC) #38
Anton Obzhirov
4 years, 2 months ago (2016-10-14 21:57:52 UTC) #39
Message was sent while issue was closed.
On 2016/10/14 08:23:05, dominicc wrote:
> On 2016/09/09 at 10:24:53, a.obzhirov wrote:
> > On 2016/09/09 02:57:17, tkent wrote:
> > > not lgtm.  We should not introduce HashSets to resolve the issue.
> > > 
> > > We should fix the interoperability bug, but the bug is not urgent, and not
> worth
> > > to add ugly hack.
> > 
> > OK, I will do the investigation and will update about its results.
> 
> I'm going to close this for now, but feel free to reopen it with investigation
> results. Or we can discuss on the bug.

OK, I will try to find sometime soon to investigate.

Powered by Google App Engine
This is Rietveld 408576698