Convert RapporRecorder to use mojo.

Convert RapporRecorder to use mojo.

Manually tested by printf'ing the arguments to RapporRecorderImpl
methods and navigating a bunch of tabs. The args looked the same as
for ChromeRenderMessageFilter prior to this CL.

BUG=577685
Committed: https://crrev.com/2f67376dec4b104ccc55b95df8516e4d0640830f
Cr-Commit-Position: refs/heads/master@{#425248}

[nigeltao1, 2016-09-30 05:39:09]

nigeltao@chromium.org changed reviewers:
+ sammc@chromium.org, tibell@chromium.org

[nigeltao1, 2016-09-30 05:39:37]

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

[commit-bot: I haz the power, 2016-09-30 05:39:55]

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-09-30 05:42:04]

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

[commit-bot: I haz the power, 2016-09-30 05:42:05]

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

[nigeltao1, 2016-09-30 05:58:53]

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

[commit-bot: I haz the power, 2016-09-30 05:59:07]

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-09-30 06:01:09]

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

[commit-bot: I haz the power, 2016-09-30 06:01:10]

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

[nigeltao1, 2016-09-30 06:23:54]

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

[commit-bot: I haz the power, 2016-09-30 06:24:15]

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-09-30 07:17:48]

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

[commit-bot: I haz the power, 2016-09-30 07:17:49]

Dry run: This issue passed the CQ dry run.

[Sam McNally, 2016-10-05 09:17:25]

lgtm

https://codereview.chromium.org/2376403003/diff/40001/chrome/browser/rappor_r...
File chrome/browser/rappor_recorder_impl.cc (right):

https://codereview.chromium.org/2376403003/diff/40001/chrome/browser/rappor_r...
chrome/browser/rappor_recorder_impl.cc:26:
rappor::SampleString(g_browser_process->rappor_service(), metric,
I think it's possible g_browser_process could be null.

[nigeltao1, 2016-10-06 04:49:37]

https://codereview.chromium.org/2376403003/diff/40001/chrome/browser/rappor_r...
File chrome/browser/rappor_recorder_impl.cc (right):

https://codereview.chromium.org/2376403003/diff/40001/chrome/browser/rappor_r...
chrome/browser/rappor_recorder_impl.cc:26:
rappor::SampleString(g_browser_process->rappor_service(), metric,
On 2016/10/05 09:17:25, Sam McNally wrote:
> I think it's possible g_browser_process could be null.

Done.

[nigeltao1, 2016-10-06 04:55:16]

Description was changed from

==========
Convert RapporRecorder to use mojo.

Manually tested by printf'ing the arguments to RapporRecorderImpl
methods and navigating a bunch of tabs. The args looked the same as
for ChromeRenderMessageFilter prior to this CL.

BUG=577685
==========

to

==========
Convert RapporRecorder to use mojo.

Manually tested by printf'ing the arguments to RapporRecorderImpl
methods and navigating a bunch of tabs. The args looked the same as
for ChromeRenderMessageFilter prior to this CL.

BUG=577685
==========

[nigeltao1, 2016-10-06 04:55:16]

nigeltao@chromium.org changed reviewers:
+ nasko@chromium.org, sky@chromium.org
- sammc@chromium.org, tibell@chromium.org

[nigeltao1, 2016-10-06 04:56:43]

sky, nasko: this is my first Chromium CL in many years. I'm a noob to the
process, but "git cl upload" listed you as suggested OWNERS. Please redirect if
there is somebody more appropriate.

sammc@chromium has already given LGTM for Mojo expertise.

Re AUTHORS and the CLA, I am also nigeltao@google.com.

[nigeltao1, 2016-10-06 04:57:00]

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

[commit-bot: I haz the power, 2016-10-06 04:57:18]

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-10-06 06:59:05]

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

[commit-bot: I haz the power, 2016-10-06 06:59:06]

Dry run: This issue passed the CQ dry run.

[sky, 2016-10-06 17:28:44]

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/chrome_c...
File chrome/browser/chrome_content_browser_manifest_overlay.json (right):

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/chrome_c...
chrome/browser/chrome_content_browser_manifest_overlay.json:10:
"mojom::RapporRecorder",
I don't think we want a top-level namespace named mojom, use a more specific
qualifier. chrome::mojom::RapporRecorder is fine.

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/rappor_r...
File chrome/browser/rappor_recorder_impl.cc (right):

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/rappor_r...
chrome/browser/rappor_recorder_impl.cc:26: if (g_browser_process != NULL) {
nullptr, or just if (g_browser_process).
That said, can g_browser_process really be null here? I would think this should
be a DCHECK. Same coment below.

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/rappor_r...
File chrome/browser/rappor_recorder_impl.h (right):

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/rappor_r...
chrome/browser/rappor_recorder_impl.h:12: class RapporRecorderImpl : public
mojom::RapporRecorder {
Please add a description. Imagine looking at this a year from now. Will you
remember what it does?

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/rappor_r...
chrome/browser/rappor_recorder_impl.h:12: class RapporRecorderImpl : public
mojom::RapporRecorder {
Is there a reason not to have this code live in components/rappor? It uses
g_browser_process->rappor_service() to get the rappor_service, but couldn't the
constructor take that?

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/rappor_r...
chrome/browser/rappor_recorder_impl.h:20: void RecordRappor(const std::string&
metric,
Generally we prefix overrides with where they come from, e.g.
// chrome::mojom::RapporRecorder:

(see style guide for more details).

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/rappor_r...
chrome/browser/rappor_recorder_impl.h:23: };
DISALLOW...

https://codereview.chromium.org/2376403003/diff/60001/chrome/common/BUILD.gn
File chrome/common/BUILD.gn (right):

https://codereview.chromium.org/2376403003/diff/60001/chrome/common/BUILD.gn#...
chrome/common/BUILD.gn:671: mojom("mojo_bindings_no_new_wrapper_types") {
Why are you separating out the the mojo_bindings target?

https://codereview.chromium.org/2376403003/diff/60001/chrome/renderer/chrome_...
File chrome/renderer/chrome_content_renderer_client.cc (right):

https://codereview.chromium.org/2376403003/diff/60001/chrome/renderer/chrome_...
chrome/renderer/chrome_content_renderer_client.cc:1332: if (!rappor_recorder_) {
not {} here and 1340.

[sky, 2016-10-06 17:31:02]

sky@chromium.org changed reviewers:
+ tsepez@chromium.org

[sky, 2016-10-06 17:32:02]

Also, you'll need a security review for new mojom
+tsepez

And I see Sam made you add the null checks for g_browser_process. Sam, can you
outline when g_browser_process would be null?

[Tom Sepez, 2016-10-06 18:29:57]

Mojom itself LGTM but do follow up on the issue sky raised.

[Sam McNally, 2016-10-06 23:07:03]

On 2016/10/06 17:32:02, sky wrote:
> Also, you'll need a security review for new mojom
> +tsepez
> 
> And I see Sam made you add the null checks for g_browser_process. Sam, can you
> outline when g_browser_process would be null?

Taking a closer look at shutdown, g_browser_process should be safe when handling
IPCs. It's only during destruction that it could be null.

[nigeltao1, 2016-10-07 03:26:24]

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

[commit-bot: I haz the power, 2016-10-07 03:26:35]

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

[nigeltao1, 2016-10-07 03:30:52]

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/chrome_c...
File chrome/browser/chrome_content_browser_manifest_overlay.json (right):

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/chrome_c...
chrome/browser/chrome_content_browser_manifest_overlay.json:10:
"mojom::RapporRecorder",
On 2016/10/06 17:28:44, sky wrote:
> I don't think we want a top-level namespace named mojom, use a more specific
> qualifier. chrome::mojom::RapporRecorder is fine.

I was cargo-culting the other .mojom files under chrome/common, which are all
"mojom" and not "chrome::mojom". In contrast, .mojom files under content/common
are all "content::mojom". So says grep:

$ grep --no-filename ^module chrome/common/*.mojom
module mojom;
module mojom;
module mojom;
module mojom;
module mojom;
$ grep --no-filename ^module content/common/*.mojom
module content.mojom;
module content.mojom;
module content.mojom;
module content.mojom;
module content.mojom;
module content.mojom;
module content.mojom;
module content.mojom;
module content.mojom;
module content.mojom;
module content.mojom;
module content.mojom;
module content.mojom;

But in any case, I'll move it into a component, so that it's "rappor::mojom",
similar to the existing "safe_json::mojom" defined by components/safe_json.

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/rappor_r...
File chrome/browser/rappor_recorder_impl.cc (right):

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/rappor_r...
chrome/browser/rappor_recorder_impl.cc:26: if (g_browser_process != NULL) {
On 2016/10/06 17:28:44, sky wrote:
> nullptr, or just if (g_browser_process).
> That said, can g_browser_process really be null here? I would think this
should
> be a DCHECK. Same coment below.

As discussed, I've dropped this check.

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/rappor_r...
File chrome/browser/rappor_recorder_impl.h (right):

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/rappor_r...
chrome/browser/rappor_recorder_impl.h:12: class RapporRecorderImpl : public
mojom::RapporRecorder {
On 2016/10/06 17:28:44, sky wrote:
> Please add a description. Imagine looking at this a year from now. Will you
> remember what it does?

Done.

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/rappor_r...
chrome/browser/rappor_recorder_impl.h:12: class RapporRecorderImpl : public
mojom::RapporRecorder {
On 2016/10/06 17:28:44, sky wrote:
> Is there a reason not to have this code live in components/rappor? It uses
> g_browser_process->rappor_service() to get the rappor_service, but couldn't
the
> constructor take that?

I moved it to component/rappor, and pass the RapporService* through the
constructor.

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/rappor_r...
chrome/browser/rappor_recorder_impl.h:20: void RecordRappor(const std::string&
metric,
On 2016/10/06 17:28:44, sky wrote:
> Generally we prefix overrides with where they come from, e.g.
> // chrome::mojom::RapporRecorder:
> 
> (see style guide for more details).

Done.

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/rappor_r...
chrome/browser/rappor_recorder_impl.h:23: };
On 2016/10/06 17:28:44, sky wrote:
> DISALLOW...

Done.

https://codereview.chromium.org/2376403003/diff/60001/chrome/common/BUILD.gn
File chrome/common/BUILD.gn (right):

https://codereview.chromium.org/2376403003/diff/60001/chrome/common/BUILD.gn#...
chrome/common/BUILD.gn:671: mojom("mojo_bindings_no_new_wrapper_types") {
On 2016/10/06 17:28:44, sky wrote:
> Why are you separating out the the mojo_bindings target?

Because updating the existing interfaces for use_new_wrapper_types = the default
value instead of false is out of scope of this CL.

But in any case, this is all moot, as I moved the BUILD target from
chrome/common to components/rappor.

https://codereview.chromium.org/2376403003/diff/60001/chrome/renderer/chrome_...
File chrome/renderer/chrome_content_renderer_client.cc (right):

https://codereview.chromium.org/2376403003/diff/60001/chrome/renderer/chrome_...
chrome/renderer/chrome_content_renderer_client.cc:1332: if (!rappor_recorder_) {
On 2016/10/06 17:28:44, sky wrote:
> not {} here and 1340.

Done.

I'm a recovering Go programmer. :-)

[nigeltao1, 2016-10-07 04:21:19]

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

[commit-bot: I haz the power, 2016-10-07 04:21:25]

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-10-07 05:58:16]

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

[commit-bot: I haz the power, 2016-10-07 05:58:17]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[sky, 2016-10-07 16:17:37]

sky@chromium.org changed reviewers:
+ asvitkine@chromium.org

[sky, 2016-10-07 16:17:39]

LGTM - but see comments above separate build target. I'm randomly adding
asvitkine as he is an owner of components/rappor.

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/chrome_c...
File chrome/browser/chrome_content_browser_manifest_overlay.json (right):

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/chrome_c...
chrome/browser/chrome_content_browser_manifest_overlay.json:10:
"mojom::RapporRecorder",
On 2016/10/07 03:30:51, nigeltao1 wrote:
> On 2016/10/06 17:28:44, sky wrote:
> > I don't think we want a top-level namespace named mojom, use a more specific
> > qualifier. chrome::mojom::RapporRecorder is fine.
> 
> I was cargo-culting the other .mojom files under chrome/common, which are all
> "mojom" and not "chrome::mojom". In contrast, .mojom files under
content/common
> are all "content::mojom". So says grep:
> 
> $ grep --no-filename ^module chrome/common/*.mojom
> module mojom;
> module mojom;
> module mojom;
> module mojom;
> module mojom;

I discussed the namespace issue with Ken the other day before sending this out
and he agreed a namespace should be used. So, I would assume these will change
with time too.

> $ grep --no-filename ^module content/common/*.mojom
> module content.mojom;
> module content.mojom;
> module content.mojom;
> module content.mojom;
> module content.mojom;
> module content.mojom;
> module content.mojom;
> module content.mojom;
> module content.mojom;
> module content.mojom;
> module content.mojom;
> module content.mojom;
> module content.mojom;
> 
> But in any case, I'll move it into a component, so that it's "rappor::mojom",
> similar to the existing "safe_json::mojom" defined by components/safe_json.
>

https://codereview.chromium.org/2376403003/diff/100001/components/rappor/reco...
File components/rappor/recorder/BUILD.gn (right):

https://codereview.chromium.org/2376403003/diff/100001/components/rappor/reco...
components/rappor/recorder/BUILD.gn:5: static_library("recorder") {
IMO I would move this code into components/rappor and put these files there.
That said, I'm not an owner of this directory. So, you'll need to work with
local owners.

[Alexei Svitkine (slow), 2016-10-07 16:31:59]

https://codereview.chromium.org/2376403003/diff/100001/chrome/renderer/chrome...
File chrome/renderer/chrome_content_renderer_client.h (right):

https://codereview.chromium.org/2376403003/diff/100001/chrome/renderer/chrome...
chrome/renderer/chrome_content_renderer_client.h:260: };
Nit: While you're here, add DISALLOW_COPY_AND_ASSIGN()?

https://codereview.chromium.org/2376403003/diff/100001/components/rappor/reco...
File components/rappor/recorder/BUILD.gn (right):

https://codereview.chromium.org/2376403003/diff/100001/components/rappor/reco...
components/rappor/recorder/BUILD.gn:5: static_library("recorder") {
On 2016/10/07 16:17:38, sky wrote:
> IMO I would move this code into components/rappor and put these files there.
> That said, I'm not an owner of this directory. So, you'll need to work with
> local owners.

Agree, I don't think it's worth having a separate sub-directory for this.

https://codereview.chromium.org/2376403003/diff/100001/components/rappor/reco...
File components/rappor/recorder/rappor_recorder_impl.cc (right):

https://codereview.chromium.org/2376403003/diff/100001/components/rappor/reco...
components/rappor/recorder/rappor_recorder_impl.cc:28:
rappor::SampleString(rappor_service_, metric,
Nit: Remove rappor:: prefix here and on line 35.

[nigeltao1, 2016-10-11 23:44:55]

https://codereview.chromium.org/2376403003/diff/100001/chrome/renderer/chrome...
File chrome/renderer/chrome_content_renderer_client.h (right):

https://codereview.chromium.org/2376403003/diff/100001/chrome/renderer/chrome...
chrome/renderer/chrome_content_renderer_client.h:260: };
On 2016/10/07 16:31:58, Alexei Svitkine (slow) wrote:
> Nit: While you're here, add DISALLOW_COPY_AND_ASSIGN()?

Done.

https://codereview.chromium.org/2376403003/diff/100001/components/rappor/reco...
File components/rappor/recorder/BUILD.gn (right):

https://codereview.chromium.org/2376403003/diff/100001/components/rappor/reco...
components/rappor/recorder/BUILD.gn:5: static_library("recorder") {
On 2016/10/07 16:31:58, Alexei Svitkine (slow) wrote:
> On 2016/10/07 16:17:38, sky wrote:
> > IMO I would move this code into components/rappor and put these files there.
> > That said, I'm not an owner of this directory. So, you'll need to work with
> > local owners.
> 
> Agree, I don't think it's worth having a separate sub-directory for this.

Done.

https://codereview.chromium.org/2376403003/diff/100001/components/rappor/reco...
File components/rappor/recorder/rappor_recorder_impl.cc (right):

https://codereview.chromium.org/2376403003/diff/100001/components/rappor/reco...
components/rappor/recorder/rappor_recorder_impl.cc:28:
rappor::SampleString(rappor_service_, metric,
On 2016/10/07 16:31:58, Alexei Svitkine (slow) wrote:
> Nit: Remove rappor:: prefix here and on line 35.

Done, and also on line 29.

[nigeltao1, 2016-10-11 23:45:45]

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

[commit-bot: I haz the power, 2016-10-11 23: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-10-12 01:14:10]

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

[commit-bot: I haz the power, 2016-10-12 01:14:10]

Dry run: This issue passed the CQ dry run.

[Alexei Svitkine (slow), 2016-10-13 19:25:25]

lgtm

https://codereview.chromium.org/2376403003/diff/160001/components/rappor/rapp...
File components/rappor/rappor_recorder_impl.cc (right):

https://codereview.chromium.org/2376403003/diff/160001/components/rappor/rapp...
components/rappor/rappor_recorder_impl.cc:20:
rappor::mojom::RapporRecorderRequest request) {
Nit: Remove rappor:: prefix

https://codereview.chromium.org/2376403003/diff/160001/components/rappor/rapp...
File components/rappor/rappor_recorder_impl.h (right):

https://codereview.chromium.org/2376403003/diff/160001/components/rappor/rapp...
components/rappor/rappor_recorder_impl.h:19: class RapporRecorderImpl : public
rappor::mojom::RapporRecorder {
Nit: Remove rappor:: prefix

https://codereview.chromium.org/2376403003/diff/160001/components/rappor/rapp...
components/rappor/rappor_recorder_impl.h:25:
rappor::mojom::RapporRecorderRequest request);
Nit: Remove rappor:: prefix

[nigeltao1, 2016-10-14 02:09:55]

The CQ bit was checked by nigeltao@chromium.org

[nigeltao1, 2016-10-14 02:09:56]

The patchset sent to the CQ was uploaded after l-g-t-m from sammc@chromium.org,
tsepez@chromium.org, sky@chromium.org, asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2376403003/#ps180001
(title: "Convert RapporRecorder to use mojo.")

[commit-bot: I haz the power, 2016-10-14 02:10:07]

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-10-14 03:28:10]

Description was changed from

==========
Convert RapporRecorder to use mojo.

Manually tested by printf'ing the arguments to RapporRecorderImpl
methods and navigating a bunch of tabs. The args looked the same as
for ChromeRenderMessageFilter prior to this CL.

BUG=577685
==========

to

==========
Convert RapporRecorder to use mojo.

Manually tested by printf'ing the arguments to RapporRecorderImpl
methods and navigating a bunch of tabs. The args looked the same as
for ChromeRenderMessageFilter prior to this CL.

BUG=577685
==========

[commit-bot: I haz the power, 2016-10-14 03:28:12]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-10-14 03:31:45]

Description was changed from

==========
Convert RapporRecorder to use mojo.

Manually tested by printf'ing the arguments to RapporRecorderImpl
methods and navigating a bunch of tabs. The args looked the same as
for ChromeRenderMessageFilter prior to this CL.

BUG=577685
==========

to

==========
Convert RapporRecorder to use mojo.

Manually tested by printf'ing the arguments to RapporRecorderImpl
methods and navigating a bunch of tabs. The args looked the same as
for ChromeRenderMessageFilter prior to this CL.

BUG=577685

Committed: https://crrev.com/2f67376dec4b104ccc55b95df8516e4d0640830f
Cr-Commit-Position: refs/heads/master@{#425248}
==========

[commit-bot: I haz the power, 2016-10-14 03:31:46]

Patchset 10 (id:??) landed as
https://crrev.com/2f67376dec4b104ccc55b95df8516e4d0640830f
Cr-Commit-Position: refs/heads/master@{#425248}

[nigeltao1, 2016-10-29 04:31:09]

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/chrome_c...
File chrome/browser/chrome_content_browser_manifest_overlay.json (right):

https://codereview.chromium.org/2376403003/diff/60001/chrome/browser/chrome_c...
chrome/browser/chrome_content_browser_manifest_overlay.json:10:
"mojom::RapporRecorder",
On 2016/10/07 16:17:38, sky wrote:
> On 2016/10/07 03:30:51, nigeltao1 wrote:
> > $ grep --no-filename ^module chrome/common/*.mojom
> > module mojom;
> > module mojom;
> > module mojom;
> > module mojom;
> > module mojom;
> 
> I discussed the namespace issue with Ken the other day before sending this out
> and he agreed a namespace should be used. So, I would assume these will change
> with time too.

OK, I sent out https://codereview.chromium.org/2465623002# to change one of
these from "module mojom" to "module chrome.mojom". If that goes well, I'll fix
the others as well.