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

Issue 200116: Fix bug where content scripts can get GC'd if they don't attach (Closed)

Created:
11 years, 3 months ago by Aaron Boodman
Modified:
9 years, 6 months ago
Reviewers:
Matt Perry
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Fix bug where content scripts can get GC'd if they don't attach to any DOM events. BUG=17410 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=26136

Patch Set 1 #

Patch Set 2 : remove extra braces #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -5 lines) Patch
M chrome/renderer/extensions/bindings_utils.h View 2 chunks +9 lines, -1 line 0 comments Download
M chrome/renderer/extensions/bindings_utils.cc View 1 chunk +9 lines, -0 lines 0 comments Download
M chrome/renderer/extensions/event_bindings.cc View 1 4 chunks +15 lines, -4 lines 2 comments Download

Messages

Total messages: 3 (0 generated)
Aaron Boodman
11 years, 3 months ago (2009-09-14 05:55:36 UTC) #1
Matt Perry
woohoo! lgtm http://codereview.chromium.org/200116/diff/1004/7 File chrome/renderer/extensions/event_bindings.cc (right): http://codereview.chromium.org/200116/diff/1004/7#newcode86 Line 86: bool allow_api = has_permission; may as ...
11 years, 3 months ago (2009-09-14 18:07:46 UTC) #2
Aaron Boodman
11 years, 3 months ago (2009-09-14 18:59:33 UTC) #3
On 2009/09/14 18:07:46, Matt Perry wrote:
> http://codereview.chromium.org/200116/diff/1004/7#newcode121
> Line 121: if (current_context_info &&
> why the NULL-check here but not in AttachEvent?

Because in the case where the context got GC'd, we have already removed context
info from the list at this point (in UnregisterContext).

Powered by Google App Engine
This is Rietveld 408576698