Fixes for issues

Fixes for issues

30504 - https://github.com/dart-lang/sdk/issues/30504
gen_snapshot fails to run from kernel

30517 - https://github.com/dart-lang/sdk/issues/30517
vm-kernel-precomp 'expected: !dfe.UseDartFrontend() && !dfe.kernel_file_specified()' tests regression

R=aam@google.com

Committed: https://github.com/dart-lang/sdk/commit/eac18a324c168de509f056a2bfcd3cdb56a312e4

[siva, 2017-08-24 18:09:50]

Description was changed from

==========
Fixes for issues
==========

to

==========
Fixes for issues

30504 - https://github.com/dart-lang/sdk/issues/30504
gen_snapshot fails to run from kernel    

30517 - https://github.com/dart-lang/sdk/issues/30517
vm-kernel-precomp 'expected: !dfe.UseDartFrontend() &&
!dfe.kernel_file_specified()' tests regression
==========

[siva, 2017-08-24 18:10:16]

asiva@google.com changed reviewers:
+ aam@google.com

[aam, 2017-08-24 19:20:06]

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

https://codereview.chromium.org/3004563002/diff/1/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:5035: // returned by the kernel reader.
We know that this will happen when we request to load .dill file, in which case
url string, pointing to .dill file, is different from script name (by '.dill'
extension). Is there some other scenario where LookupLibrary(resolved_url_str)
returns null? Are we okay falling back to using library object returned by the
kernel reader in those cases?

[siva, 2017-08-24 23:00:42]

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

https://codereview.chromium.org/3004563002/diff/1/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:5035: // returned by the kernel reader.
On 2017/08/24 19:20:06, aam wrote:
> We know that this will happen when we request to load .dill file, in which
case
> url string, pointing to .dill file, is different from script name (by '.dill'
> extension). Is there some other scenario where LookupLibrary(resolved_url_str)
> returns null? Are we okay falling back to using library object returned by the
> kernel reader in those cases?

The scenarios when this can happen is when resolved_url_str does not match the
url string for the library objects. Currently the only known case for this is
the '.dill' extension case.

[aam, 2017-08-24 23:17:01]

On 2017/08/24 23:00:42, siva wrote:
> https://codereview.chromium.org/3004563002/diff/1/runtime/vm/dart_api_impl.cc
> File runtime/vm/dart_api_impl.cc (right):
> 
>
https://codereview.chromium.org/3004563002/diff/1/runtime/vm/dart_api_impl.cc...
> runtime/vm/dart_api_impl.cc:5035: // returned by the kernel reader.
> On 2017/08/24 19:20:06, aam wrote:
> > We know that this will happen when we request to load .dill file, in which
> case
> > url string, pointing to .dill file, is different from script name (by
'.dill'
> > extension). Is there some other scenario where
LookupLibrary(resolved_url_str)
> > returns null? Are we okay falling back to using library object returned by
the
> > kernel reader in those cases?
> 
> The scenarios when this can happen is when resolved_url_str does not match the
> url string for the library objects. Currently the only known case for this is
> the '.dill' extension case.

lgtm!

As discussed, since for this .dill-file loading we fall back to 'fragile' kernel
loading behavior of looking for 'main' method to determine main library, let's
have a bug to track resolution of that.

[siva, 2017-08-25 16:28:18]

On 2017/08/24 23:17:01, aam wrote:
> On 2017/08/24 23:00:42, siva wrote:
> >
https://codereview.chromium.org/3004563002/diff/1/runtime/vm/dart_api_impl.cc
> > File runtime/vm/dart_api_impl.cc (right):
> > 
> >
>
https://codereview.chromium.org/3004563002/diff/1/runtime/vm/dart_api_impl.cc...
> > runtime/vm/dart_api_impl.cc:5035: // returned by the kernel reader.
> > On 2017/08/24 19:20:06, aam wrote:
> > > We know that this will happen when we request to load .dill file, in which
> > case
> > > url string, pointing to .dill file, is different from script name (by
> '.dill'
> > > extension). Is there some other scenario where
> LookupLibrary(resolved_url_str)
> > > returns null? Are we okay falling back to using library object returned by
> the
> > > kernel reader in those cases?
> > 
> > The scenarios when this can happen is when resolved_url_str does not match
the
> > url string for the library objects. Currently the only known case for this
is
> > the '.dill' extension case.
> 
> lgtm!
> 
> As discussed, since for this .dill-file loading we fall back to 'fragile'
kernel
> loading behavior of looking for 'main' method to determine main library, let's
> have a bug to track resolution of that.

We already have a bug tracking this
(https://github.com/dart-lang/sdk/issues/29836)

[siva, 2017-08-25 17:05:16]

Description was changed from

==========
Fixes for issues

30504 - https://github.com/dart-lang/sdk/issues/30504
gen_snapshot fails to run from kernel    

30517 - https://github.com/dart-lang/sdk/issues/30517
vm-kernel-precomp 'expected: !dfe.UseDartFrontend() &&
!dfe.kernel_file_specified()' tests regression
==========

to

==========
Fixes for issues

30504 - https://github.com/dart-lang/sdk/issues/30504
gen_snapshot fails to run from kernel

30517 - https://github.com/dart-lang/sdk/issues/30517
vm-kernel-precomp 'expected: !dfe.UseDartFrontend() &&
!dfe.kernel_file_specified()' tests regression

R=aam@google.com

Committed:
https://github.com/dart-lang/sdk/commit/eac18a324c168de509f056a2bfcd3cdb56a312e4
==========

[siva, 2017-08-25 17:05:22]

Committed patchset #1 (id:1) manually as
eac18a324c168de509f056a2bfcd3cdb56a312e4 (presubmit successful).

[aam, 2017-08-25 21:56:03]

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

https://codereview.chromium.org/3004563002/diff/1/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:5036: library ^= Api::UnwrapHandle(result);
Sorry, I didn't realize that `result` here will be Dart_Null if
LoadKernelProgram at line 5028 succeeded.

[siva, 2017-08-25 21:59:47]

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

https://codereview.chromium.org/3004563002/diff/1/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:5036: library ^= Api::UnwrapHandle(result);
On 2017/08/25 21:56:03, aam wrote:
> Sorry, I didn't realize that `result` here will be Dart_Null if
> LoadKernelProgram at line 5028 succeeded.

Good point will fix that.