Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(853)

Issue 2746293005: Migrated ChromeViewHostMsg_PageHasOSDD to Mojo. (Closed)

Created:
1 year, 3 months ago by martis
Modified:
1 year, 3 months ago
CC:
Aaron Boodman, abarth-chromium, chromium-reviews, darin (slow to review), qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, Martin Barbella
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Migrated ChromeViewHostMsg_PageHasOSDD to Mojo. Tested by manually loading two search engines (DuckDuckGo and Ask) and logging the input to AssociateURLFetcherWithWebContents, which was found to be the same before and after the change. BUG=577685 Review-Url: https://codereview.chromium.org/2746293005 Cr-Commit-Position: refs/heads/master@{#458647} Committed: https://chromium.googlesource.com/chromium/src/+/e3cf9093ddcaa466a5cf7153745e37d579679ab7

Patch Set 1 #

Patch Set 2 : Manually reverted format changes (from my auto-formatting plugin). #

Total comments: 9

Patch Set 3 : Review changes. #

Patch Set 4 : Cleaned up comments and includes. #

Patch Set 5 : Removed public constructor and re-added mojo file. #

Total comments: 1

Patch Set 6 : Merged with another Mojo CL. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+47 lines, -38 lines) Patch
M chrome/browser/chrome_content_browser_manifest_overlay.json View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/search_engines/search_engine_tab_helper.h View 1 2 3 4 2 chunks +11 lines, -8 lines 0 comments Download
M chrome/browser/ui/search_engines/search_engine_tab_helper.cc View 1 2 3 4 2 chunks +9 lines, -20 lines 0 comments Download
M chrome/common/BUILD.gn View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
A chrome/common/open_search_description_document_handler.mojom View 1 2 3 4 1 chunk +14 lines, -0 lines 0 comments Download
M chrome/common/render_messages.h View 1 2 3 4 5 1 chunk +0 lines, -6 lines 0 comments Download
M chrome/renderer/chrome_render_frame_observer.cc View 1 2 3 4 5 3 chunks +11 lines, -4 lines 0 comments Download

Messages

Total messages: 49 (29 generated)
Sam McNally
Rietveld doesn't send emails about reviewer changes so you'll need to mail a comment when ...
1 year, 3 months ago (2017-03-15 00:38:49 UTC) #3
martis
On 2017/03/15 00:38:49, Sam McNally wrote: > Rietveld doesn't send emails about reviewer changes so ...
1 year, 3 months ago (2017-03-15 05:00:28 UTC) #4
martis
Thanks for the review; I've attempted to address your concerns. https://codereview.chromium.org/2746293005/diff/20001/chrome/browser/ui/search_engines/search_engine_tab_helper.h File chrome/browser/ui/search_engines/search_engine_tab_helper.h (right): https://codereview.chromium.org/2746293005/diff/20001/chrome/browser/ui/search_engines/search_engine_tab_helper.h#newcode24 ...
1 year, 3 months ago (2017-03-15 05:00:56 UTC) #5
Sam McNally
lgtm https://codereview.chromium.org/2746293005/diff/20001/chrome/browser/ui/search_engines/search_engine_tab_helper.h File chrome/browser/ui/search_engines/search_engine_tab_helper.h (right): https://codereview.chromium.org/2746293005/diff/20001/chrome/browser/ui/search_engines/search_engine_tab_helper.h#newcode24 chrome/browser/ui/search_engines/search_engine_tab_helper.h:24: SearchEngineTabHelper(); On 2017/03/15 05:00:56, martis wrote: > On ...
1 year, 3 months ago (2017-03-15 05:55:15 UTC) #8
martis
On 2017/03/15 05:55:15, Sam McNally wrote: > lgtm > > https://codereview.chromium.org/2746293005/diff/20001/chrome/browser/ui/search_engines/search_engine_tab_helper.h > File chrome/browser/ui/search_engines/search_engine_tab_helper.h (right): ...
1 year, 3 months ago (2017-03-15 06:21:34 UTC) #11
Sam McNally
On 2017/03/15 06:21:34, martis wrote: > On 2017/03/15 05:55:15, Sam McNally wrote: > > lgtm ...
1 year, 3 months ago (2017-03-15 06:26:12 UTC) #12
martis
On 2017/03/15 06:26:12, Sam McNally wrote: > On 2017/03/15 06:21:34, martis wrote: > > On ...
1 year, 3 months ago (2017-03-15 06:47:34 UTC) #13
martis
For OWNERS +jochen +dcheng Thanks very much :)
1 year, 3 months ago (2017-03-15 07:11:43 UTC) #17
jochen (gone - plz use gerrit)
lgtm
1 year, 3 months ago (2017-03-15 15:50:15 UTC) #20
dcheng
mojo lgtm https://codereview.chromium.org/2746293005/diff/80001/chrome/browser/ui/search_engines/search_engine_tab_helper.cc File chrome/browser/ui/search_engines/search_engine_tab_helper.cc (right): https://codereview.chromium.org/2746293005/diff/80001/chrome/browser/ui/search_engines/search_engine_tab_helper.cc#newcode105 chrome/browser/ui/search_engines/search_engine_tab_helper.cc:105: web_contents()->GetMainFrame()) Huh. Did we not check this ...
1 year, 3 months ago (2017-03-16 05:57:44 UTC) #21
martis
+mbarbella for IPC security OWNERS Thank you
1 year, 3 months ago (2017-03-17 01:26:03 UTC) #23
dcheng
On 2017/03/17 01:26:03, martis wrote: > +mbarbella for IPC security OWNERS > Thank you FWIW, ...
1 year, 3 months ago (2017-03-17 01:39:12 UTC) #24
martis
On 2017/03/17 01:39:12, dcheng wrote: > On 2017/03/17 01:26:03, martis wrote: > > +mbarbella for ...
1 year, 3 months ago (2017-03-17 02:01:08 UTC) #28
dcheng
On 2017/03/17 02:01:08, martis wrote: > On 2017/03/17 01:39:12, dcheng wrote: > > On 2017/03/17 ...
1 year, 3 months ago (2017-03-17 02:02:53 UTC) #29
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/2746293005/100001
1 year, 3 months ago (2017-03-21 22:40:17 UTC) #38
commit-bot: I haz the power
Try jobs failed on following builders: ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xcode-clang/builds/63513)
1 year, 3 months ago (2017-03-21 23:06:43 UTC) #40
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/2746293005/100001
1 year, 3 months ago (2017-03-22 00:28:50 UTC) #42
commit-bot: I haz the power
Try jobs failed on following builders: ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't started yet, builder ...
1 year, 3 months ago (2017-03-22 02:30:04 UTC) #44
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/2746293005/100001
1 year, 3 months ago (2017-03-22 06:09:01 UTC) #46
commit-bot: I haz the power
1 year, 3 months ago (2017-03-22 06:48:38 UTC) #49
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/e3cf9093ddcaa466a5cf7153745e...

Powered by Google App Engine
This is Rietveld 408576698