Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(312)

Issue 2310583002: Convert some NetError[Tab]Helper messages to mojom (Closed)

Created:
4 years, 3 months ago by Ken Rockot(use gerrit already)
Modified:
4 years, 3 months ago
CC:
Aaron Boodman, abarth-chromium, cbentzel+watch_chromium.org, chromium-reviews, darin (slow to review), qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Convert some NetError[Tab]Helper messages to mojom Converts ChromeViewHostMsg_RunNetworkDiagnostics and ChromeViewMsg_SetCanShowNetworkDiagnosticsDialg to mojom without impacting message ordering. The browser-side interface is exposed via a WebContentsFrameBindingSet<T>. The renderer-side (client) interface is exposed via RenderFrameImpl's AssociatedInterfaceRegistry. This CL serves as a simple example of converting routed IPC messages which are handled by WebContentsObservers or other WebContents-bound things. BUG=612500 Committed: https://crrev.com/e1807f0bc6e60da26c609da2e87aaaae4b03c7b2 Cr-Commit-Position: refs/heads/master@{#419259}

Patch Set 1 #

Total comments: 4

Patch Set 2 : . #

Patch Set 3 : . #

Total comments: 8

Patch Set 4 : rebase #

Patch Set 5 : . #

Patch Set 6 : rebase #

Total comments: 4

Patch Set 7 : rebase #

Patch Set 8 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+104 lines, -40 lines) Patch
M chrome/browser/net/net_error_tab_helper.h View 1 2 3 4 5 6 7 6 chunks +14 lines, -4 lines 0 comments Download
M chrome/browser/net/net_error_tab_helper.cc View 1 2 3 4 5 6 7 4 chunks +16 lines, -8 lines 0 comments Download
M chrome/browser/net/net_error_tab_helper_unittest.cc View 1 2 3 4 5 6 7 3 chunks +13 lines, -7 lines 0 comments Download
M chrome/common/BUILD.gn View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
A chrome/common/network_diagnostics.mojom View 1 chunk +17 lines, -0 lines 0 comments Download
M chrome/common/render_messages.h View 2 chunks +0 lines, -8 lines 0 comments Download
M chrome/renderer/net/net_error_helper.h View 6 chunks +16 lines, -3 lines 0 comments Download
M chrome/renderer/net/net_error_helper.cc View 1 2 3 4 5 8 chunks +26 lines, -10 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 70 (54 generated)
Ken Rockot(use gerrit already)
Here's what I came up with for helping WebContentsObservers do stuff. I started out by ...
4 years, 3 months ago (2016-09-02 20:16:41 UTC) #5
tibell
https://codereview.chromium.org/2310583002/diff/1/chrome/browser/net/net_error_tab_helper.cc File chrome/browser/net/net_error_tab_helper.cc (right): https://codereview.chromium.org/2310583002/diff/1/chrome/browser/net/net_error_tab_helper.cc#newcode189 chrome/browser/net/net_error_tab_helper.cc:189: case 0: // MSVC complains if we have a ...
4 years, 3 months ago (2016-09-05 00:47:33 UTC) #8
Ken Rockot(use gerrit already)
Updated CL #3 with some changes and rebased here https://codereview.chromium.org/2310583002/diff/1/chrome/browser/net/net_error_tab_helper.cc File chrome/browser/net/net_error_tab_helper.cc (right): https://codereview.chromium.org/2310583002/diff/1/chrome/browser/net/net_error_tab_helper.cc#newcode189 chrome/browser/net/net_error_tab_helper.cc:189: ...
4 years, 3 months ago (2016-09-07 00:47:12 UTC) #13
Sam McNally
https://codereview.chromium.org/2310583002/diff/40001/chrome/browser/net/net_error_tab_helper.cc File chrome/browser/net/net_error_tab_helper.cc (right): https://codereview.chromium.org/2310583002/diff/40001/chrome/browser/net/net_error_tab_helper.cc#newcode103 chrome/browser/net/net_error_tab_helper.cc:103: remote_network_diagnostics_client_.ForFrame(render_frame_host) Do we need to keep connections alive beyond ...
4 years, 3 months ago (2016-09-07 01:48:47 UTC) #18
Ken Rockot(use gerrit already)
https://codereview.chromium.org/2310583002/diff/40001/chrome/browser/net/net_error_tab_helper.cc File chrome/browser/net/net_error_tab_helper.cc (right): https://codereview.chromium.org/2310583002/diff/40001/chrome/browser/net/net_error_tab_helper.cc#newcode103 chrome/browser/net/net_error_tab_helper.cc:103: remote_network_diagnostics_client_.ForFrame(render_frame_host) On 2016/09/07 at 01:48:47, Sam McNally wrote: > ...
4 years, 3 months ago (2016-09-07 22:48:18 UTC) #22
tibell
lgtm
4 years, 3 months ago (2016-09-09 04:06:43 UTC) #25
Sam McNally
lgtm https://codereview.chromium.org/2310583002/diff/100001/chrome/browser/net/net_error_tab_helper.h File chrome/browser/net/net_error_tab_helper.h (right): https://codereview.chromium.org/2310583002/diff/100001/chrome/browser/net/net_error_tab_helper.h#newcode24 chrome/browser/net/net_error_tab_helper.h:24: class WebContentsInterfaceRegistry; Remove. https://codereview.chromium.org/2310583002/diff/100001/chrome/browser/net/net_error_tab_helper_unittest.cc File chrome/browser/net/net_error_tab_helper_unittest.cc (right): https://codereview.chromium.org/2310583002/diff/100001/chrome/browser/net/net_error_tab_helper_unittest.cc#newcode48 ...
4 years, 3 months ago (2016-09-09 06:08:12 UTC) #26
Ken Rockot(use gerrit already)
https://codereview.chromium.org/2310583002/diff/100001/chrome/browser/net/net_error_tab_helper.h File chrome/browser/net/net_error_tab_helper.h (right): https://codereview.chromium.org/2310583002/diff/100001/chrome/browser/net/net_error_tab_helper.h#newcode24 chrome/browser/net/net_error_tab_helper.h:24: class WebContentsInterfaceRegistry; On 2016/09/09 at 06:08:12, Sam McNally wrote: ...
4 years, 3 months ago (2016-09-09 17:22:27 UTC) #32
Ken Rockot(use gerrit already)
+tsepez@ for mojom
4 years, 3 months ago (2016-09-15 23:53:44 UTC) #58
jam
lgtm
4 years, 3 months ago (2016-09-16 00:18:57 UTC) #59
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2310583002/220001
4 years, 3 months ago (2016-09-16 00:19:51 UTC) #62
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/260699)
4 years, 3 months ago (2016-09-16 00:29:45 UTC) #64
Tom Sepez
mojom lgtm
4 years, 3 months ago (2016-09-16 19:50:17 UTC) #65
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2310583002/220001
4 years, 3 months ago (2016-09-16 19:52:35 UTC) #67
commit-bot: I haz the power
Committed patchset #8 (id:220001)
4 years, 3 months ago (2016-09-16 19:58:24 UTC) #68
commit-bot: I haz the power
4 years, 3 months ago (2016-09-16 19:59:56 UTC) #70
Message was sent while issue was closed.
Patchset 8 (id:??) landed as
https://crrev.com/e1807f0bc6e60da26c609da2e87aaaae4b03c7b2
Cr-Commit-Position: refs/heads/master@{#419259}

Powered by Google App Engine
This is Rietveld 408576698