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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 419103003: Handle load errors in deferred code (Closed)

Created:
6 years, 4 months ago by hausner
Modified:
6 years, 4 months ago
Reviewers:
Ivan Posva
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Visibility:
Public.

Description

Handle load errors in deferred code IO errors on the URL of a deferred library are forwarded to the Future that handles the deferred load. Syntax errors and finalization errors are lethal, killing the isolate. Committed: https://code.google.com/p/dart/source/detail?r=38800

Patch Set 1 #

Total comments: 14

Patch Set 2 : #

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+184 lines, -34 lines) Patch
M runtime/bin/builtin.dart View 1 8 chunks +17 lines, -12 lines 0 comments Download
M runtime/bin/builtin_natives.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M runtime/bin/dartutils.cc View 1 3 chunks +27 lines, -5 lines 0 comments Download
M runtime/include/dart_api.h View 1 1 chunk +15 lines, -0 lines 0 comments Download
M runtime/lib/lib_prefix.dart View 2 chunks +8 lines, -3 lines 0 comments Download
M runtime/lib/object.cc View 1 chunk +13 lines, -0 lines 0 comments Download
M runtime/vm/bootstrap.cc View 1 chunk +3 lines, -1 line 0 comments Download
M runtime/vm/bootstrap_natives.h View 1 chunk +1 line, -0 lines 0 comments Download
M runtime/vm/dart_api_impl.cc View 4 chunks +33 lines, -2 lines 0 comments Download
M runtime/vm/debugger.cc View 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/debugger_api_impl.cc View 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/object.h View 2 chunks +7 lines, -3 lines 0 comments Download
M runtime/vm/object.cc View 4 chunks +46 lines, -4 lines 0 comments Download
M runtime/vm/object_store.h View 2 chunks +8 lines, -0 lines 0 comments Download
M runtime/vm/object_store.cc View 1 chunk +1 line, -0 lines 0 comments Download
M runtime/vm/raw_object.h View 1 chunk +2 lines, -1 line 0 comments Download
M tests/standalone/issue14236_test.dart View Binary file 0 comments Download

Messages

Total messages: 4 (0 generated)
hausner
6 years, 4 months ago (2014-07-28 18:42:41 UTC) #1
Ivan Posva
https://codereview.chromium.org/419103003/diff/1/runtime/bin/dartutils.cc File runtime/bin/dartutils.cc (right): https://codereview.chromium.org/419103003/diff/1/runtime/bin/dartutils.cc#newcode721 runtime/bin/dartutils.cc:721: // The and IO error is not fatal. Comment ...
6 years, 4 months ago (2014-07-31 18:29:05 UTC) #2
hausner
Committed patchset #3 manually as r38800 (presubmit successful).
6 years, 4 months ago (2014-07-31 21:09:54 UTC) #3
hausner
6 years, 4 months ago (2014-07-31 21:12:45 UTC) #4
Message was sent while issue was closed. 
https://codereview.chromium.org/419103003/diff/1/runtime/bin/dartutils.cc
File runtime/bin/dartutils.cc (right):

https://codereview.chromium.org/419103003/diff/1/runtime/bin/dartutils.cc#new...
runtime/bin/dartutils.cc:721: // The and IO error is not fatal.
On 2014/07/31 18:29:04, Ivan Posva wrote:
> Comment does not parse.

Done.

https://codereview.chromium.org/419103003/diff/1/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/419103003/diff/1/runtime/include/dart_api.h#n...
runtime/include/dart_api.h:2632: * \param error The error object containing the
load error.
On 2014/07/31 18:29:04, Ivan Posva wrote:
> Please reword the comment to signal that this is an instance object.

Done.

https://codereview.chromium.org/419103003/diff/1/runtime/lib/object.cc
File runtime/lib/object.cc (right):

https://codereview.chromium.org/419103003/diff/1/runtime/lib/object.cc#newcod...
runtime/lib/object.cc:301: const Object& error =
Object::Handle(prefix.LoadError());
On 2014/07/31 18:29:04, Ivan Posva wrote:
> In that case this should be:
> const Instance& error = Instance::Handle(prefix.LoadError());

Done.

https://codereview.chromium.org/419103003/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/419103003/diff/1/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:9888: pending_deferred_loads.Add(deferred_lib);
On 2014/07/31 18:29:04, Ivan Posva wrote:
> Please make sure this is reset when the loads are finished.

Added to Dart_FinalizeLoading()

https://codereview.chromium.org/419103003/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/419103003/diff/1/runtime/vm/object.h#newcode2640
runtime/vm/object.h:2640: RawObject* LoadError() const { return
raw_ptr()->load_error_; }
On 2014/07/31 18:29:05, Ivan Posva wrote:
> RawInstance* here and other related places.

Done.

https://codereview.chromium.org/419103003/diff/1/runtime/vm/object.h#newcode4374
runtime/vm/object.h:4374: RawObject* LoadError() const;
On 2014/07/31 18:29:05, Ivan Posva wrote:
> Thinking about this more as I go through the CL. Can we name this something
> different from Error to reduce the potential for confusion?

Leaving the name but changing to type Instance, as discussed offline.

https://codereview.chromium.org/419103003/diff/1/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

https://codereview.chromium.org/419103003/diff/1/runtime/vm/raw_object.h#newc...
runtime/vm/raw_object.h:801: RawObject* load_error_;        // Error iff
load_state_ == kLoadError.
On 2014/07/31 18:29:05, Ivan Posva wrote:
> RawInstance*

Done.

Powered by Google App Engine
This is Rietveld 408576698