Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[html] Crash when ImageElement.src is not a valid image uri #19948

Closed
DartBot opened this issue Jul 10, 2014 · 9 comments
Closed

[html] Crash when ImageElement.src is not a valid image uri #19948

DartBot opened this issue Jul 10, 2014 · 9 comments
Assignees
@leafpetersen
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures
Milestone
1.6

Comments

@DartBot
Copy link

DartBot commented Jul 10, 2014

This issue was originally filed by Ilya.Vaz...@gmail.com

Crash on content shell and timeout on dartium

tests/co19/src/LayoutTests/fast/canvas/drawImage-with-broken-image_t01.dart

main() {
  var ctx, invalidImage;

  draw(_) {
    // null images should throw TypeError
    shouldThrow(() => ctx.drawImage(null, 0, 0));
    shouldThrow(() => ctx.drawImageScaled(null, 0, 0, 20, 20));
    shouldThrow(() => ctx. drawImageScaledFromSource(null, 0, 0, 20, 20, 0, 0, 20, 20));

    // broken images should not throw
    shouldThrow(() => ctx.drawImage(invalidImage, 0, 0));
    shouldThrow(() => ctx.drawImageScaled(invalidImage, 0, 0, 20, 20));
    shouldThrow(() => ctx.drawImageScaledFromSource(invalidImage, 0, 0, 20, 20, 0, 0, 20, 20));
    shouldThrow(() => ctx.drawImageScaled(invalidImage, 0, 0, 0, 20));
    shouldThrow(() => ctx.drawImageScaledFromSource(invalidImage, 0, 0, 0, 20, 0, 0, 20, 20));

    asyncEnd();
  }

  asyncStart();

  // Create an image with invalid data.
  invalidImage = new ImageElement();
  invalidImage.onError.listen(draw);
  invalidImage.src = '$root/drawImage-with-broken-image_t01.dart'; // test's file itself

  ctx = document.createElement("canvas").getContext('2d');
}
@anders-sandholm
Copy link
Contributor

Crash seems bad. Feel free to re-assign prio and milestone though.

Added this to the 1.6 milestone.
Removed Priority-Unassigned label.
Added Priority-High, Area-Dartium, Triaged labels.

@vsmenon
Copy link
Member

vsmenon commented Jul 15, 2014

leaf : can you take a look at the crash?

Set owner to @leafpetersen.

@leafpetersen
Copy link
Member

Can you tell me where to get this test file? There's no such file in the dart or dartium repositories that I can find. The fragment above is incomplete, but I tried shortening it to be self contained as follows:

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
<head>
</head>
<script type="application/dart">
import 'dart:html';
main() {
  var ctx, invalidImage;

  draw(_) {
    ctx.drawImage(invalidImage, 0, 0);
  }

  // Create an image with invalid data.
  invalidImage = new ImageElement();
  invalidImage.onError.listen(draw);
  invalidImage.src = 'drawImage-bug.html'; // test's file itself

  ctx = document.createElement("canvas").getContext('2d');
}
</script>
</body>
</html>

This runs correctly (or at least, does not crash for me) in current dartium/dart bleeding edge.

Added NeedsInfo label.

@DartBot
Copy link
Author

DartBot commented Jul 23, 2014

This comment was originally written by Ilya.Vazh...@gmail.com

gclient sync updates to some snapshot of co19 tests. To get latest version:

cd $dart-ws/tests/co19/src
svn up

cd ../../../

./tools/test.py -t10 -ax64 -mrelease -rdrt co19/LayoutTests/fast/canvas/drawImage-with-broken-image

#CRASH

@DartBot
Copy link
Author

DartBot commented Jul 23, 2014

This comment was originally written by Ilya.V...@gmail.com

To enable running co19 tests on drt target comment the line in config file:

$ svn diff tests/co19/co19-dartium.status
Index: tests/co19/co19-dartium.status
===================================================================

--- tests/co19/co19-dartium.status (revision 38449)
+++ tests/co19/co19-dartium.status (working copy)
@@ -3,7 +3,7 @­@
 # BSD-style license that can be found in the LICENSE file.
 
 [ $compiler == none && $runtime == drt ]
-: Skip # running co19 tests on content_shell would make our dartium cycle-times very long
+#: Skip # running co19 tests on content_shell would make our dartium cycle-times very long
 
 [ $compiler == none && ($runtime == dartium || $runtime == ContentShellOnAndroid) ]
 # Temporarily skip tests, to narrow down new breakage to a particular test by binary search:

@leafpetersen
Copy link
Member

Great, thanks, I can reproduce. Looking into it.

Added Started label.

@leafpetersen
Copy link
Member

Ok, there are two things going on here. The first is that we are generating the dart:html bindings from old chrome 34 IDL (see dartbug/20123). In that IDL, the first argument to drawImage was nullable (meaning that the underlying blink code was prepared to deal with null values (by throwing an error). In the new IDL, this argument is no longer nullable, and the underlying blink code will simply crash if passed a null ptr. So, to really fix this, I think we need to do the following:

  1. Update to the new IDL
  2. Possibly change what we do with nullable arguments in overloading resolution
  3. Do the right thing when null values are passed into the bindings code for non-nullable parameters.

Marked this as being blocked by #20123, #20172.

@leafpetersen
Copy link
Member

This should no longer crash as of https://codereview.chromium.org/469373002/ .

@leafpetersen
Copy link
Member

Fixed in r39299

Added Fixed label.

@DartBot DartBot added Type-Defect P1 A high priority bug; for example, a single project is unusable or has many test failures labels Aug 20, 2014
@DartBot DartBot added this to the 1.6 milestone Aug 20, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@leafpetersen leafpetersen

Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Milestone
1.6
Development

No branches or pull requests
4 participants
@vsmenon @anders-sandholm @leafpetersen @DartBot