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

Issue 2746293005: Migrated ChromeViewHostMsg_PageHasOSDD to Mojo. (Closed)

Created:
8 months, 2 weeks ago by martis
Modified:
8 months, 1 week 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
Trybot results:  chromium_presubmit   linux_android_rel_ng   ios-simulator-xcode-clang   linux_chromium_chromeos_ozone_rel_ng   chromium_presubmit   win_chromium_rel_ng   linux_chromium_asan_rel_ng   ios-simulator-xcode-clang   win_chromium_x64_rel_ng   chromeos_amd64-generic_chromium_compile_only_ng   mac_chromium_rel_ng   linux_chromium_chromeos_rel_ng   ios-device-xcode-clang   linux_chromium_compile_dbg_ng   linux_chromium_tsan_rel_ng   ios-device   ios-simulator   linux_chromium_rel_ng   mac_chromium_compile_dbg_ng   win_chromium_compile_dbg_ng   ios-simulator-xcode-clang   win_clang   android_n5x_swarming_rel   cast_shell_android   android_cronet   android_compile_dbg   android_arm64_dbg_recipe   cast_shell_linux   chromium_presubmit   android_clang_dbg_recipe   chromeos_daisy_chromium_compile_only_ng   linux_android_rel_ng   linux_chromium_chromeos_ozone_rel_ng   android_n5x_swarming_rel   linux_chromium_compile_dbg_ng   ios-simulator   linux_chromium_tsan_rel_ng   chromeos_daisy_chromium_compile_only_ng   android_clang_dbg_recipe   chromium_presubmit   linux_chromium_chromeos_rel_ng   chromeos_amd64-generic_chromium_compile_only_ng   linux_chromium_asan_rel_ng   ios-device-xcode-clang   android_arm64_dbg_recipe   android_cronet   ios-simulator-xcode-clang   linux_android_rel_ng   win_chromium_rel_ng   win_chromium_x64_rel_ng   cast_shell_linux   mac_chromium_compile_dbg_ng   win_clang   linux_chromium_rel_ng   mac_chromium_rel_ng   ios-device   android_compile_dbg   cast_shell_android   win_chromium_compile_dbg_ng   linux_chromium_chromeos_ozone_rel_ng 

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