Chromium Code Reviews
Help | Chromium Project | Sign in
(8)

Issue 2827763002: Remove context checks in CurrentActivityLogger and CurrentActivityLoggerIfIsolatedWorld (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 4 days ago by adithyas
Modified:
1 week, 2 days ago
CC:
blink-reviews, blink-reviews-bindings_chromium.org, blink-reviews-dom_chromium.org, blink-reviews-frames_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, rwlbuis, sof, Yoav Weiss
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove context checks in CurrentActivityLogger and CurrentActivityLoggerIfIsolatedWorld The checks appear to be unnecessary as isolate->GetCurrentContext() should return a valid context object if isolate->InContext() returns true, and the script state field is set on v8::Context objects immediately after they are created, so the context should be properly initialized. BUG=682322 Review-Url: https://codereview.chromium.org/2827763002 Cr-Commit-Position: refs/heads/master@{#466004} Committed: https://chromium.googlesource.com/chromium/src/+/bd0add4325a532ca0a0d9c23eaa55f0084317b6f

Patch Set 1 #

Patch Set 2 : Remove toLocalDOMWindow checks #

Messages

Total messages: 25 (14 generated)
adithyas
I'm not exactly sure what purpose the !ToLocalDOMWindow() check serves, my guess is that it ...
1 week, 4 days ago (2017-04-18 19:34:40 UTC) #4
haraken
On 2017/04/18 19:34:40, adithyas wrote: > I'm not exactly sure what purpose the !ToLocalDOMWindow() check ...
1 week, 4 days ago (2017-04-18 20:25:25 UTC) #7
Yuki(ooo_til_may8)
On 2017/04/18 20:25:25, haraken wrote: > On 2017/04/18 19:34:40, adithyas wrote: > > I'm not ...
1 week, 3 days ago (2017-04-19 09:45:35 UTC) #8
Yuki(ooo_til_may8)
On 2017/04/19 09:45:35, Yuki wrote: > On 2017/04/18 20:25:25, haraken wrote: > > On 2017/04/18 ...
1 week, 3 days ago (2017-04-19 09:46:45 UTC) #9
adithyas
On 2017/04/19 at 09:46:45, yukishiino wrote: > On 2017/04/19 09:45:35, Yuki wrote: > > On ...
1 week, 3 days ago (2017-04-19 17:05:51 UTC) #10
jbroman
On 2017/04/19 at 17:05:51, adithyas wrote: > On 2017/04/19 at 09:46:45, yukishiino wrote: > > ...
1 week, 3 days ago (2017-04-19 20:35:25 UTC) #11
adithyas
On 2017/04/19 at 20:35:25, jbroman wrote: > On 2017/04/19 at 17:05:51, adithyas wrote: > > ...
1 week, 3 days ago (2017-04-19 21:30:27 UTC) #14
haraken
LGTM
1 week, 3 days ago (2017-04-19 21:56:50 UTC) #15
Yuki(ooo_til_may8)
LGTM. Could you update the CL subject and description?
1 week, 2 days ago (2017-04-20 09:26:16 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2827763002/20001
1 week, 2 days ago (2017-04-20 14:36:13 UTC) #22
commit-bot: I haz the power
1 week, 2 days ago (2017-04-20 14:41:18 UTC) #25
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/bd0add4325a532ca0a0d9c23eaa5...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46