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

Issue 2746293005: Migrated ChromeViewHostMsg_PageHasOSDD to Mojo. (Closed)

Created:
1 year, 1 month ago by martis
Modified:
1 year, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month ago (2017-03-15 06:47:34 UTC) #13
martis
For OWNERS +jochen +dcheng Thanks very much :)
1 year, 1 month ago (2017-03-15 07:11:43 UTC) #17
jochen (gone - plz use gerrit)
lgtm
1 year, 1 month 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, 1 month ago (2017-03-16 05:57:44 UTC) #21
martis
+mbarbella for IPC security OWNERS Thank you
1 year, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month ago (2017-03-22 06:09:01 UTC) #46
commit-bot: I haz the power
1 year, 1 month 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