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

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

Created:
3 years, 8 months ago by adithyas
Modified:
3 years, 8 months ago
Reviewers:
haraken, jbroman, Yuki
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 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -4 lines) Patch
M third_party/WebKit/Source/bindings/core/v8/V8DOMActivityLogger.cpp View 1 2 chunks +0 lines, -4 lines 0 comments Download

Messages

Total messages: 25 (14 generated)
adithyas
I'm not exactly sure what purpose the !ToLocalDOMWindow() check serves, my guess is that it ...
3 years, 8 months 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 ...
3 years, 8 months ago (2017-04-18 20:25:25 UTC) #7
Yuki
On 2017/04/18 20:25:25, haraken wrote: > On 2017/04/18 19:34:40, adithyas wrote: > > I'm not ...
3 years, 8 months ago (2017-04-19 09:45:35 UTC) #8
Yuki
On 2017/04/19 09:45:35, Yuki wrote: > On 2017/04/18 20:25:25, haraken wrote: > > On 2017/04/18 ...
3 years, 8 months 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 ...
3 years, 8 months 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: > > ...
3 years, 8 months 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: > > ...
3 years, 8 months ago (2017-04-19 21:30:27 UTC) #14
haraken
LGTM
3 years, 8 months ago (2017-04-19 21:56:50 UTC) #15
Yuki
LGTM. Could you update the CL subject and description?
3 years, 8 months 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
3 years, 8 months ago (2017-04-20 14:36:13 UTC) #22
commit-bot: I haz the power
3 years, 8 months 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...

Powered by Google App Engine
This is Rietveld 408576698