Issue 164157: Delay releasing plugin process for 10s.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 164157: Delay releasing plugin process for 10s. (Closed)

Created:
11 years, 4 months ago by chase

Modified:
9 years, 7 months ago

Reviewers:
jam

CC:
chromium-reviews_googlegroups.com

Visibility:
Public.

Description

Delay releasing plugin process for 10s. Change plugin process release from immediately after we navigate away from our last plugin reference to a task that is run 10s later. Original patch by John Abd-El-Malek (jam@chromium.org). BUG=11341 TEST=Open Chrome, load youtube.com, click Videos, verify all pages load correctly.

Patch Set 1 #

Patch Set 2 : <=80chars #

Total comments: 1

Patch Set 3 : prefer constant over magic number #

Total comments: 1

Created: 11 years, 4 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+12 lines, -1 line)			Patch
	M	chrome/plugin/plugin_channel.cc	View	1 2	2 chunks	+12 lines, -1 line	1 comment	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

jam

lgtm with the one nit, and also updating the changelist description. what does "TEST=Open Chrome, ...

11 years, 4 months ago (2009-08-07 21:26:38 UTC) #2

chase

On 2009/08/07 21:26:38, John Abd-El-Malek wrote: > lgtm with the one nit, and also updating ...

11 years, 4 months ago (2009-08-07 22:56:11 UTC) #3

jam

11 years, 4 months ago (2009-08-07 23:33:26 UTC) #4

lgtm

being extra nitty, the issue I have with the "Test" line is that it passes
before and after this change.  The Test line should really be something that
didn't work before, and now works.  so it should specify that the same plugin
process is still being used.

http://codereview.chromium.org/164157/diff/8/9
File chrome/plugin/plugin_channel.cc (right):

http://codereview.chromium.org/164157/diff/8/9#newcode28
Line 28: static const int kPluginReleaseTimeMS = 10000;
nit: the convention is to use "Ms"

Expand Messages | Collapse Messages