Refactor cross-platform UI, especially Android and Desktop, to share more code.
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/580529891757508ee30404e36a2b187db2eedbfd commit 580529891757508ee30404e36a2b187db2eedbfd Author: Tommy C. Li <tommycli@chromium.org> Date: Mon Dec 10 21:38:08 2018 Omnibox Cross-platform UI: Fold QueryInOmnibox into LocationBarModel Now that QueryInOmnibox is stateless (since we are now using connection_info_initialized), it's logic is now small enough to fold into LocationBarModel. Previously, all the inputs of the function were sourced from LocationBarModel anyways, so it's a natural fit. Guide to reviewers: 1. Moves everything from query_in_omnibox.h/cc to location_bar_model_impl.h/cc. 2. Moves everything from query_in_omnibox_unittest.cc to location_bar_model_impl_unittest.cc. 3. Some end-to-end testing code in LocationBarLayoutTest for Query in Omnibox removed in favor of our native unit tests. There's now only one test that tests the integration between LocationBarModel and the Android View. 4. Everything else is mostly just plumbing. This CL removes ~380 SLOC and two classes. Bug: 874592, 912722 Change-Id: I1079d84a6b69a57410bc204ab8cedfdc5d27b3a3 Reviewed-on: https://chromium-review.googlesource.com/c/1366816 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Ted Choc <tedchoc@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#615268} [delete] https://crrev.com/30a9458e826a1c73880e8a267a324392d2e74eaa/chrome/android/java/src/org/chromium/chrome/browser/omnibox/QueryInOmnibox.java [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/android/java/src/org/chromium/chrome/browser/toolbar/LocationBarModel.java [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/android/java_sources.gni [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/LocationBarLayoutTest.java [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/BUILD.gn [delete] https://crrev.com/30a9458e826a1c73880e8a267a324392d2e74eaa/chrome/browser/android/omnibox/query_in_omnibox_android.cc [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/BUILD.gn [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/android/toolbar/location_bar_model_android.cc [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/android/toolbar/location_bar_model_android.h [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/omnibox/chrome_omnibox_client.cc [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/omnibox/chrome_omnibox_client.h [delete] https://crrev.com/30a9458e826a1c73880e8a267a324392d2e74eaa/chrome/browser/ui/omnibox/query_in_omnibox_factory.cc [delete] https://crrev.com/30a9458e826a1c73880e8a267a324392d2e74eaa/chrome/browser/ui/omnibox/query_in_omnibox_factory.h [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.cc [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.h [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/views/omnibox/omnibox_view_views.cc [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/views/omnibox/omnibox_view_views_unittest.cc [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/BUILD.gn [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/location_bar_model.h [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/location_bar_model_delegate.cc [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/location_bar_model_delegate.h [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/location_bar_model_impl.cc [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/location_bar_model_impl.h [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/location_bar_model_impl_unittest.cc [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/omnibox_client.cc [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/omnibox_client.h [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/omnibox_edit_model.cc [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/omnibox_edit_model.h [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/omnibox_edit_model_unittest.cc [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/omnibox_view.cc [delete] https://crrev.com/30a9458e826a1c73880e8a267a324392d2e74eaa/components/omnibox/browser/query_in_omnibox.cc [delete] https://crrev.com/30a9458e826a1c73880e8a267a324392d2e74eaa/components/omnibox/browser/query_in_omnibox.h [delete] https://crrev.com/30a9458e826a1c73880e8a267a324392d2e74eaa/components/omnibox/browser/query_in_omnibox_unittest.cc [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/test_location_bar_model.cc [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/test_location_bar_model.h [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/test_omnibox_client.cc [modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/test_omnibox_client.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/16c7a2e742f5eebf9bcd85d062a98e9c05fcd273 commit 16c7a2e742f5eebf9bcd85d062a98e9c05fcd273 Author: Tommy C. Li <tommycli@chromium.org> Date: Thu Dec 13 20:03:29 2018 Omnibox Cross-platform UI: Clean extra LocationBarModelDelegate methods. Eliminates: 1. LocationBarModelDelegate::FailsMalwareCheck 2. LocationBarModelDelegate::FailsBillingCheck Now that we have a general-purpose LocationBarModelDelegate::GetSecurityInfo, we don't need more the more specific methods. This is generally a good thing, as the delegate should be limited only to the code that you need the embedder to execute. Any logic that can live in the LocationBarModel should not live in LocationBarModelDelegate. This CL does change the behavior slightly for LocationBarModelDelegateIOS. Previously, FailsBillingCheck was always false on iOS. Now, it's possible for us to display the empty string for FailsBillingCheck if the status is MALICIOUS_CONTENT_STATUS_BILLING. That being said, it appears that it's the BillingInterstitial is always disabled on iOS anyways. Bug: 912722 Change-Id: I6f17c385360f1f82f9ce1f2214b5fb919f0b4d72 Reviewed-on: https://chromium-review.googlesource.com/c/1369643 Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org> Reviewed-by: Varun Khaneja <vakh@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#616411} [modify] https://crrev.com/16c7a2e742f5eebf9bcd85d062a98e9c05fcd273/chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.cc [modify] https://crrev.com/16c7a2e742f5eebf9bcd85d062a98e9c05fcd273/chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.h [modify] https://crrev.com/16c7a2e742f5eebf9bcd85d062a98e9c05fcd273/components/omnibox/browser/location_bar_model_delegate.cc [modify] https://crrev.com/16c7a2e742f5eebf9bcd85d062a98e9c05fcd273/components/omnibox/browser/location_bar_model_delegate.h [modify] https://crrev.com/16c7a2e742f5eebf9bcd85d062a98e9c05fcd273/components/omnibox/browser/location_bar_model_impl.cc [modify] https://crrev.com/16c7a2e742f5eebf9bcd85d062a98e9c05fcd273/ios/chrome/browser/ui/location_bar/location_bar_model_delegate_ios.h [modify] https://crrev.com/16c7a2e742f5eebf9bcd85d062a98e9c05fcd273/ios/chrome/browser/ui/location_bar/location_bar_model_delegate_ios.mm
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a commit 8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a Author: Tommy C. Li <tommycli@chromium.org> Date: Fri Jan 18 18:45:51 2019 Omnibox: Code health - Remove select_all parameter from FocusLocationBar Currently there is a a select_all parameter that we plumb through all layers of FocusLocationBar or its equivalent. This CL removes that parameter, and assumes it's always true. In other words, any time we focus the location bar, we also select-all. This parameter is pretty much always set to true. The only place where that parameter is set to false is from renderer-initiated focuses. Renderer-initiated focuses are used these scenarios: 1. When the New Tab Page is loaded, in which case the omnibox is empty, and selecting all does nothing. 2. For media OffscreenTab, there is no location bar, but the absent location bar is "focused" to prevent the page content from getting input focus. See OffscreenTab::ShouldFocusLocationBarByDefault. 3. For the PresentationReceiverWindowController, there is also no location bar. It's also to prevent the page content from getting focus. See PresentationReceiverWindowController:: ShouldFocusLocationBarByDefault. 4. When a new tab loads with about:blank, this will change the behavior to automatically select-all the "about:blank" text when the location bar is auto-focused. This is a desirable change. In all these cases, setting select_all to true would have no negative effect. Bug: 921777, 912722 Change-Id: Idefb493b196f89e23928e925ec9fcf015cac02fa Reviewed-on: https://chromium-review.googlesource.com/c/1418450 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Cr-Commit-Position: refs/heads/master@{#624213} [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/autocomplete/autocomplete_browsertest.cc [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/ash/chrome_new_window_client.cc [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/browser.cc [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/browser.h [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/browser_commands.cc [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/browser_window.h [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/location_bar/location_bar.h [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/search/instant_uitest_base.cc [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/views/dropdown_bar_host.cc [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/views/dropdown_bar_host_delegate.h [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/views/find_bar_view.cc [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/views/find_bar_view.h [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/views/frame/browser_view.cc [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/views/frame/browser_view.h [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/views/keyboard_access_browsertest.cc [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/views/location_bar/location_bar_view.h [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/browser/ui/views/toolbar/toolbar_view.cc [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/chrome/test/base/test_browser_window.h [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/content/browser/frame_host/render_frame_host_manager.cc [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/content/browser/frame_host/render_frame_host_manager.h [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/content/browser/web_contents/web_contents_impl.cc [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/content/browser/web_contents/web_contents_impl.h [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/content/browser/web_contents/web_contents_view_android.cc [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/content/browser/web_contents/web_contents_view_aura.cc [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/content/browser/web_contents/web_contents_view_mac.mm [modify] https://crrev.com/8bb5bf47fcd1ca8158ca67540c2573b507cfcc8a/content/public/browser/web_contents_delegate.h
Comment 1 by bugdroid1@chromium.org
, Dec 10