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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1171463003: Oilpan: fix build after r196513. (Closed)

Created:
5 years, 6 months ago by sof
Modified:
5 years, 6 months ago
CC:
blink-reviews, caseq+blink_chromium.org, arv+blink, vivekg_samsung, vivekg, yurys+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, blink-reviews-bindings_chromium.org, devtools-reviews_chromium.org, apavlov+blink_chromium.org, sergeyv+blink_chromium.org, kozyatinskiy+blink_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Oilpan: fix build after r196513. TBR=oilpan-reviews BUG=466631 NOTRY=true Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196524

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -5 lines) Patch
M Source/bindings/core/v8/inspector/V8InjectedScriptHost.h View 2 chunks +2 lines, -1 line 2 comments Download
M Source/bindings/core/v8/inspector/V8InjectedScriptHost.cpp View 2 chunks +4 lines, -4 lines 1 comment Download

Messages

Total messages: 9 (2 generated)
sof
please take a look. oilpan-reviews@'s offer to quickly look over any CL that involves Oilpan, ...
5 years, 6 months ago (2015-06-04 19:11:12 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1171463003/1
5 years, 6 months ago (2015-06-04 19:11:38 UTC) #4
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://src.chromium.org/viewvc/blink?view=rev&revision=196524
5 years, 6 months ago (2015-06-04 19:14:49 UTC) #5
sof
https://codereview.chromium.org/1171463003/diff/1/Source/bindings/core/v8/inspector/V8InjectedScriptHost.cpp File Source/bindings/core/v8/inspector/V8InjectedScriptHost.cpp (right): https://codereview.chromium.org/1171463003/diff/1/Source/bindings/core/v8/inspector/V8InjectedScriptHost.cpp#newcode636 Source/bindings/core/v8/inspector/V8InjectedScriptHost.cpp:636: RefPtrWillBePersistent<InjectedScriptHost> m_host; This could be too strong a reference ...
5 years, 6 months ago (2015-06-04 21:44:16 UTC) #6
haraken
LGTM
5 years, 6 months ago (2015-06-04 23:05:12 UTC) #7
yurys
https://codereview.chromium.org/1171463003/diff/1/Source/bindings/core/v8/inspector/V8InjectedScriptHost.h File Source/bindings/core/v8/inspector/V8InjectedScriptHost.h (right): https://codereview.chromium.org/1171463003/diff/1/Source/bindings/core/v8/inspector/V8InjectedScriptHost.h#newcode9 Source/bindings/core/v8/inspector/V8InjectedScriptHost.h:9: #include "wtf/Forward.h" Actually, since this code is going to ...
5 years, 6 months ago (2015-06-05 07:54:58 UTC) #8
sof
5 years, 6 months ago (2015-06-05 07:59:26 UTC) #9
Message was sent while issue was closed. 
https://codereview.chromium.org/1171463003/diff/1/Source/bindings/core/v8/ins...
File Source/bindings/core/v8/inspector/V8InjectedScriptHost.h (right):

https://codereview.chromium.org/1171463003/diff/1/Source/bindings/core/v8/ins...
Source/bindings/core/v8/inspector/V8InjectedScriptHost.h:9: #include
"wtf/Forward.h"
On 2015/06/05 07:54:58, yurys wrote:
> Actually, since this code is going to be move out of blink it cannot depend on
> Oilpan.  I wish we could avoid adding such dependencies as they place
additional
> burden both on you as you need to make it work with Oilpan and later on me
later
> as I'll need to convert it back to non-oilpan types.

Yeah, this is a time waster - breaks things on the Oilpan side atm. Trying to
address.

Powered by Google App Engine
This is Rietveld 408576698