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

Issue 1130293002: dart2js: compute the currentScript in the preamble for d8 and jsshell. (Closed)

Created:
5 years, 7 months ago by floitsch
Modified:
5 years, 7 months ago
Reviewers:
herhut, sra1
CC:
reviews_dartlang.org, herhut
Target Ref:
refs/remotes/git-svn
Visibility:
Public.

Description

dart2js: compute the currentScript in the preamble for d8 and jsshell. This change enables deferred loading for the lazy emitter on d8. Without it the computeThisScript function would be executed within an 'evaled' piece of code, and thus not find the correct URL. This change also reduces the code of the generated code (by a tiny amount), R=herhut@google.com Committed: https://code.google.com/p/dart/source/detail?r=45671

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+80 lines, -12 lines) Patch
M sdk/lib/_internal/compiler/js_lib/isolate_helper.dart View 1 chunk +3 lines, -12 lines 3 comments Download
M sdk/lib/_internal/compiler/js_lib/preambles/d8.js View 1 chunk +37 lines, -0 lines 0 comments Download
M sdk/lib/_internal/compiler/js_lib/preambles/jsshell.js View 1 chunk +40 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (2 generated)
floitsch
https://codereview.chromium.org/1130293002/diff/1/sdk/lib/_internal/compiler/js_lib/isolate_helper.dart File sdk/lib/_internal/compiler/js_lib/isolate_helper.dart (right): https://codereview.chromium.org/1130293002/diff/1/sdk/lib/_internal/compiler/js_lib/isolate_helper.dart#newcode761 sdk/lib/_internal/compiler/js_lib/isolate_helper.dart:761: var currentScript = JS_EMBEDDED_GLOBAL('', CURRENT_SCRIPT); The embedded global is ...
5 years, 7 months ago (2015-05-07 21:09:51 UTC) #2
herhut
lgtm https://codereview.chromium.org/1130293002/diff/1/sdk/lib/_internal/compiler/js_lib/isolate_helper.dart File sdk/lib/_internal/compiler/js_lib/isolate_helper.dart (left): https://codereview.chromium.org/1130293002/diff/1/sdk/lib/_internal/compiler/js_lib/isolate_helper.dart#oldcode764 sdk/lib/_internal/compiler/js_lib/isolate_helper.dart:764: if (Primitives.isD8) return computeThisScriptD8(); Do we still need ...
5 years, 7 months ago (2015-05-08 08:06:27 UTC) #4
floitsch
Committed patchset #1 (id:1) manually as 45671 (presubmit successful).
5 years, 7 months ago (2015-05-09 00:59:31 UTC) #5
floitsch
https://codereview.chromium.org/1130293002/diff/1/sdk/lib/_internal/compiler/js_lib/isolate_helper.dart File sdk/lib/_internal/compiler/js_lib/isolate_helper.dart (left): https://codereview.chromium.org/1130293002/diff/1/sdk/lib/_internal/compiler/js_lib/isolate_helper.dart#oldcode764 sdk/lib/_internal/compiler/js_lib/isolate_helper.dart:764: if (Primitives.isD8) return computeThisScriptD8(); On 2015/05/08 08:06:27, herhut wrote: ...
5 years, 7 months ago (2015-05-09 01:07:54 UTC) #6
sra1
lgtm. If the stack trace string format changes, is there a test that fails that ...
5 years, 7 months ago (2015-05-11 16:35:06 UTC) #7
floitsch
5 years, 7 months ago (2015-05-12 00:21:46 UTC) #8
Message was sent while issue was closed.
On 2015/05/11 16:35:06, sra1 wrote:
> lgtm.
> 
> If the stack trace string format changes, is there a test that fails that
makes
> it obvious that this is the problem?

All deferred library tests would fail (and an additional test would likely
drown).
However, if the format changes, it is also very likely that the regexp wouldn't
match anymore, and that you would get a null-pointer exception in the return:
'return lastMatch[1]'.

Powered by Google App Engine
This is Rietveld 408576698