[wasm][asm.js] Routing asm.js warnings to the dev console.

[wasm][asm.js] Routing asm.js warnings to the dev console.

Route asm.js warnings from V8 to the developer console.

Depends on: https://codereview.chromium.org/2526703002/

BUG=v8:4203, chromium:660016
R=dgozman@chromium.org,jochen@chromium.org
CC=danno@chromium.org,bmeurer@chromium.org

Committed: https://crrev.com/07d5887d7471da0cf7bacb87d9742927f83f0c6d
Cr-Commit-Position: refs/heads/master@{#438511}

[bradn, 2016-11-23 08:36:28]

bradnelson@google.com changed reviewers:
+ bradnelson@google.com

[bradnelson, 2016-11-29 06:39:27]

Ping?

[jochen (gone - plz use gerrit), 2016-11-29 15:45:11]

was it possible to use the regular message handler?

[dgozman, 2016-11-29 18:53:39]

On 2016/11/29 15:45:11, jochen wrote:
> was it possible to use the regular message handler?

Creating a separate but similar message handler was my suggestion, as I'm
worried that existing clients could break after receiving an unexpected message.
For example, blink treats all messages as errors, while this one is intended to
be a warning. Any suggestions maybe?

[blink-reviews, 2016-11-30 09:24:25]

Dimitry and I talked.
I'm going to change these two round so that jsmessage is shared (adds an
error level field). And add a parameter to AddMessageListener to select
which error levels to receive. It will default to the current behavior (ie
just errors).
How does this sound?

On Tue, Nov 29, 2016, 10:53 AM <dgozman@chromium.org> wrote:

> On 2016/11/29 15:45:11, jochen wrote:
> > was it possible to use the regular message handler?
>
> Creating a separate but similar message handler was my suggestion, as I'm
> worried that existing clients could break after receiving an unexpected
> message.
> For example, blink treats all messages as errors, while this one is
> intended to
> be a warning. Any suggestions maybe?
>
> https://codereview.chromium.org/2530503002/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-11-30 09:24:25]

Dimitry and I talked.
I'm going to change these two round so that jsmessage is shared (adds an
error level field). And add a parameter to AddMessageListener to select
which error levels to receive. It will default to the current behavior (ie
just errors).
How does this sound?

On Tue, Nov 29, 2016, 10:53 AM <dgozman@chromium.org> wrote:

> On 2016/11/29 15:45:11, jochen wrote:
> > was it possible to use the regular message handler?
>
> Creating a separate but similar message handler was my suggestion, as I'm
> worried that existing clients could break after receiving an unexpected
> message.
> For example, blink treats all messages as errors, while this one is
> intended to
> be a warning. Any suggestions maybe?
>
> https://codereview.chromium.org/2530503002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jochen (gone - plz use gerrit), 2016-11-30 19:02:49]

that sounds already better.

I wonder, however, have we considered adding the message directly from V8 to the
console, now that the console object is part of v8?

[jochen (gone - plz use gerrit), 2016-11-30 19:03:27]

(sorry for my delayed responses, somehow this CL never shows up as pending
feedback from me in the codereview dashboard..)

[dgozman, 2016-11-30 19:10:33]

On 2016/11/30 19:02:49, jochen wrote:
> that sounds already better.
> 
> I wonder, however, have we considered adding the message directly from V8 to
the
> console, now that the console object is part of v8?

We don't have such a mechanism yet, except for console.xxx APIs, but that's
user-land.

[bradnelson, 2016-12-01 09:33:07]

PTAL at this and the V8 side change.

[dgozman, 2016-12-01 18:38:34]

Is it possible to produce wasm warning in a test?

https://codereview.chromium.org/2530503002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp (right):

https://codereview.chromium.org/2530503002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp:167:
frame->document()->addConsoleMessage(ConsoleMessage::create(
ExecutionContext has addConsoleMessage method, let's just call it. There is no
need to artificially constrain messages to documents only.

[jochen (gone - plz use gerrit), 2016-12-05 15:42:41]

also, where will warnings from workers go?

[bradnelson, 2016-12-12 22:45:09]

The CQ bit was checked by bradnelson@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-12 22:46:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-12 23:05:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-12 23:05:56]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[bradnelson, 2016-12-12 23:10:40]

The CQ bit was checked by bradnelson@chromium.org to run a CQ dry run

[bradnelson, 2016-12-12 23:12:17]

The CQ bit was checked by bradnelson@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-12 23:12:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[bradn, 2016-12-12 23:13:20]

PTAL

Added tests, added worker support.

[commit-bot: I haz the power, 2016-12-12 23:26:36]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[commit-bot: I haz the power, 2016-12-12 23:38:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-12 23:38:28]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[bradnelson, 2016-12-13 06:49:12]

The CQ bit was checked by bradnelson@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-13 06:49:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 07:02:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-13 07:02:21]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[bradnelson, 2016-12-13 09:54:54]

The CQ bit was checked by bradnelson@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-13 09:55:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 11:39:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-13 11:39:35]

Dry run: This issue passed the CQ dry run.

[bradnelson, 2016-12-14 08:26:39]

https://codereview.chromium.org/2530503002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp (right):

https://codereview.chromium.org/2530503002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp:167:
frame->document()->addConsoleMessage(ConsoleMessage::create(
On 2016/12/01 18:38:33, dgozman wrote:
> ExecutionContext has addConsoleMessage method, let's just call it. There is no
> need to artificially constrain messages to documents only.

Done.

[bradnelson, 2016-12-14 08:26:50]

Ping?

[jochen (gone - plz use gerrit), 2016-12-14 11:42:43]

lgtm

https://codereview.chromium.org/2530503002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp (right):

https://codereview.chromium.org/2530503002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp:142: DCHECK(false);
NOTREACHED();

https://codereview.chromium.org/2530503002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp:146: };
no ; at the end of namespace. Please also add a comment like // namespace

[bradn, 2016-12-14 12:18:14]

https://codereview.chromium.org/2530503002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp (right):

https://codereview.chromium.org/2530503002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp:142: DCHECK(false);
On 2016/12/14 11:42:43, jochen wrote:
> NOTREACHED();

Done.

https://codereview.chromium.org/2530503002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp:146: };
On 2016/12/14 11:42:43, jochen wrote:
> no ; at the end of namespace. Please also add a comment like // namespace

Done.

[bradn, 2016-12-14 12:18:20]

The CQ bit was checked by bradnelson@google.com

[bradn, 2016-12-14 12:18:21]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/2530503002/#ps140001
(title: "fix")

[commit-bot: I haz the power, 2016-12-14 12:18:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-14 12:20:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-14 12:20:08]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[bradn, 2016-12-14 12:26:23]

The CQ bit was checked by bradnelson@google.com

[bradn, 2016-12-14 12:26:24]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/2530503002/#ps160001
(title: "merge")

[commit-bot: I haz the power, 2016-12-14 12:26:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-14 14:21:33]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1481718383867150,
"parent_rev": "48ae264eaf2f4c1e1a506f1b80e848e62cc21baa", "commit_rev":
"fd462705097457eee26eae732a7cedb96b1e3b6d"}

[commit-bot: I haz the power, 2016-12-14 14:22:00]

Description was changed from

==========
[wasm][asm.js] Routing asm.js warnings to the dev console.

Route asm.js warnings from V8 to the developer console.

Depends on: https://codereview.chromium.org/2526703002/

BUG=v8:4203, chromium:660016
R=dgozman@chromium.org,jochen@chromium.org
CC=danno@chromium.org,bmeurer@chromium.org
==========

to

==========
[wasm][asm.js] Routing asm.js warnings to the dev console.

Route asm.js warnings from V8 to the developer console.

Depends on: https://codereview.chromium.org/2526703002/

BUG=v8:4203, chromium:660016
R=dgozman@chromium.org,jochen@chromium.org
CC=danno@chromium.org,bmeurer@chromium.org

Review-Url: https://codereview.chromium.org/2530503002
==========

[commit-bot: I haz the power, 2016-12-14 14:22:04]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-12-14 14:24:23]

Description was changed from

==========
[wasm][asm.js] Routing asm.js warnings to the dev console.

Route asm.js warnings from V8 to the developer console.

Depends on: https://codereview.chromium.org/2526703002/

BUG=v8:4203, chromium:660016
R=dgozman@chromium.org,jochen@chromium.org
CC=danno@chromium.org,bmeurer@chromium.org

Review-Url: https://codereview.chromium.org/2530503002
==========

to

==========
[wasm][asm.js] Routing asm.js warnings to the dev console.

Route asm.js warnings from V8 to the developer console.

Depends on: https://codereview.chromium.org/2526703002/

BUG=v8:4203, chromium:660016
R=dgozman@chromium.org,jochen@chromium.org
CC=danno@chromium.org,bmeurer@chromium.org

Committed: https://crrev.com/07d5887d7471da0cf7bacb87d9742927f83f0c6d
Cr-Commit-Position: refs/heads/master@{#438511}
==========

[commit-bot: I haz the power, 2016-12-14 14:24:25]

Patchset 9 (id:??) landed as
https://crrev.com/07d5887d7471da0cf7bacb87d9742927f83f0c6d
Cr-Commit-Position: refs/heads/master@{#438511}