Mojo JS bindings: make sure that console.log() could be used in the bindings.

Mojo JS bindings: make sure that console.log() could be used in the bindings.

It is convenient for debugging.

Although console.log() is supported by chrome directly, mojo bindings currently are loaded using gin. Therefore, console.log() is not always available and needs to be defined as a module. After we move away from gin/AMD, we should be able to get rid of this. 


BUG=579646
TBR=jochen@chromium.org
(deferring to eugenebut)

Review-Url: https://codereview.chromium.org/2618693004
Cr-Commit-Position: refs/heads/master@{#442972}
Committed: https://chromium.googlesource.com/chromium/src/+/1ff21f95261daa80eeff96b7df798da9b85b5eee

[yzshen1, 2017-01-05 19:51:53]

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

[commit-bot: I haz the power, 2017-01-05 19:52:17]

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, 2017-01-05 21:00:55]

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

[commit-bot: I haz the power, 2017-01-05 21:00:56]

Dry run: This issue passed the CQ dry run.

[yzshen1, 2017-01-09 22:24:56]

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

[commit-bot: I haz the power, 2017-01-09 22:25:50]

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

[yzshen1, 2017-01-09 23:03:12]

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

[commit-bot: I haz the power, 2017-01-09 23:04:00]

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

[yzshen1, 2017-01-09 23:09:41]

Description was changed from

==========
Mojo JS bindings: make sure that console.log() could be used in the bindings.

BUG=579646
==========

to

==========
Mojo JS bindings: make sure that console.log() could be used in the bindings.

Although console.log() is supported by chrome directly, mojo bindings currently
are loaded using gin. Therefore, console.log() is not always available and needs
to be defined as a module. After we move away from gin/AMD, we should be able to
get rid of this. 


BUG=579646
==========

[yzshen1, 2017-01-09 23:10:25]

Description was changed from

==========
Mojo JS bindings: make sure that console.log() could be used in the bindings.

Although console.log() is supported by chrome directly, mojo bindings currently
are loaded using gin. Therefore, console.log() is not always available and needs
to be defined as a module. After we move away from gin/AMD, we should be able to
get rid of this. 


BUG=579646
==========

to

==========
Mojo JS bindings: make sure that console.log() could be used in the bindings.

It is convenient for debugging.

Although console.log() is supported by chrome directly, mojo bindings currently
are loaded using gin. Therefore, console.log() is not always available and needs
to be defined as a module. After we move away from gin/AMD, we should be able to
get rid of this. 


BUG=579646
==========

[yzshen1, 2017-01-09 23:13:07]

yzshen@chromium.org changed reviewers:
+ eugenebut@chromium.org, jochen@chromium.org

[yzshen1, 2017-01-09 23:13:08]

Hi, Jochen and Eugene.

Would you please take a look? Thanks!

Jochen:
content/renderer/render_frame_impl.cc
extensions/renderer/api_test_base.cc

Eugene:
ios/web/*

[Eugene But (OOO till 7-30), 2017-01-09 23:28:35]

Isn't console.log already supported on iOS?:
https://cs.chromium.org/chromium/src/ios/web/web_state/ui/crw_web_controller....

[yzshen1, 2017-01-10 00:11:23]

On 2017/01/09 23:28:35, Eugene But wrote:
> Isn't console.log already supported on iOS?:
>
https://cs.chromium.org/chromium/src/ios/web/web_state/ui/crw_web_controller....

Thanks for the information!

One inconvenience with the existing console.log() is that it logs with DVLOG()
and the tests are run on the bots without showing them.
And trybots is my primary/only way of debugging mojo JS bindings on IOS. (I
guess that is also true for other mojo people.)
If we decide to use the existing console.log(), maybe we should change it to
DLOG or change testing config on the bots with necessary command line params?

(Btw, in either way, I will still have to keep the console.js module. The
difference is whether to let console.js call console.log() or uses
syncMessageChannel to call its own implementation.)

[yzshen1, 2017-01-10 06:58:20]

Patchset #10 (id:180001) has been deleted

[jochen (gone - plz use gerrit), 2017-01-10 09:50:38]

(deferring to eugenebut)

[Eugene But (OOO till 7-30), 2017-01-10 18:39:11]

On 2017/01/10 09:50:38, jochen wrote:
> (deferring to eugenebut)
Per offline discussion, let's keep using existing console.log, which you have in
patch #11:
https://codereview.chromium.org/2618693004/patch/220001/230011

and unwind changes in MojoFacade.

Thanks!

[yzshen1, 2017-01-10 18:52:13]

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

[commit-bot: I haz the power, 2017-01-10 18:52:53]

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

[yzshen1, 2017-01-10 18:53:16]

On 2017/01/10 18:39:11, Eugene But wrote:
> On 2017/01/10 09:50:38, jochen wrote:
> > (deferring to eugenebut)
> Per offline discussion, let's keep using existing console.log, which you have
in
> patch #11:
> https://codereview.chromium.org/2618693004/patch/220001/230011
> 
> and unwind changes in MojoFacade.
> 
> Thanks!

Thanks Eugene! PTAL

[Eugene But (OOO till 7-30), 2017-01-10 19:32:52]

lgtm

https://codereview.chromium.org/2618693004/diff/240001/ios/web/webui/resource...
File ios/web/webui/resources/console.js (right):

https://codereview.chromium.org/2618693004/diff/240001/ios/web/webui/resource...
ios/web/webui/resources/console.js:10: // and the |console| object is not always
available. When we move away from
nit: go/avoidwe :)

[yzshen1, 2017-01-10 20:12:19]

Description was changed from

==========
Mojo JS bindings: make sure that console.log() could be used in the bindings.

It is convenient for debugging.

Although console.log() is supported by chrome directly, mojo bindings currently
are loaded using gin. Therefore, console.log() is not always available and needs
to be defined as a module. After we move away from gin/AMD, we should be able to
get rid of this. 


BUG=579646
==========

to

==========
Mojo JS bindings: make sure that console.log() could be used in the bindings.

It is convenient for debugging.

Although console.log() is supported by chrome directly, mojo bindings currently
are loaded using gin. Therefore, console.log() is not always available and needs
to be defined as a module. After we move away from gin/AMD, we should be able to
get rid of this. 


BUG=579646
TBR=jochen@chromium.org
(deferring to eugenebut)
==========

[yzshen1, 2017-01-10 20:13:08]

https://codereview.chromium.org/2618693004/diff/240001/ios/web/webui/resource...
File ios/web/webui/resources/console.js (right):

https://codereview.chromium.org/2618693004/diff/240001/ios/web/webui/resource...
ios/web/webui/resources/console.js:10: // and the |console| object is not always
available. When we move away from
On 2017/01/10 19:32:52, Eugene But wrote:
> nit: go/avoidwe :)

Done.

[yzshen1, 2017-01-10 20:13:17]

The CQ bit was checked by yzshen@chromium.org

[yzshen1, 2017-01-10 20:13:17]

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

[commit-bot: I haz the power, 2017-01-10 20:13:48]

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, 2017-01-10 21:42:30]

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

[commit-bot: I haz the power, 2017-01-10 21:42:32]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[yzshen1, 2017-01-10 21:46:26]

The CQ bit was checked by yzshen@chromium.org

[commit-bot: I haz the power, 2017-01-10 21:47:09]

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, 2017-01-10 23:14:02]

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

[commit-bot: I haz the power, 2017-01-10 23:14:03]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[yzshen1, 2017-01-11 07:38:47]

The CQ bit was checked by yzshen@chromium.org

[commit-bot: I haz the power, 2017-01-11 07:39:10]

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

[jochen (gone - plz use gerrit), 2017-01-11 09:55:59]

lgtm

[commit-bot: I haz the power, 2017-01-11 10:30:41]

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

[commit-bot: I haz the power, 2017-01-11 10:30:43]

Failed to apply patch for content/renderer/render_frame_impl.cc:
While running git apply --index -p1;
  error: patch failed: content/renderer/render_frame_impl.cc:141
  error: content/renderer/render_frame_impl.cc: patch does not apply

Patch:       content/renderer/render_frame_impl.cc
Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc
b/content/renderer/render_frame_impl.cc
index
f315311a35500819d4a2ecaf3db6f98d3c0393cd..11614a4a69334254c87d510747484c7944214a73
100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -141,6 +141,7 @@
 #include "content/renderer/web_ui_extension.h"
 #include "content/renderer/websharedworker_proxy.h"
 #include "crypto/sha2.h"
+#include "gin/modules/console.h"
 #include "gin/modules/module_registry.h"
 #include "media/audio/audio_output_device.h"
 #include "media/base/audio_renderer_mixer_input.h"
@@ -2616,6 +2617,9 @@ void RenderFrameImpl::EnsureMojoBuiltinsAreAvailable(
     return;
 
   v8::HandleScope handle_scope(isolate);
+
+  registry->AddBuiltinModule(isolate, gin::Console::kModuleName,
+                             gin::Console::GetModule(isolate));
   registry->AddBuiltinModule(isolate, mojo::edk::js::Core::kModuleName,
                              mojo::edk::js::Core::GetModule(isolate));
   registry->AddBuiltinModule(isolate, mojo::edk::js::Support::kModuleName,

[yzshen1, 2017-01-11 17:27:25]

The CQ bit was checked by yzshen@chromium.org

[yzshen1, 2017-01-11 17:27:26]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
eugenebut@chromium.org
Link to the patchset: https://codereview.chromium.org/2618693004/#ps280001
(title: "merge & resolve")

[commit-bot: I haz the power, 2017-01-11 17:27:53]

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, 2017-01-11 18:31:49]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1484155645757220,
"parent_rev": "8cfdf8fa9481346a3324901efcd93e281e2e757a", "commit_rev":
"a1782ed87a42a09a5ee3730ee8c0564cdb92466e"}

[commit-bot: I haz the power, 2017-01-11 18:38:50]

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

[commit-bot: I haz the power, 2017-01-11 18:38:52]

Prior attempt to commit was detected, but we were not able to check whether the
issue was successfully committed. Please check Git history manually and re-check
CQ or close this issue as needed.

[yzshen1, 2017-01-11 18:58:31]

The CQ bit was checked by yzshen@chromium.org

[commit-bot: I haz the power, 2017-01-11 18:59:04]

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, 2017-01-11 19:05:27]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1484161111464320,
"parent_rev": "34db9f35de6a4531e82d3bf068f0529ca94b348b", "commit_rev":
"1ff21f95261daa80eeff96b7df798da9b85b5eee"}

[commit-bot: I haz the power, 2017-01-11 19:05:58]

Description was changed from

==========
Mojo JS bindings: make sure that console.log() could be used in the bindings.

It is convenient for debugging.

Although console.log() is supported by chrome directly, mojo bindings currently
are loaded using gin. Therefore, console.log() is not always available and needs
to be defined as a module. After we move away from gin/AMD, we should be able to
get rid of this. 


BUG=579646
TBR=jochen@chromium.org
(deferring to eugenebut)
==========

to

==========
Mojo JS bindings: make sure that console.log() could be used in the bindings.

It is convenient for debugging.

Although console.log() is supported by chrome directly, mojo bindings currently
are loaded using gin. Therefore, console.log() is not always available and needs
to be defined as a module. After we move away from gin/AMD, we should be able to
get rid of this. 


BUG=579646
TBR=jochen@chromium.org
(deferring to eugenebut)

Review-Url: https://codereview.chromium.org/2618693004
Cr-Commit-Position: refs/heads/master@{#442972}
Committed:
https://chromium.googlesource.com/chromium/src/+/1ff21f95261daa80eeff96b7df79...
==========

[commit-bot: I haz the power, 2017-01-11 19:06:09]

Committed patchset #14 (id:280001) as
https://chromium.googlesource.com/chromium/src/+/1ff21f95261daa80eeff96b7df79...