Issue 367263004: Oilpan: fix build after r177450.

Issue 367263004: Oilpan: fix build after r177450. (Closed)

Created:
6 years, 5 months ago by sof

Modified:
6 years, 5 months ago

Reviewers:
haraken, oilpan-reviews, dominicc (has gone to gerrit)

CC:
blink-reviews, blink-reviews-bindings_chromium.org, arv+blink, abarth-chromium

Base URL:
https://chromium.googlesource.com/chromium/blink.git@master

Project:
blink

Visibility:
Public.

Description

Oilpan: fix build after r177450. Update ScriptPromise tests to be Oilpan compatible. R=haraken@chromium.org BUG=363967 NOTRY=true Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=177463

Patch Set 1 #

Patch Set 2 : Move ScriptPromisePropertyBase to the heap #

Created: 6 years, 5 months ago

	Unified diffs	Side-by-side diffs	Stats (+24 lines, -13 lines)
M	Source/bindings/core/v8/ScriptPromiseProperty.h	View	3 chunks	+10 lines, -1 line	0 comments
M	Source/bindings/core/v8/ScriptPromisePropertyBase.h	View	2 chunks	+3 lines, -1 line	0 comments
M	Source/bindings/core/v8/ScriptPromisePropertyTest.cpp	View	7 chunks	+11 lines, -11 lines	0 comments

Messages

Total messages: 10 (0 generated)

Expand Messages | Collapse Messages

sof

On 2014/07/03 09:07:41, haraken wrote: > LGTM. Sorry I reviewed the CL. Thanks - if ...

6 years, 5 months ago (2014-07-03 09:10:25 UTC) #3

sof

On 2014/07/03 09:10:25, sof wrote: > On 2014/07/03 09:07:41, haraken wrote: > > LGTM. Sorry ...

6 years, 5 months ago (2014-07-03 09:47:29 UTC) #4

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sigbjornf@opera.com/367263004/20001

6 years, 5 months ago (2014-07-03 10:01:29 UTC) #6

dominicc (has gone to gerrit)

On 2014/07/03 10:27:23, haraken wrote: > still LGTM Um... what about tracing the resolved and ...

6 years, 5 months ago (2014-07-03 23:56:44 UTC) #9

haraken

6 years, 5 months ago (2014-07-04 00:52:01 UTC) #10

Message was sent while issue was closed.

On 2014/07/03 23:56:44, dominicc wrote:
> On 2014/07/03 10:27:23, haraken wrote:
> > still LGTM
> 
> Um... what about tracing the resolved and rejected values?
> 
> There was feedback on this review that because this is a templatized type, it
> shouldn't be Oilpanified yet? https://codereview.chromium.org/361863003

Sorry, I was confused. I was missing the fact that oilpaned objects can be
passed to HolderType. Also I was missing the fact that oilpaned objects can be
also passed to ResolvedType and RejectedType (but currently not by accident).

So I think you're right that we need to trace all of m_holder, m_resolved and
m_rejected.

Once someone passes not-yet-oilpaned objects to
HolderType/ResolvedType/RejectedType, it might cause errors, but let's worry
about it when it actually happens.

Expand Messages | Collapse Messages