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

Issue 10781019: Fix compilation for Opera. (Closed)

Created:
8 years, 5 months ago by floitsch
Modified:
8 years, 5 months ago
Reviewers:
vsm, sra1
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Fix compilation for Opera. 2 issues: - Opera claims to support __proto__ but for some cases it is buggy: it can happen that the changing of __proto__ makes the object lose fields. - Opera sometimes returns "Function.prototype" for the 'name'-property of the constructor of DOM objects. The first issue is fixed with a supportsProtoCheck. To do this, I slightly refactored the defineClass method, so it doesn't have a side-effect (except for the arguments that are passed in). This also made it necessary to use the "new" syntax for BoundClosure classes: $$.BoundClosure = { ... }; Normally this is nicer anyways. The only problem could be that I missed a case where we generate a bound closure and do not call "finishClasses" afterwards. Fixes issue 3812. Committed: https://code.google.com/p/dart/source/detail?r=9736

Patch Set 1 #

Total comments: 2

Patch Set 2 : Address comment. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -21 lines) Patch
M lib/compiler/implementation/emitter.dart View 1 10 chunks +39 lines, -20 lines 0 comments Download
M lib/compiler/implementation/lib/native_helper.dart View 3 chunks +10 lines, -1 line 0 comments Download

Messages

Total messages: 3 (0 generated)
floitsch
8 years, 5 months ago (2012-07-16 17:26:33 UTC) #1
vsm
LGTM! Can you add a test? I added a short one to the bug. https://chromiumcodereview.appspot.com/10781019/diff/1/lib/compiler/implementation/emitter.dart ...
8 years, 5 months ago (2012-07-18 05:33:31 UTC) #2
floitsch
8 years, 5 months ago (2012-07-18 12:19:33 UTC) #3
I'm not completely sure how to add an Opera test.
I will commit this CL, and then we can discuss how to add the test(s) offline.

https://chromiumcodereview.appspot.com/10781019/diff/1/lib/compiler/implement...
File lib/compiler/implementation/emitter.dart (right):

https://chromiumcodereview.appspot.com/10781019/diff/1/lib/compiler/implement...
lib/compiler/implementation/emitter.dart:133: // On
On 2012/07/18 05:33:31, vsm wrote:
> Delete last line.

Done.

Powered by Google App Engine
This is Rietveld 408576698