[Page Load Metrics] PageLoadMetrics Mojofication.

chrome/common/page_load_metrics/page_load_metrics.mojom

Index: chrome/common/page_load_metrics/page_load_metrics.mojom
diff --git a/chrome/common/page_load_metrics/page_load_metrics.mojom b/chrome/common/page_load_metrics/page_load_metrics.mojom
new file mode 100644
index 0000000000000000000000000000000000000000..13b8d18db582c5b6e6d474702310ffb4c8b4b8af
--- /dev/null
+++ b/chrome/common/page_load_metrics/page_load_metrics.mojom
@@ -0,0 +1,56 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+module page_load_metrics.mojom;
+
+import "mojo/common/time.mojom";
+
+// See comments in page_load_timing.h for details on each field.

[Bryan McQuade, 2017/04/24 21:50:55]

interesting, do all types sent over mojo have to be declared in a mojom file? we
can't for instance include page_load_timing.h? or convert page_load_timing.h to
a mojom file which just declares structs, and then include it here, to avoid the
duplication?

[lpy, 2017/04/25 09:03:37]

I think that's how mojo works since I am following mojo wiki, that's also what
legacy IPC used to require. 

+rockot@ for further answer.

[Ken Rockot(use gerrit already), 2017/04/25 19:31:46]

Absolutely yes, everything sent over Mojo has to be defined in a mojom file.
Otherwise bindings would be limited to C++.

We do support C++ typemapping[1] which allows you to teach the bindings
generator how to serialize some arbitrary C++ type as a mojom struct and then
generate bindings which use that type in any relevant generated C++ interfaces.

We also support some trickery[2] for exploiting existing legacy IPC traits
definitions to avoid duplicate mojom definitions for existing IPC structs. Use
of this feature should generally be discouraged though and will not be supported
indefinitely.

[1]
https://chromium.googlesource.com/chromium/src/+/master/mojo/public/cpp/bindi...
[2]
https://chromium.googlesource.com/chromium/src/+/master/ipc/README.md#Using-L...

[Bryan McQuade, 2017/04/26 01:55:42]

Ah, ok, thanks! Is it possible to declare the structs in a mojom file and then
use them for the IPC code flow as well? That way we have only one canonical
place that we define the structs (mojom file) rather than having a mojom file
and a separate header for the IPC variant.

[Ken Rockot(use gerrit already), 2017/04/26 03:08:01]

No, and we won't be supporting that. I mean, you could always call
mojom::Foo::Serialize(foo) to get a std::vector<uint8_t> of serialized bytes and
send that over legacy IPC, but that's pretty awful.

If you have a structure which is already used by legacy IPC code though, you can
define a mojom alias (no duplicate definition in Mojom beyond a simple name
declaration) and the bindings will repurpose the existing serialization format.
Of course this limits the type to use with C++ bindings, but that kind of goes
without saying if you plan to use legacy IPC serialization. This is the feature
outlined by [2] in my previous reply.

Of course if we're talking about a data structure which is not currently used in
any legacy IPC messages, the point is moot because we should not be introducing
any new legacy IPC code without very compelling reasons.

+
+struct DocumentTiming {
+  mojo.common.mojom.TimeDelta? dom_content_loaded_event_start;
+  mojo.common.mojom.TimeDelta? load_event_start;
+  mojo.common.mojom.TimeDelta? first_layout;
+};
+
+struct PaintTiming {
+  mojo.common.mojom.TimeDelta? first_paint;
+  mojo.common.mojom.TimeDelta? first_text_paint;
+  mojo.common.mojom.TimeDelta? first_image_paint;
+  mojo.common.mojom.TimeDelta? first_contentful_paint;
+  mojo.common.mojom.TimeDelta? first_meaningful_paint;
+};
+
+struct ParseTiming {
+  mojo.common.mojom.TimeDelta? parse_start;
+  mojo.common.mojom.TimeDelta? parse_stop;
+  mojo.common.mojom.TimeDelta? parse_blocked_on_script_load_duration;
+  mojo.common.mojom.TimeDelta? parse_blocked_on_script_load_from_document_write_duration;
+  mojo.common.mojom.TimeDelta? parse_blocked_on_script_execution_duration;
+  mojo.common.mojom.TimeDelta? parse_blocked_on_script_execution_from_document_write_duration;
+};
+
+struct StyleSheetTiming {
+  mojo.common.mojom.TimeDelta? author_style_sheet_parse_duration_before_fcp;
+  mojo.common.mojom.TimeDelta? update_style_duration_before_fcp;
+};
+
+struct PageLoadTiming {
+  mojo.common.mojom.Time navigation_start;
+  mojo.common.mojom.TimeDelta? response_start;
+  DocumentTiming document_timing;
+  PaintTiming paint_timing;
+  ParseTiming parse_timing;
+  StyleSheetTiming style_sheet_timing;
+};
+
+struct PageLoadMetadata {
+  int32 behavior_flags;
+};
+
+// Sent from renderer to browser process when the PageLoadTiming for the
+// associated frame changed.
+interface PageLoadMetrics {

[Bryan McQuade, 2017/04/24 21:50:55]

though I don't have a better name for this service, I find the name
PageLoadMetrics a little confusing. That name doesn't sound like a service name
that you execute actions on but rather a passive data structure that holds
metrics data. I'm slightly inclined to call this something like
PageLoadMetricsChannel to better describe that it's a communication channel that
the PageLoadMetrics are sent over, but I'm not sure if that's the best name or
not. Can you think of a name to better describe that this is a service interface
that page load metrics data is sent over?

[Ken Rockot(use gerrit already), 2017/04/25 19:31:46]

terminology nit: it's not a service.

All mojo interfaces are by convention in a mojom subnamespace, so this will
always be read as mojom::PageLoadMetrics in C++ code. That should be sufficient
to convey the detail that it's an IPC interface. I do not think it is correct
for people to start adding nouns like "Channel" or "Pipe" or whatever to
interface names.

[Bryan McQuade, 2017/04/26 01:55:42]

the mojom:: namespace does help to clarify the class's purpose here - sounds
fine to me thanks.

+  UpdateTiming(PageLoadTiming page_load_timing, PageLoadMetadata page_load_metadata);
+};