Issue 24715003: [oilpan] Fix NPObject bindings by not using HandleScopes in WebBindings for non-webcore specific me…

Issue 24715003: [oilpan] Fix NPObject bindings by not using HandleScopes in WebBindings for non-webcore specific me… (Closed)

Created:
7 years, 2 months ago by Mads Ager (chromium)

Modified:
7 years, 2 months ago

Reviewers:
Vyacheslav Egorov (Google), Vyacheslav Egorov (Chromium), zerny-chromium, Erik Corry, haraken, wibling-chromium

CC:
blink-reviews, adamk+oilpan_chromium.org, abarth-chromium

Base URL:
svn://svn.chromium.org/blink/branches/oilpan

Visibility:
Public.

More Reviews

Description

[oilpan] Fix NPObject bindings by not using HandleScopes in WebBindings for non-webcore specific methods. R=vegorov@google.com, zerny@chromium.org, erik.corry@gmail.com, haraken@chromium.org, vegorov@chromium.org, wibling@chromium.org BUG= Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=158375

Patch Set 2 : Don't use NoHandleScope either #

Patch Set 5 : Address review comment. #

Created: 7 years, 2 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+26 lines, -13 lines)			Patch
	M	Source/WebKit/chromium/src/WebBindings.cpp	View	1 2 3 4	7 chunks	+26 lines, -13 lines	0 comments	Download

Messages

Total messages: 8 (0 generated)

Expand Messages | Collapse Messages

Mads Ager (chromium)

Maybe I should add explicit comments stating why there is not HandleScopes nor NoHandleScopes here.

7 years, 2 months ago (2013-09-26 11:18:58 UTC) #2

zerny-chromium

lgtm https://codereview.chromium.org/24715003/diff/7001/Source/WebKit/chromium/src/WebBindings.cpp File Source/WebKit/chromium/src/WebBindings.cpp (right): https://codereview.chromium.org/24715003/diff/7001/Source/WebKit/chromium/src/WebBindings.cpp#newcode215 Source/WebKit/chromium/src/WebBindings.cpp:215: { Nit: comment here too?

7 years, 2 months ago (2013-09-26 11:27:52 UTC) #4

Mads Ager (chromium)

https://codereview.chromium.org/24715003/diff/7001/Source/WebKit/chromium/src/WebBindings.cpp File Source/WebKit/chromium/src/WebBindings.cpp (right): https://codereview.chromium.org/24715003/diff/7001/Source/WebKit/chromium/src/WebBindings.cpp#newcode215 Source/WebKit/chromium/src/WebBindings.cpp:215: { On 2013/09/26 11:27:52, zerny-chromium wrote: > Nit: comment ...

7 years, 2 months ago (2013-09-26 11:28:56 UTC) #5

Mads Ager (chromium)

Committed patchset #5 manually as r158375 (presubmit successful).

7 years, 2 months ago (2013-09-26 11:29:57 UTC) #6

haraken

LGTM, but how did you select these methods from WebBindings? Manual investigation, or compile error ...

7 years, 2 months ago (2013-09-26 14:24:19 UTC) #7

Mads Ager (chromium)

7 years, 2 months ago (2013-09-26 14:30:18 UTC) #8

Message was sent while issue was closed.

On 2013/09/26 14:24:19, haraken wrote:
> LGTM, but how did you select these methods from WebBindings? Manual
> investigation, or compile error in some bulids?

Mainly manual inspection based on hints from code like this:

https://code.google.com/p/chromium/codesearch#chromium/src/content/child/npap...

which avoids calling the WebCore specific code for plugin processes.

Expand Messages | Collapse Messages