diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index 52abc0a..64d3ff7a 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd
@@ -490,11 +490,6 @@ <message name="IDS_EDIT" desc="Edit menu item"> &Edit </message> - <if expr="is_macosx"> - <message name="IDS_ACCNAME_TAB" desc="A generic description of a tab button's role" meaning="UI element"> - Tab - </message> - </if> <message name="IDS_SAVE" desc="Used on a button to save information you are editing."> Save </message>
diff --git a/chrome/browser/android/banners/app_banner_manager_android.cc b/chrome/browser/android/banners/app_banner_manager_android.cc index a8a21c59..8ae8d3c3 100644 --- a/chrome/browser/android/banners/app_banner_manager_android.cc +++ b/chrome/browser/android/banners/app_banner_manager_android.cc
@@ -107,6 +107,9 @@ native_app_package_ = ConvertJavaStringToUTF8(env, japp_package); icon_url_ = GURL(ConvertJavaStringToUTF8(env, jicon_url)); + if (!CheckIfShouldShowBanner()) + return false; + return ManifestIconDownloader::Download( web_contents(), icon_url_, GetIdealIconSizeInPx(), GetMinimumIconSizeInPx(),
diff --git a/chrome/browser/banners/app_banner_manager.cc b/chrome/browser/banners/app_banner_manager.cc index 55e03e54..908b932 100644 --- a/chrome/browser/banners/app_banner_manager.cc +++ b/chrome/browser/banners/app_banner_manager.cc
@@ -240,14 +240,7 @@ } void AppBannerManager::PerformInstallableCheck() { - if (IsWebAppInstalled(web_contents()->GetBrowserContext(), - manifest_.start_url, manifest_url_) && - !IsDebugMode()) { - ReportStatus(web_contents(), ALREADY_INSTALLED); - Stop(); - } - - if (!is_active_) + if (!CheckIfShouldShowBanner()) return; // Fetch and verify the other required information. @@ -338,13 +331,6 @@ void AppBannerManager::SendBannerPromptRequest() { RecordCouldShowBanner(); - // Given all of the other checks that have been made, the only possible reason - // for stopping now is that the app has been added to the homescreen. - if (!IsDebugMode() && !CheckIfShouldShowBanner()) { - Stop(); - return; - } - TrackBeforeInstallEvent(BEFORE_INSTALL_EVENT_CREATED); event_request_id_ = ++gCurrentRequestID; @@ -450,37 +436,43 @@ } bool AppBannerManager::CheckIfShouldShowBanner() { - content::WebContents* contents = web_contents(); - DCHECK(contents); - - InstallableStatusCode code = AppBannerSettingsHelper::ShouldShowBanner( - contents, validated_url_, GetAppIdentifier(), GetCurrentTime()); - if (code == NO_ERROR_DETECTED) + if (IsDebugMode()) return true; - switch (code) { - case ALREADY_INSTALLED: - banners::TrackDisplayEvent(banners::DISPLAY_EVENT_INSTALLED_PREVIOUSLY); - break; - case PREVIOUSLY_BLOCKED: - banners::TrackDisplayEvent(banners::DISPLAY_EVENT_BLOCKED_PREVIOUSLY); - break; - case PREVIOUSLY_IGNORED: - banners::TrackDisplayEvent(banners::DISPLAY_EVENT_IGNORED_PREVIOUSLY); - break; - case INSUFFICIENT_ENGAGEMENT: - banners::TrackDisplayEvent(banners::DISPLAY_EVENT_NOT_VISITED_ENOUGH); - break; - default: - break; + // Check whether we are permitted to show the banner. If we have already + // added this site to homescreen, or if the banner has been shown too + // recently, prevent the banner from being shown. + content::WebContents* contents = web_contents(); + InstallableStatusCode code = AppBannerSettingsHelper::ShouldShowBanner( + contents, validated_url_, GetAppIdentifier(), GetCurrentTime()); + + if (code == NO_ERROR_DETECTED && + IsWebAppInstalled(contents->GetBrowserContext(), manifest_.start_url, + manifest_url_)) { + code = ALREADY_INSTALLED; } - // If we are in debug mode, AppBannerSettingsHelper::ShouldShowBanner must - // return NO_ERROR_DETECTED (bypass flag is set) or we must not have entered - // this method. - DCHECK(!IsDebugMode()); - ReportStatus(web_contents(), code); - return false; + if (code != NO_ERROR_DETECTED) { + switch (code) { + case ALREADY_INSTALLED: + banners::TrackDisplayEvent(banners::DISPLAY_EVENT_INSTALLED_PREVIOUSLY); + break; + case PREVIOUSLY_BLOCKED: + banners::TrackDisplayEvent(banners::DISPLAY_EVENT_BLOCKED_PREVIOUSLY); + break; + case PREVIOUSLY_IGNORED: + banners::TrackDisplayEvent(banners::DISPLAY_EVENT_IGNORED_PREVIOUSLY); + break; + case PACKAGE_NAME_OR_START_URL_EMPTY: + break; + default: + NOTREACHED(); + } + ReportStatus(contents, code); + Stop(); + return false; + } + return true; } void AppBannerManager::OnBannerPromptReply(
diff --git a/chrome/browser/banners/app_banner_manager.h b/chrome/browser/banners/app_banner_manager.h index 4301ca90..be43a91 100644 --- a/chrome/browser/banners/app_banner_manager.h +++ b/chrome/browser/banners/app_banner_manager.h
@@ -104,6 +104,11 @@ explicit AppBannerManager(content::WebContents* web_contents); ~AppBannerManager() override; + // Returns true if the banner should be shown. Returns false if the banner has + // been shown too recently, or if the app has already been installed. + // GetAppIdentifier() must return a valid value for this method to work. + bool CheckIfShouldShowBanner(); + // Return a string identifying this app for metrics. virtual std::string GetAppIdentifier(); @@ -221,9 +226,6 @@ // platform-specific. virtual void ShowBanner() = 0; - // Returns true if the banner should be shown. - bool CheckIfShouldShowBanner(); - // Called after the manager sends a message to the renderer regarding its // intention to show a prompt. The renderer will send a message back with the // opportunity to cancel.
diff --git a/chrome/browser/banners/app_banner_settings_helper.cc b/chrome/browser/banners/app_banner_settings_helper.cc index f314a0c..28989395 100644 --- a/chrome/browser/banners/app_banner_settings_helper.cc +++ b/chrome/browser/banners/app_banner_settings_helper.cc
@@ -246,12 +246,6 @@ const GURL& origin_url, const std::string& package_name_or_start_url, base::Time time) { - // Ignore all checks if the flag to do so is set. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kBypassAppBannerEngagementChecks)) { - return NO_ERROR_DETECTED; - } - // Never show a banner when the package name or URL is empty. if (package_name_or_start_url.empty()) return PACKAGE_NAME_OR_START_URL_EMPTY; @@ -277,9 +271,8 @@ base::Time shown_time = GetSingleBannerEvent(web_contents, origin_url, package_name_or_start_url, APP_BANNER_EVENT_DID_SHOW); - if (time - shown_time < base::TimeDelta::FromDays(gDaysAfterIgnoredToShow)) { + if (time - shown_time < base::TimeDelta::FromDays(gDaysAfterIgnoredToShow)) return PREVIOUSLY_IGNORED; - } return NO_ERROR_DETECTED; }
diff --git a/chrome/browser/chromeos/arc/arc_service_launcher.cc b/chrome/browser/chromeos/arc/arc_service_launcher.cc index 356664c3..06182b0 100644 --- a/chrome/browser/chromeos/arc/arc_service_launcher.cc +++ b/chrome/browser/chromeos/arc/arc_service_launcher.cc
@@ -98,20 +98,20 @@ base::MakeUnique<ArcBootPhaseMonitorBridge>(arc_bridge_service)); arc_service_manager_->AddService( base::MakeUnique<ArcClipboardBridge>(arc_bridge_service)); - arc_service_manager_->AddService( - base::MakeUnique<ArcFileSystemService>(arc_bridge_service)); arc_service_manager_->AddService(base::MakeUnique<ArcCrashCollectorBridge>( arc_bridge_service, arc_service_manager_->blocking_task_runner())); arc_service_manager_->AddService( base::MakeUnique<ArcDownloadsWatcherService>(arc_bridge_service)); arc_service_manager_->AddService( base::MakeUnique<ArcEnterpriseReportingService>(arc_bridge_service)); + arc_service_manager_->AddService( + base::MakeUnique<ArcFileSystemService>(arc_bridge_service)); + arc_service_manager_->AddService( + base::MakeUnique<ArcImeService>(arc_bridge_service)); arc_service_manager_->AddService(base::MakeUnique<ArcIntentHelperBridge>( arc_bridge_service, arc_service_manager_->icon_loader(), arc_service_manager_->activity_resolver())); arc_service_manager_->AddService( - base::MakeUnique<ArcImeService>(arc_bridge_service)); - arc_service_manager_->AddService( base::MakeUnique<ArcMetricsService>(arc_bridge_service)); arc_service_manager_->AddService( base::MakeUnique<ArcNetHostImpl>(arc_bridge_service));
diff --git a/chrome/browser/chromeos/extensions/file_manager/private_api_strings.cc b/chrome/browser/chromeos/extensions/file_manager/private_api_strings.cc index 2bfa997..8f9fcf3 100644 --- a/chrome/browser/chromeos/extensions/file_manager/private_api_strings.cc +++ b/chrome/browser/chromeos/extensions/file_manager/private_api_strings.cc
@@ -160,6 +160,15 @@ IDS_FILE_BROWSER_SYNC_SERVICE_UNAVAILABLE_ERROR); } +void AddStringsForMediaView(base::DictionaryValue* dict) { + SET_STRING("MEDIA_VIEW_AUDIO_ROOT_LABEL", + IDS_FILE_BROWSER_MEDIA_VIEW_AUDIO_ROOT_LABEL); + SET_STRING("MEDIA_VIEW_IMAGES_ROOT_LABEL", + IDS_FILE_BROWSER_MEDIA_VIEW_IMAGES_ROOT_LABEL); + SET_STRING("MEDIA_VIEW_VIDEOS_ROOT_LABEL", + IDS_FILE_BROWSER_MEDIA_VIEW_VIDEOS_ROOT_LABEL); +} + void AddStringsForGallery(base::DictionaryValue* dict) { SET_STRING("GALLERY_ASPECT_RATIO_16_9", IDS_FILE_BROWSER_GALLERY_ASPECT_RATIO_16_9); @@ -357,6 +366,7 @@ std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue()); AddStringsForDrive(dict.get()); + AddStringsForMediaView(dict.get()); AddStringsForFileTypes(dict.get()); AddStringsForGallery(dict.get()); AddStringsForMediaPlayer(dict.get());
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn index 7488e79..de0ea0a 100644 --- a/chrome/browser/ui/BUILD.gn +++ b/chrome/browser/ui/BUILD.gn
@@ -473,6 +473,7 @@ "//chrome/browser/ui/webui/omnibox:mojo_bindings", "//chrome/browser/ui/webui/usb_internals:mojo_bindings", "//chrome/common", + "//chrome/common:instant_mojom", "//chrome/common/net", "//chrome/installer/util:with_no_strings", "//components/app_modal",
diff --git a/chrome/browser/ui/cocoa/tabs/tab_view.mm b/chrome/browser/ui/cocoa/tabs/tab_view.mm index 04f9b165..f454cfb6 100644 --- a/chrome/browser/ui/cocoa/tabs/tab_view.mm +++ b/chrome/browser/ui/cocoa/tabs/tab_view.mm
@@ -15,7 +15,6 @@ #import "chrome/browser/ui/cocoa/tabs/tab_window_controller.h" #import "chrome/browser/ui/cocoa/themed_window.h" #import "chrome/browser/ui/cocoa/view_id_util.h" -#include "chrome/grit/generated_resources.h" #include "chrome/grit/theme_resources.h" #include "skia/ext/skia_utils_mac.h" #import "third_party/google_toolbox_for_mac/src/AppKit/GTMFadeTruncatingTextFieldCell.h" @@ -26,6 +25,7 @@ #include "ui/base/material_design/material_design_controller.h" #include "ui/base/resource/resource_bundle.h" #include "ui/gfx/scoped_ns_graphics_context_save_gstate_mac.h" +#include "ui/strings/grit/ui_strings.h" namespace { @@ -759,7 +759,7 @@ if ([attribute isEqual:NSAccessibilityRoleAttribute]) return NSAccessibilityRadioButtonRole; if ([attribute isEqual:NSAccessibilityRoleDescriptionAttribute]) - return l10n_util::GetNSStringWithFixup(IDS_ACCNAME_TAB); + return l10n_util::GetNSStringWithFixup(IDS_ACCNAME_TAB_ROLE_DESCRIPTION); if ([attribute isEqual:NSAccessibilityTitleAttribute]) return [controller_ title]; if ([attribute isEqual:NSAccessibilityValueAttribute])
diff --git a/chrome/browser/ui/search/instant_search_prerenderer_unittest.cc b/chrome/browser/ui/search/instant_search_prerenderer_unittest.cc index 075768b..5d18b07 100644 --- a/chrome/browser/ui/search/instant_search_prerenderer_unittest.cc +++ b/chrome/browser/ui/search/instant_search_prerenderer_unittest.cc
@@ -28,10 +28,12 @@ #include "chrome/browser/search/search.h" #include "chrome/browser/search_engines/template_url_service_factory.h" #include "chrome/browser/ui/browser_instant_controller.h" +#include "chrome/browser/ui/search/search_ipc_router.h" #include "chrome/browser/ui/search/search_tab_helper.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/common/render_messages.h" #include "chrome/common/search/instant_types.h" +#include "chrome/common/search/mock_searchbox.h" #include "components/omnibox/browser/autocomplete_match.h" #include "content/public/browser/navigation_controller.h" #include "content/public/browser/web_contents.h" @@ -43,9 +45,20 @@ #include "ui/gfx/geometry/size.h" using base::ASCIIToUTF16; +using testing::_; +using testing::AllOf; +using testing::Eq; +using testing::Field; +using testing::Return; namespace { +class MockSearchBoxClientFactory + : public SearchIPCRouter::SearchBoxClientFactory { + public: + MOCK_METHOD0(GetSearchBox, chrome::mojom::SearchBox*(void)); +}; + using content::Referrer; using prerender::Origin; using prerender::PrerenderContents; @@ -56,13 +69,13 @@ class DummyPrerenderContents : public PrerenderContents { public: - DummyPrerenderContents( - PrerenderManager* prerender_manager, - Profile* profile, - const GURL& url, - const Referrer& referrer, - Origin origin, - bool call_did_finish_load); + DummyPrerenderContents(PrerenderManager* prerender_manager, + Profile* profile, + const GURL& url, + const Referrer& referrer, + Origin origin, + bool call_did_finish_load, + chrome::mojom::SearchBox* search_box); void StartPrerendering( const gfx::Rect& bounds, @@ -74,15 +87,16 @@ Profile* profile_; const GURL url_; bool call_did_finish_load_; + chrome::mojom::SearchBox* search_box_; DISALLOW_COPY_AND_ASSIGN(DummyPrerenderContents); }; class DummyPrerenderContentsFactory : public PrerenderContents::Factory { public: - explicit DummyPrerenderContentsFactory(bool call_did_finish_load) - : call_did_finish_load_(call_did_finish_load) { - } + DummyPrerenderContentsFactory(bool call_did_finish_load, + chrome::mojom::SearchBox* search_box) + : call_did_finish_load_(call_did_finish_load), search_box_(search_box) {} PrerenderContents* CreatePrerenderContents( PrerenderManager* prerender_manager, @@ -93,6 +107,7 @@ private: bool call_did_finish_load_; + chrome::mojom::SearchBox* search_box_; DISALLOW_COPY_AND_ASSIGN(DummyPrerenderContentsFactory); }; @@ -103,12 +118,13 @@ const GURL& url, const Referrer& referrer, Origin origin, - bool call_did_finish_load) + bool call_did_finish_load, + chrome::mojom::SearchBox* search_box) : PrerenderContents(prerender_manager, profile, url, referrer, origin), profile_(profile), url_(url), - call_did_finish_load_(call_did_finish_load) { -} + call_did_finish_load_(call_did_finish_load), + search_box_(search_box) {} void DummyPrerenderContents::StartPrerendering( const gfx::Rect& bounds, @@ -122,6 +138,12 @@ content::NavigationController::LoadURLParams params(url_); prerender_contents_->GetController().LoadURLWithParams(params); SearchTabHelper::CreateForWebContents(prerender_contents_.get()); + auto* search_tab = + SearchTabHelper::FromWebContents(prerender_contents_.get()); + auto factory = base::MakeUnique<MockSearchBoxClientFactory>(); + ON_CALL(*factory, GetSearchBox()).WillByDefault(Return(search_box_)); + search_tab->ipc_router_for_testing() + .set_search_box_client_factory_for_testing(std::move(factory)); prerendering_has_started_ = true; DCHECK(session_storage_namespace); @@ -149,7 +171,7 @@ const Referrer& referrer, Origin origin) { return new DummyPrerenderContents(prerender_manager, profile, url, referrer, - origin, call_did_finish_load_); + origin, call_did_finish_load_, search_box_); } } // namespace @@ -170,8 +192,8 @@ AddTab(browser(), GURL(url::kAboutBlankURL)); PrerenderManagerFactory::GetForBrowserContext(browser()->profile()) - ->SetPrerenderContentsFactoryForTest( - new DummyPrerenderContentsFactory(call_did_finish_load)); + ->SetPrerenderContentsFactoryForTest(new DummyPrerenderContentsFactory( + call_did_finish_load, &mock_search_box_)); if (prerender_search_results_base_page) { content::SessionStorageNamespace* session_storage_namespace = GetActiveWebContents()->GetController(). @@ -199,13 +221,6 @@ return GetInstantSearchPrerenderer()->prerender_contents(); } - bool MessageWasSent(uint32_t id) { - content::MockRenderProcessHost* process = - static_cast<content::MockRenderProcessHost*>( - prerender_contents()->GetMainFrame()->GetProcess()); - return process->sink().GetFirstMessageMatching(id) != NULL; - } - content::WebContents* GetActiveWebContents() const { return browser()->tab_strip_model()->GetWebContentsAt(0); } @@ -222,6 +237,11 @@ EXPECT_TRUE(prerenderer->CanCommitQuery(GetActiveWebContents(), query)); EXPECT_NE(static_cast<PrerenderHandle*>(NULL), prerender_handle()); } + + MockSearchBox* mock_search_box() { return &mock_search_box_; } + + private: + MockSearchBox mock_search_box_; }; TEST_F(InstantSearchPrerendererTest, GetSearchTermsFromPrerenderedPage) { @@ -247,11 +267,10 @@ Init(true, true); EXPECT_TRUE(prerender_handle()->IsFinishedLoading()); InstantSearchPrerenderer* prerenderer = GetInstantSearchPrerenderer(); + EXPECT_CALL(*mock_search_box(), SetSuggestionToPrefetch(_)); prerenderer->Prerender( InstantSuggestion(ASCIIToUTF16("flowers"), std::string())); EXPECT_EQ("flowers", base::UTF16ToASCII(prerenderer->get_last_query())); - EXPECT_TRUE(MessageWasSent( - ChromeViewMsg_SearchBoxSetSuggestionToPrefetch::ID)); } TEST_F(InstantSearchPrerendererTest, DoNotPrefetchSearchResults) { @@ -259,11 +278,10 @@ // Page hasn't finished loading yet. EXPECT_FALSE(prerender_handle()->IsFinishedLoading()); InstantSearchPrerenderer* prerenderer = GetInstantSearchPrerenderer(); + EXPECT_CALL(*mock_search_box(), SetSuggestionToPrefetch(_)).Times(0); prerenderer->Prerender( InstantSuggestion(ASCIIToUTF16("flowers"), std::string())); EXPECT_EQ("", base::UTF16ToASCII(prerenderer->get_last_query())); - EXPECT_FALSE(MessageWasSent( - ChromeViewMsg_SearchBoxSetSuggestionToPrefetch::ID)); } TEST_F(InstantSearchPrerendererTest, CanCommitQuery) { @@ -285,8 +303,8 @@ base::string16 query = ASCIIToUTF16("flowers"); PrerenderSearchQuery(query); InstantSearchPrerenderer* prerenderer = GetInstantSearchPrerenderer(); + EXPECT_CALL(*mock_search_box(), Submit(_, _)); prerenderer->Commit(query, EmbeddedSearchRequestParams()); - EXPECT_TRUE(MessageWasSent(ChromeViewMsg_SearchBoxSubmit::ID)); } TEST_F(InstantSearchPrerendererTest, CancelPrerenderRequestOnTabChangeEvent) { @@ -435,7 +453,7 @@ void SetUp() override { ASSERT_TRUE(base::FieldTrialList::CreateFieldTrial("EmbeddedSearch", "Group1 strk:20")); - InstantUnitTestBase::SetUp(); + InstantSearchPrerendererTest::SetUp(); } }; @@ -504,6 +522,17 @@ TEST_F(TestUsePrerenderPage, SetEmbeddedSearchRequestParams) { PrerenderSearchQuery(ASCIIToUTF16("foo")); EXPECT_TRUE(browser()->instant_controller()); + EXPECT_CALL( + *mock_search_box(), + Submit(Eq(ASCIIToUTF16("foo")), + AllOf(Field(&EmbeddedSearchRequestParams::original_query, + Eq(base::ASCIIToUTF16("f"))), + Field(&EmbeddedSearchRequestParams::input_encoding, + Eq(base::ASCIIToUTF16("utf-8"))), + Field(&EmbeddedSearchRequestParams::rlz_parameter_value, + Eq(base::ASCIIToUTF16(""))), + Field(&EmbeddedSearchRequestParams::assisted_query_stats, + Eq(base::ASCIIToUTF16("chrome...0")))))); // Open a search results page. Query extraction flag is disabled in field // trials. Search results page URL does not contain search terms replacement @@ -511,21 +540,5 @@ GURL url("https://www.google.com/url?bar=foo&aqs=chrome...0&ie=utf-8&oq=f"); browser()->instant_controller()->OpenInstant( WindowOpenDisposition::CURRENT_TAB, url); - content::MockRenderProcessHost* process = - static_cast<content::MockRenderProcessHost*>( - prerender_contents()->GetMainFrame()->GetProcess()); - const IPC::Message* message = process->sink().GetFirstMessageMatching( - ChromeViewMsg_SearchBoxSubmit::ID); - ASSERT_TRUE(message); - - // Verify the IPC message params. - std::tuple<base::string16, EmbeddedSearchRequestParams> params; - ChromeViewMsg_SearchBoxSubmit::Read(message, ¶ms); - EXPECT_EQ("foo", base::UTF16ToASCII(std::get<0>(params))); - EXPECT_EQ("f", base::UTF16ToASCII(std::get<1>(params).original_query)); - EXPECT_EQ("utf-8", base::UTF16ToASCII(std::get<1>(params).input_encoding)); - EXPECT_EQ("", base::UTF16ToASCII(std::get<1>(params).rlz_parameter_value)); - EXPECT_EQ("chrome...0", - base::UTF16ToASCII(std::get<1>(params).assisted_query_stats)); } #endif
diff --git a/chrome/browser/ui/search/instant_tab_unittest.cc b/chrome/browser/ui/search/instant_tab_unittest.cc index b417daf..8991f26 100644 --- a/chrome/browser/ui/search/instant_tab_unittest.cc +++ b/chrome/browser/ui/search/instant_tab_unittest.cc
@@ -9,9 +9,11 @@ #include <memory> #include "base/command_line.h" +#include "chrome/browser/ui/search/search_ipc_router.h" #include "chrome/browser/ui/search/search_tab_helper.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/render_messages.h" +#include "chrome/common/search/mock_searchbox.h" #include "chrome/common/url_constants.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h" #include "content/public/browser/navigation_controller.h" @@ -23,6 +25,8 @@ #include "testing/gtest/include/gtest/gtest.h" #include "url/gurl.h" +using testing::Return; + class Profile; namespace { @@ -36,8 +40,13 @@ void(const content::WebContents* contents, bool supports_instant)); MOCK_METHOD2(InstantTabAboutToNavigateMainFrame, - void(const content::WebContents* contents, - const GURL& url)); + void(const content::WebContents* contents, const GURL& url)); +}; + +class MockSearchBoxClientFactory + : public SearchIPCRouter::SearchBoxClientFactory { + public: + MOCK_METHOD0(GetSearchBox, chrome::mojom::SearchBox*(void)); }; } // namespace @@ -56,11 +65,17 @@ std::unique_ptr<InstantTab> page; FakePageDelegate delegate; + MockSearchBox mock_search_box; }; void InstantTabTest::SetUp() { ChromeRenderViewHostTestHarness::SetUp(); SearchTabHelper::CreateForWebContents(web_contents()); + auto factory = base::MakeUnique<MockSearchBoxClientFactory>(); + ON_CALL(*factory, GetSearchBox()).WillByDefault(Return(&mock_search_box)); + search_tab() + ->ipc_router_for_testing() + .set_search_box_client_factory_for_testing(std::move(factory)); } TEST_F(InstantTabTest, DetermineIfPageSupportsInstant_Local) { @@ -78,13 +93,8 @@ EXPECT_FALSE(SupportsInstant()); page->Init(); NavigateAndCommit(GURL("chrome-search://foo/bar")); - process()->sink().ClearMessages(); + EXPECT_CALL(mock_search_box, DetermineIfPageSupportsInstant()); search_tab()->DetermineIfPageSupportsInstant(); - const IPC::Message* message = process()->sink().GetFirstMessageMatching( - ChromeViewMsg_DetermineIfPageSupportsInstant::ID); - ASSERT_TRUE(message != NULL); - EXPECT_EQ(web_contents()->GetRenderViewHost()->GetRoutingID(), - message->routing_id()); } TEST_F(InstantTabTest, PageURLDoesntBelongToInstantRenderer) { @@ -97,13 +107,10 @@ // SearchTabHelper::DeterminerIfPageSupportsInstant() should return // immediately without dispatching any message to the renderer. NavigateAndCommit(GURL("http://www.example.com")); - process()->sink().ClearMessages(); EXPECT_CALL(delegate, InstantSupportDetermined(web_contents(), false)); + EXPECT_CALL(mock_search_box, DetermineIfPageSupportsInstant()).Times(0); search_tab()->DetermineIfPageSupportsInstant(); - const IPC::Message* message = process()->sink().GetFirstMessageMatching( - ChromeViewMsg_DetermineIfPageSupportsInstant::ID); - ASSERT_TRUE(message == NULL); EXPECT_FALSE(SupportsInstant()); } @@ -114,13 +121,8 @@ EXPECT_FALSE(SupportsInstant()); page->Init(); NavigateAndCommit(GURL("chrome-search://foo/bar")); - process()->sink().ClearMessages(); + EXPECT_CALL(mock_search_box, DetermineIfPageSupportsInstant()); search_tab()->DetermineIfPageSupportsInstant(); - const IPC::Message* message = process()->sink().GetFirstMessageMatching( - ChromeViewMsg_DetermineIfPageSupportsInstant::ID); - ASSERT_TRUE(message != NULL); - EXPECT_EQ(web_contents()->GetRenderViewHost()->GetRoutingID(), - message->routing_id()); EXPECT_CALL(delegate, InstantSupportDetermined(web_contents(), true));
diff --git a/chrome/browser/ui/search/search_ipc_router.cc b/chrome/browser/ui/search/search_ipc_router.cc index f57f188..2dcba5b 100644 --- a/chrome/browser/ui/search/search_ipc_router.cc +++ b/chrome/browser/ui/search/search_ipc_router.cc
@@ -11,7 +11,66 @@ #include "chrome/common/render_messages.h" #include "components/search/search.h" #include "content/public/browser/navigation_details.h" +#include "content/public/browser/render_frame_host.h" +#include "content/public/browser/render_process_host.h" #include "content/public/browser/web_contents.h" +#include "content/public/common/associated_interface_provider.h" +#include "content/public/common/child_process_host.h" + +namespace { + +bool IsRenderedInInstantProcess(content::WebContents* web_contents) { + // TODO(tibell): Instead of rejecting messages check at connection time if we + // want to talk to the render frame or not. Later, in an out-of-process iframe + // world, the IsRenderedInInstantProcess check will have to be done, as it's + // based on the RenderView. + Profile* profile = + Profile::FromBrowserContext(web_contents->GetBrowserContext()); + return search::IsRenderedInInstantProcess(web_contents, profile); +} + +class SearchBoxClientFactoryImpl + : public SearchIPCRouter::SearchBoxClientFactory { + public: + // The |web_contents| must outlive this object. + explicit SearchBoxClientFactoryImpl(content::WebContents* web_contents) + : web_contents_(web_contents), + last_connected_rfh_( + std::make_pair(content::ChildProcessHost::kInvalidUniqueID, + MSG_ROUTING_NONE)) {} + chrome::mojom::SearchBox* GetSearchBox() override; + + private: + content::WebContents* web_contents_; + chrome::mojom::SearchBoxAssociatedPtr search_box_; + + // The proccess ID and routing ID of the last connected main frame. May be + // (ChildProcessHost::kInvalidUniqueID, MSG_ROUTING_NONE) if we haven't + // connected yet. + std::pair<int, int> last_connected_rfh_; + + DISALLOW_COPY_AND_ASSIGN(SearchBoxClientFactoryImpl); +}; + +chrome::mojom::SearchBox* SearchBoxClientFactoryImpl::GetSearchBox() { + content::RenderFrameHost* frame = web_contents_->GetMainFrame(); + auto id = std::make_pair(frame->GetProcess()->GetID(), frame->GetRoutingID()); + // Avoid reconnecting repeatedly to the same RenderFrameHost for performance + // reasons. + if (id != last_connected_rfh_) { + if (IsRenderedInInstantProcess(web_contents_)) { + frame->GetRemoteAssociatedInterfaces()->GetInterface(&search_box_); + } else { + // Renderer is not an instant process. We'll create a connection that + // drops all messages. + mojo::GetIsolatedProxy(&search_box_); + } + last_connected_rfh_ = id; + } + return search_box_.get(); +} + +} // namespace SearchIPCRouter::SearchIPCRouter(content::WebContents* web_contents, Delegate* delegate, @@ -20,7 +79,9 @@ delegate_(delegate), policy_(std::move(policy)), commit_counter_(0), - is_active_tab_(false) { + is_active_tab_(false), + bindings_(web_contents, this), + search_box_client_factory_(new SearchBoxClientFactoryImpl(web_contents)) { DCHECK(web_contents); DCHECK(delegate); DCHECK(policy_.get()); @@ -30,11 +91,11 @@ void SearchIPCRouter::OnNavigationEntryCommitted() { ++commit_counter_; - Send(new ChromeViewMsg_SetPageSequenceNumber(routing_id(), commit_counter_)); + search_box()->SetPageSequenceNumber(commit_counter_); } void SearchIPCRouter::DetermineIfPageSupportsInstant() { - Send(new ChromeViewMsg_DetermineIfPageSupportsInstant(routing_id())); + search_box()->DetermineIfPageSupportsInstant(); } void SearchIPCRouter::SendChromeIdentityCheckResult( @@ -43,15 +104,14 @@ if (!policy_->ShouldProcessChromeIdentityCheck()) return; - Send(new ChromeViewMsg_ChromeIdentityCheckResult(routing_id(), identity, - identity_match)); + search_box()->ChromeIdentityCheckResult(identity, identity_match); } void SearchIPCRouter::SendHistorySyncCheckResult(bool sync_history) { if (!policy_->ShouldProcessHistorySyncCheck()) return; - Send(new ChromeViewMsg_HistorySyncCheckResult(routing_id(), sync_history)); + search_box()->HistorySyncCheckResult(sync_history); } void SearchIPCRouter::SetSuggestionToPrefetch( @@ -59,16 +119,14 @@ if (!policy_->ShouldSendSetSuggestionToPrefetch()) return; - Send(new ChromeViewMsg_SearchBoxSetSuggestionToPrefetch(routing_id(), - suggestion)); + search_box()->SetSuggestionToPrefetch(suggestion); } void SearchIPCRouter::SetInputInProgress(bool input_in_progress) { if (!policy_->ShouldSendSetInputInProgress(is_active_tab_)) return; - Send(new ChromeViewMsg_SearchBoxSetInputInProgress(routing_id(), - input_in_progress)); + search_box()->SetInputInProgress(input_in_progress); } void SearchIPCRouter::OmniboxFocusChanged(OmniboxFocusState state, @@ -76,7 +134,7 @@ if (!policy_->ShouldSendOmniboxFocusChanged()) return; - Send(new ChromeViewMsg_SearchBoxFocusChanged(routing_id(), state, reason)); + search_box()->FocusChanged(state, reason); } void SearchIPCRouter::SendMostVisitedItems( @@ -84,7 +142,7 @@ if (!policy_->ShouldSendMostVisitedItems()) return; - Send(new ChromeViewMsg_SearchBoxMostVisitedItemsChanged(routing_id(), items)); + search_box()->MostVisitedChanged(items); } void SearchIPCRouter::SendThemeBackgroundInfo( @@ -92,7 +150,7 @@ if (!policy_->ShouldSendThemeBackgroundInfo()) return; - Send(new ChromeViewMsg_SearchBoxThemeChanged(routing_id(), theme_info)); + search_box()->ThemeChanged(theme_info); } void SearchIPCRouter::Submit(const base::string16& text, @@ -100,7 +158,7 @@ if (!policy_->ShouldSubmitQuery()) return; - Send(new ChromeViewMsg_SearchBoxSubmit(routing_id(), text, params)); + search_box()->Submit(text, params); } void SearchIPCRouter::OnTabActivated() { @@ -111,52 +169,19 @@ is_active_tab_ = false; } -bool SearchIPCRouter::OnMessageReceived(const IPC::Message& message) { - if (IPC_MESSAGE_CLASS(message) != ChromeMsgStart) - return false; - - Profile* profile = - Profile::FromBrowserContext(web_contents()->GetBrowserContext()); - if (!search::IsRenderedInInstantProcess(web_contents(), profile)) - return false; - - bool handled = true; - IPC_BEGIN_MESSAGE_MAP(SearchIPCRouter, message) - IPC_MESSAGE_HANDLER(ChromeViewHostMsg_InstantSupportDetermined, - OnInstantSupportDetermined) - IPC_MESSAGE_HANDLER(ChromeViewHostMsg_FocusOmnibox, OnFocusOmnibox); - IPC_MESSAGE_HANDLER(ChromeViewHostMsg_SearchBoxDeleteMostVisitedItem, - OnDeleteMostVisitedItem); - IPC_MESSAGE_HANDLER(ChromeViewHostMsg_SearchBoxUndoMostVisitedDeletion, - OnUndoMostVisitedDeletion); - IPC_MESSAGE_HANDLER(ChromeViewHostMsg_SearchBoxUndoAllMostVisitedDeletions, - OnUndoAllMostVisitedDeletions); - IPC_MESSAGE_HANDLER(ChromeViewHostMsg_LogEvent, OnLogEvent); - IPC_MESSAGE_HANDLER(ChromeViewHostMsg_LogMostVisitedImpression, - OnLogMostVisitedImpression); - IPC_MESSAGE_HANDLER(ChromeViewHostMsg_LogMostVisitedNavigation, - OnLogMostVisitedNavigation); - IPC_MESSAGE_HANDLER(ChromeViewHostMsg_PasteAndOpenDropdown, - OnPasteAndOpenDropDown); - IPC_MESSAGE_HANDLER(ChromeViewHostMsg_HistorySyncCheck, - OnHistorySyncCheck); - IPC_MESSAGE_HANDLER(ChromeViewHostMsg_ChromeIdentityCheck, - OnChromeIdentityCheck); - IPC_MESSAGE_UNHANDLED(handled = false) - IPC_END_MESSAGE_MAP() - return handled; -} - -void SearchIPCRouter::OnInstantSupportDetermined(int page_seq_no, - bool instant_support) const { +void SearchIPCRouter::InstantSupportDetermined(int page_seq_no, + bool instant_support) { + if (!IsRenderedInInstantProcess(web_contents())) + return; if (page_seq_no != commit_counter_) return; delegate_->OnInstantSupportDetermined(instant_support); } -void SearchIPCRouter::OnFocusOmnibox(int page_seq_no, - OmniboxFocusState state) const { +void SearchIPCRouter::FocusOmnibox(int page_seq_no, OmniboxFocusState state) { + if (!IsRenderedInInstantProcess(web_contents())) + return; if (page_seq_no != commit_counter_) return; @@ -167,8 +192,10 @@ delegate_->FocusOmnibox(state); } -void SearchIPCRouter::OnDeleteMostVisitedItem(int page_seq_no, - const GURL& url) const { +void SearchIPCRouter::SearchBoxDeleteMostVisitedItem(int page_seq_no, + const GURL& url) { + if (!IsRenderedInInstantProcess(web_contents())) + return; if (page_seq_no != commit_counter_) return; @@ -179,8 +206,10 @@ delegate_->OnDeleteMostVisitedItem(url); } -void SearchIPCRouter::OnUndoMostVisitedDeletion(int page_seq_no, - const GURL& url) const { +void SearchIPCRouter::SearchBoxUndoMostVisitedDeletion(int page_seq_no, + const GURL& url) { + if (!IsRenderedInInstantProcess(web_contents())) + return; if (page_seq_no != commit_counter_) return; @@ -191,7 +220,9 @@ delegate_->OnUndoMostVisitedDeletion(url); } -void SearchIPCRouter::OnUndoAllMostVisitedDeletions(int page_seq_no) const { +void SearchIPCRouter::SearchBoxUndoAllMostVisitedDeletions(int page_seq_no) { + if (!IsRenderedInInstantProcess(web_contents())) + return; if (page_seq_no != commit_counter_) return; @@ -202,9 +233,11 @@ delegate_->OnUndoAllMostVisitedDeletions(); } -void SearchIPCRouter::OnLogEvent(int page_seq_no, - NTPLoggingEventType event, - base::TimeDelta time) const { +void SearchIPCRouter::LogEvent(int page_seq_no, + NTPLoggingEventType event, + base::TimeDelta time) { + if (!IsRenderedInInstantProcess(web_contents())) + return; if (page_seq_no != commit_counter_) return; @@ -215,10 +248,12 @@ delegate_->OnLogEvent(event, time); } -void SearchIPCRouter::OnLogMostVisitedImpression( +void SearchIPCRouter::LogMostVisitedImpression( int page_seq_no, int position, - ntp_tiles::NTPTileSource tile_source) const { + ntp_tiles::NTPTileSource tile_source) { + if (!IsRenderedInInstantProcess(web_contents())) + return; if (page_seq_no != commit_counter_) return; @@ -230,10 +265,12 @@ delegate_->OnLogMostVisitedImpression(position, tile_source); } -void SearchIPCRouter::OnLogMostVisitedNavigation( +void SearchIPCRouter::LogMostVisitedNavigation( int page_seq_no, int position, - ntp_tiles::NTPTileSource tile_source) const { + ntp_tiles::NTPTileSource tile_source) { + if (!IsRenderedInInstantProcess(web_contents())) + return; if (page_seq_no != commit_counter_) return; @@ -245,8 +282,10 @@ delegate_->OnLogMostVisitedNavigation(position, tile_source); } -void SearchIPCRouter::OnPasteAndOpenDropDown(int page_seq_no, - const base::string16& text) const { +void SearchIPCRouter::PasteAndOpenDropdown(int page_seq_no, + const base::string16& text) { + if (!IsRenderedInInstantProcess(web_contents())) + return; if (page_seq_no != commit_counter_) return; @@ -257,9 +296,10 @@ delegate_->PasteIntoOmnibox(text); } -void SearchIPCRouter::OnChromeIdentityCheck( - int page_seq_no, - const base::string16& identity) const { +void SearchIPCRouter::ChromeIdentityCheck(int page_seq_no, + const base::string16& identity) { + if (!IsRenderedInInstantProcess(web_contents())) + return; if (page_seq_no != commit_counter_) return; @@ -270,7 +310,9 @@ delegate_->OnChromeIdentityCheck(identity); } -void SearchIPCRouter::OnHistorySyncCheck(int page_seq_no) const { +void SearchIPCRouter::HistorySyncCheck(int page_seq_no) { + if (!IsRenderedInInstantProcess(web_contents())) + return; if (page_seq_no != commit_counter_) return;
diff --git a/chrome/browser/ui/search/search_ipc_router.h b/chrome/browser/ui/search/search_ipc_router.h index b878018..43398249 100644 --- a/chrome/browser/ui/search/search_ipc_router.h +++ b/chrome/browser/ui/search/search_ipc_router.h
@@ -11,10 +11,12 @@ #include "base/gtest_prod_util.h" #include "base/macros.h" #include "base/time/time.h" +#include "chrome/common/instant.mojom.h" #include "chrome/common/search/instant_types.h" #include "chrome/common/search/ntp_logging_events.h" #include "components/ntp_tiles/ntp_tile_source.h" #include "components/omnibox/common/omnibox_focus_state.h" +#include "content/public/browser/web_contents_binding_set.h" #include "content/public/browser/web_contents_observer.h" #include "ui/base/window_open_disposition.h" @@ -28,7 +30,8 @@ // SearchIPCRouter is responsible for receiving and sending IPC messages between // the browser and the Instant page. -class SearchIPCRouter : public content::WebContentsObserver { +class SearchIPCRouter : public content::WebContentsObserver, + public chrome::mojom::Instant { public: // SearchIPCRouter calls its delegate in response to messages received from // the page. @@ -109,6 +112,19 @@ virtual bool ShouldSubmitQuery() = 0; }; + // Creates chrome::mojom::SearchBox connections on request. + class SearchBoxClientFactory { + public: + SearchBoxClientFactory() = default; + virtual ~SearchBoxClientFactory() = default; + + // The returned pointer is owned by the factory. + virtual chrome::mojom::SearchBox* GetSearchBox() = 0; + + private: + DISALLOW_COPY_AND_ASSIGN(SearchBoxClientFactory); + }; + SearchIPCRouter(content::WebContents* web_contents, Delegate* delegate, std::unique_ptr<Policy> policy); @@ -155,6 +171,35 @@ // Called when the tab corresponding to |this| instance is deactivated. void OnTabDeactivated(); + // chrome::mojom::Instant: + void InstantSupportDetermined(int page_seq_no, + bool supports_instant) override; + void FocusOmnibox(int page_id, OmniboxFocusState state) override; + void SearchBoxDeleteMostVisitedItem(int page_seq_no, + const GURL& url) override; + void SearchBoxUndoMostVisitedDeletion(int page_seq_no, + const GURL& url) override; + void SearchBoxUndoAllMostVisitedDeletions(int page_seq_no) override; + void LogEvent(int page_seq_no, + NTPLoggingEventType event, + base::TimeDelta time) override; + void LogMostVisitedImpression(int page_seq_no, + int position, + ntp_tiles::NTPTileSource tile_source) override; + void LogMostVisitedNavigation(int page_seq_no, + int position, + ntp_tiles::NTPTileSource tile_source) override; + void PasteAndOpenDropdown(int page_seq_no, + const base::string16& text) override; + void ChromeIdentityCheck(int page_seq_no, + const base::string16& identity) override; + void HistorySyncCheck(int page_seq_no) override; + + void set_search_box_client_factory_for_testing( + std::unique_ptr<SearchBoxClientFactory> factory) { + search_box_client_factory_ = std::move(factory); + } + private: friend class SearchIPCRouterPolicyTest; friend class SearchIPCRouterTest; @@ -168,29 +213,6 @@ IgnoreMessageIfThePageIsNotActive); FRIEND_TEST_ALL_PREFIXES(SearchIPCRouterTest, HandleTabChangedEvents); - // Overridden from contents::WebContentsObserver: - bool OnMessageReceived(const IPC::Message& message) override; - - void OnInstantSupportDetermined(int page_seq_no, bool supports_instant) const; - void OnFocusOmnibox(int page_id, OmniboxFocusState state) const; - void OnDeleteMostVisitedItem(int page_seq_no, const GURL& url) const; - void OnUndoMostVisitedDeletion(int page_seq_no, const GURL& url) const; - void OnUndoAllMostVisitedDeletions(int page_seq_no) const; - void OnLogEvent(int page_seq_no, - NTPLoggingEventType event, - base::TimeDelta time) const; - void OnLogMostVisitedImpression(int page_seq_no, - int position, - ntp_tiles::NTPTileSource tile_source) const; - void OnLogMostVisitedNavigation(int page_seq_no, - int position, - ntp_tiles::NTPTileSource tile_source) const; - void OnPasteAndOpenDropDown(int page_seq_no, - const base::string16& text) const; - void OnChromeIdentityCheck(int page_seq_no, - const base::string16& identity) const; - void OnHistorySyncCheck(int page_seq_no) const; - // Used by unit tests to set a fake delegate. void set_delegate_for_testing(Delegate* delegate); @@ -203,6 +225,10 @@ // Used by unit tests. int page_seq_no_for_testing() const { return commit_counter_; } + chrome::mojom::SearchBox* search_box() { + return search_box_client_factory_->GetSearchBox(); + } + Delegate* delegate_; std::unique_ptr<Policy> policy_; @@ -213,6 +239,10 @@ // Set to true, when the tab corresponding to |this| instance is active. bool is_active_tab_; + content::WebContentsFrameBindingSet<chrome::mojom::Instant> bindings_; + + std::unique_ptr<SearchBoxClientFactory> search_box_client_factory_; + DISALLOW_COPY_AND_ASSIGN(SearchIPCRouter); };
diff --git a/chrome/browser/ui/search/search_ipc_router_policy_unittest.cc b/chrome/browser/ui/search/search_ipc_router_policy_unittest.cc index 8d52521a..ba6b7ef 100644 --- a/chrome/browser/ui/search/search_ipc_router_policy_unittest.cc +++ b/chrome/browser/ui/search/search_ipc_router_policy_unittest.cc
@@ -31,7 +31,7 @@ SearchTabHelper* search_tab_helper = SearchTabHelper::FromWebContents(web_contents()); EXPECT_NE(static_cast<SearchTabHelper*>(NULL), search_tab_helper); - return search_tab_helper->ipc_router().policy_for_testing(); + return search_tab_helper->ipc_router_for_testing().policy_for_testing(); } void SetIncognitoProfile() {
diff --git a/chrome/browser/ui/search/search_ipc_router_unittest.cc b/chrome/browser/ui/search/search_ipc_router_unittest.cc index 6b183c0d..bb7db73 100644 --- a/chrome/browser/ui/search/search_ipc_router_unittest.cc +++ b/chrome/browser/ui/search/search_ipc_router_unittest.cc
@@ -24,6 +24,7 @@ #include "chrome/common/chrome_switches.h" #include "chrome/common/render_messages.h" #include "chrome/common/search/instant_types.h" +#include "chrome/common/search/mock_searchbox.h" #include "chrome/common/search/ntp_logging_events.h" #include "chrome/common/url_constants.h" #include "chrome/test/base/browser_with_test_window_test.h" @@ -42,6 +43,9 @@ #include "ui/base/window_open_disposition.h" #include "url/gurl.h" +using testing::_; +using testing::Return; + namespace { class MockSearchIPCRouterDelegate : public SearchIPCRouter::Delegate { @@ -84,6 +88,12 @@ MOCK_METHOD0(ShouldSubmitQuery, bool()); }; +class MockSearchBoxClientFactory + : public SearchIPCRouter::SearchBoxClientFactory { + public: + MOCK_METHOD0(GetSearchBox, chrome::mojom::SearchBox*(void)); +}; + } // namespace class SearchIPCRouterTest : public BrowserWithTestWindowTest { @@ -113,18 +123,12 @@ TemplateURL* template_url = template_url_service->Add(base::MakeUnique<TemplateURL>(data)); template_url_service->SetUserSelectedDefaultSearchProvider(template_url); - process()->sink().ClearMessages(); } content::WebContents* web_contents() { return browser()->tab_strip_model()->GetActiveWebContents(); } - content::MockRenderProcessHost* process() { - return static_cast<content::MockRenderProcessHost*>( - web_contents()->GetMainFrame()->GetProcess()); - } - SearchTabHelper* GetSearchTabHelper( content::WebContents* web_contents) { EXPECT_NE(static_cast<content::WebContents*>(NULL), web_contents); @@ -136,13 +140,14 @@ ASSERT_NE(static_cast<content::WebContents*>(NULL), contents); SearchTabHelper* search_tab_helper = GetSearchTabHelper(contents); ASSERT_NE(static_cast<SearchTabHelper*>(NULL), search_tab_helper); - search_tab_helper->ipc_router().set_delegate_for_testing(mock_delegate()); - search_tab_helper->ipc_router().set_policy_for_testing( + search_tab_helper->ipc_router_for_testing().set_delegate_for_testing( + mock_delegate()); + search_tab_helper->ipc_router_for_testing().set_policy_for_testing( base::WrapUnique(new MockSearchIPCRouterPolicy)); - } - - bool MessageWasSent(uint32_t id) { - return process()->sink().GetFirstMessageMatching(id) != NULL; + auto factory = base::MakeUnique<MockSearchBoxClientFactory>(); + ON_CALL(*factory, GetSearchBox()).WillByDefault(Return(&mock_search_box_)); + GetSearchIPCRouter().set_search_box_client_factory_for_testing( + std::move(factory)); } MockSearchIPCRouterDelegate* mock_delegate() { return &delegate_; } @@ -153,35 +158,29 @@ SearchTabHelper* search_tab_helper = GetSearchTabHelper(contents); EXPECT_NE(static_cast<SearchTabHelper*>(NULL), search_tab_helper); return static_cast<MockSearchIPCRouterPolicy*>( - search_tab_helper->ipc_router().policy_for_testing()); + search_tab_helper->ipc_router_for_testing().policy_for_testing()); } SearchIPCRouter& GetSearchIPCRouter() { - return GetSearchTabHelper(web_contents())->ipc_router(); + return GetSearchTabHelper(web_contents())->ipc_router_for_testing(); } int GetSearchIPCRouterSeqNo() { return GetSearchIPCRouter().page_seq_no_for_testing(); } - void OnMessageReceived(const IPC::Message& message) { - bool should_handle_message = - search::IsRenderedInInstantProcess(web_contents(), profile()); - bool handled = GetSearchIPCRouter().OnMessageReceived(message); - ASSERT_EQ(should_handle_message, handled); - } - - bool OnSpuriousMessageReceived(const IPC::Message& message) { - return GetSearchIPCRouter().OnMessageReceived(message); - } - bool IsActiveTab(content::WebContents* contents) { - return GetSearchTabHelper(contents)->ipc_router().is_active_tab_; + return GetSearchTabHelper(contents) + ->ipc_router_for_testing() + .is_active_tab_; } + MockSearchBox* mock_search_box() { return &mock_search_box_; } + private: MockSearchIPCRouterDelegate delegate_; base::FieldTrialList field_trial_list_; + MockSearchBox mock_search_box_; }; TEST_F(SearchIPCRouterTest, IgnoreMessagesFromNonInstantRenderers) { @@ -195,9 +194,8 @@ MockSearchIPCRouterPolicy* policy = GetSearchIPCRouterPolicy(); EXPECT_CALL(*policy, ShouldProcessFocusOmnibox(is_active_tab)).Times(0); - OnMessageReceived(ChromeViewHostMsg_FocusOmnibox( - contents->GetRenderViewHost()->GetRoutingID(), GetSearchIPCRouterSeqNo(), - OMNIBOX_FOCUS_VISIBLE)); + GetSearchIPCRouter().FocusOmnibox(GetSearchIPCRouterSeqNo(), + OMNIBOX_FOCUS_VISIBLE); } TEST_F(SearchIPCRouterTest, ProcessFocusOmniboxMsg) { @@ -212,9 +210,8 @@ EXPECT_CALL(*policy, ShouldProcessFocusOmnibox(is_active_tab)).Times(1) .WillOnce(testing::Return(true)); - OnMessageReceived(ChromeViewHostMsg_FocusOmnibox( - contents->GetRenderViewHost()->GetRoutingID(), GetSearchIPCRouterSeqNo(), - OMNIBOX_FOCUS_VISIBLE)); + GetSearchIPCRouter().FocusOmnibox(GetSearchIPCRouterSeqNo(), + OMNIBOX_FOCUS_VISIBLE); } TEST_F(SearchIPCRouterTest, IgnoreFocusOmniboxMsg) { @@ -229,9 +226,8 @@ EXPECT_CALL(*policy, ShouldProcessFocusOmnibox(is_active_tab)).Times(1) .WillOnce(testing::Return(false)); - OnMessageReceived(ChromeViewHostMsg_FocusOmnibox( - contents->GetRenderViewHost()->GetRoutingID(), GetSearchIPCRouterSeqNo(), - OMNIBOX_FOCUS_VISIBLE)); + GetSearchIPCRouter().FocusOmnibox(GetSearchIPCRouterSeqNo(), + OMNIBOX_FOCUS_VISIBLE); } TEST_F(SearchIPCRouterTest, HandleTabChangedEvents) { @@ -264,10 +260,8 @@ EXPECT_CALL(*policy, ShouldProcessLogEvent()).Times(1) .WillOnce(testing::Return(true)); - content::WebContents* contents = web_contents(); - OnMessageReceived(ChromeViewHostMsg_LogEvent( - contents->GetRenderViewHost()->GetRoutingID(), GetSearchIPCRouterSeqNo(), - NTP_ALL_TILES_LOADED, delta)); + GetSearchIPCRouter().LogEvent(GetSearchIPCRouterSeqNo(), NTP_ALL_TILES_LOADED, + delta); } TEST_F(SearchIPCRouterTest, IgnoreLogEventMsg) { @@ -280,10 +274,8 @@ EXPECT_CALL(*policy, ShouldProcessLogEvent()).Times(1) .WillOnce(testing::Return(false)); - content::WebContents* contents = web_contents(); - OnMessageReceived(ChromeViewHostMsg_LogEvent( - contents->GetRenderViewHost()->GetRoutingID(), GetSearchIPCRouterSeqNo(), - NTP_ALL_TILES_LOADED, delta)); + GetSearchIPCRouter().LogEvent(GetSearchIPCRouterSeqNo(), NTP_ALL_TILES_LOADED, + delta); } TEST_F(SearchIPCRouterTest, ProcessLogMostVisitedImpressionMsg) { @@ -297,10 +289,9 @@ EXPECT_CALL(*policy, ShouldProcessLogEvent()).Times(1) .WillOnce(testing::Return(true)); - content::WebContents* contents = web_contents(); - OnMessageReceived(ChromeViewHostMsg_LogMostVisitedImpression( - contents->GetRenderViewHost()->GetRoutingID(), GetSearchIPCRouterSeqNo(), - 3, ntp_tiles::NTPTileSource::SUGGESTIONS_SERVICE)); + GetSearchIPCRouter().LogMostVisitedImpression( + GetSearchIPCRouterSeqNo(), 3, + ntp_tiles::NTPTileSource::SUGGESTIONS_SERVICE); } TEST_F(SearchIPCRouterTest, ProcessLogMostVisitedNavigationMsg) { @@ -314,10 +305,9 @@ EXPECT_CALL(*policy, ShouldProcessLogEvent()).Times(1) .WillOnce(testing::Return(true)); - content::WebContents* contents = web_contents(); - OnMessageReceived(ChromeViewHostMsg_LogMostVisitedNavigation( - contents->GetRenderViewHost()->GetRoutingID(), GetSearchIPCRouterSeqNo(), - 3, ntp_tiles::NTPTileSource::SUGGESTIONS_SERVICE)); + GetSearchIPCRouter().LogMostVisitedNavigation( + GetSearchIPCRouterSeqNo(), 3, + ntp_tiles::NTPTileSource::SUGGESTIONS_SERVICE); } TEST_F(SearchIPCRouterTest, ProcessChromeIdentityCheckMsg) { @@ -329,10 +319,8 @@ EXPECT_CALL(*policy, ShouldProcessChromeIdentityCheck()).Times(1) .WillOnce(testing::Return(true)); - content::WebContents* contents = web_contents(); - OnMessageReceived(ChromeViewHostMsg_ChromeIdentityCheck( - contents->GetRenderViewHost()->GetRoutingID(), GetSearchIPCRouterSeqNo(), - test_identity)); + GetSearchIPCRouter().ChromeIdentityCheck(GetSearchIPCRouterSeqNo(), + test_identity); } TEST_F(SearchIPCRouterTest, IgnoreChromeIdentityCheckMsg) { @@ -345,10 +333,8 @@ EXPECT_CALL(*policy, ShouldProcessChromeIdentityCheck()).Times(1) .WillOnce(testing::Return(false)); - content::WebContents* contents = web_contents(); - OnMessageReceived(ChromeViewHostMsg_ChromeIdentityCheck( - contents->GetRenderViewHost()->GetRoutingID(), GetSearchIPCRouterSeqNo(), - test_identity)); + GetSearchIPCRouter().ChromeIdentityCheck(GetSearchIPCRouterSeqNo(), + test_identity); } TEST_F(SearchIPCRouterTest, ProcessHistorySyncCheckMsg) { @@ -359,10 +345,7 @@ EXPECT_CALL(*policy, ShouldProcessHistorySyncCheck()).Times(1) .WillOnce(testing::Return(true)); - content::WebContents* contents = web_contents(); - OnMessageReceived(ChromeViewHostMsg_HistorySyncCheck( - contents->GetRenderViewHost()->GetRoutingID(), - GetSearchIPCRouterSeqNo())); + GetSearchIPCRouter().HistorySyncCheck(GetSearchIPCRouterSeqNo()); } TEST_F(SearchIPCRouterTest, IgnoreHistorySyncCheckMsg) { @@ -374,10 +357,7 @@ EXPECT_CALL(*policy, ShouldProcessHistorySyncCheck()).Times(1) .WillOnce(testing::Return(false)); - content::WebContents* contents = web_contents(); - OnMessageReceived(ChromeViewHostMsg_HistorySyncCheck( - contents->GetRenderViewHost()->GetRoutingID(), - GetSearchIPCRouterSeqNo())); + GetSearchIPCRouter().HistorySyncCheck(GetSearchIPCRouterSeqNo()); } TEST_F(SearchIPCRouterTest, ProcessDeleteMostVisitedItemMsg) { @@ -389,10 +369,8 @@ EXPECT_CALL(*policy, ShouldProcessDeleteMostVisitedItem()).Times(1) .WillOnce(testing::Return(true)); - content::WebContents* contents = web_contents(); - OnMessageReceived(ChromeViewHostMsg_SearchBoxDeleteMostVisitedItem( - contents->GetRenderViewHost()->GetRoutingID(), GetSearchIPCRouterSeqNo(), - item_url)); + GetSearchIPCRouter().SearchBoxDeleteMostVisitedItem(GetSearchIPCRouterSeqNo(), + item_url); } TEST_F(SearchIPCRouterTest, IgnoreDeleteMostVisitedItemMsg) { @@ -404,10 +382,8 @@ EXPECT_CALL(*policy, ShouldProcessDeleteMostVisitedItem()).Times(1) .WillOnce(testing::Return(false)); - content::WebContents* contents = web_contents(); - OnMessageReceived(ChromeViewHostMsg_SearchBoxDeleteMostVisitedItem( - contents->GetRenderViewHost()->GetRoutingID(), GetSearchIPCRouterSeqNo(), - item_url)); + GetSearchIPCRouter().SearchBoxDeleteMostVisitedItem(GetSearchIPCRouterSeqNo(), + item_url); } TEST_F(SearchIPCRouterTest, ProcessUndoMostVisitedDeletionMsg) { @@ -419,10 +395,8 @@ EXPECT_CALL(*policy, ShouldProcessUndoMostVisitedDeletion()).Times(1) .WillOnce(testing::Return(true)); - content::WebContents* contents = web_contents(); - OnMessageReceived(ChromeViewHostMsg_SearchBoxUndoMostVisitedDeletion( - contents->GetRenderViewHost()->GetRoutingID(), GetSearchIPCRouterSeqNo(), - item_url)); + GetSearchIPCRouter().SearchBoxUndoMostVisitedDeletion( + GetSearchIPCRouterSeqNo(), item_url); } TEST_F(SearchIPCRouterTest, IgnoreUndoMostVisitedDeletionMsg) { @@ -434,10 +408,8 @@ EXPECT_CALL(*policy, ShouldProcessUndoMostVisitedDeletion()).Times(1) .WillOnce(testing::Return(false)); - content::WebContents* contents = web_contents(); - OnMessageReceived(ChromeViewHostMsg_SearchBoxUndoMostVisitedDeletion( - contents->GetRenderViewHost()->GetRoutingID(), GetSearchIPCRouterSeqNo(), - item_url)); + GetSearchIPCRouter().SearchBoxUndoMostVisitedDeletion( + GetSearchIPCRouterSeqNo(), item_url); } TEST_F(SearchIPCRouterTest, ProcessUndoAllMostVisitedDeletionsMsg) { @@ -448,10 +420,8 @@ EXPECT_CALL(*policy, ShouldProcessUndoAllMostVisitedDeletions()).Times(1) .WillOnce(testing::Return(true)); - content::WebContents* contents = web_contents(); - OnMessageReceived(ChromeViewHostMsg_SearchBoxUndoAllMostVisitedDeletions( - contents->GetRenderViewHost()->GetRoutingID(), - GetSearchIPCRouterSeqNo())); + GetSearchIPCRouter().SearchBoxUndoAllMostVisitedDeletions( + GetSearchIPCRouterSeqNo()); } TEST_F(SearchIPCRouterTest, IgnoreUndoAllMostVisitedDeletionsMsg) { @@ -462,10 +432,8 @@ EXPECT_CALL(*policy, ShouldProcessUndoAllMostVisitedDeletions()).Times(1) .WillOnce(testing::Return(false)); - content::WebContents* contents = web_contents(); - OnMessageReceived(ChromeViewHostMsg_SearchBoxUndoAllMostVisitedDeletions( - contents->GetRenderViewHost()->GetRoutingID(), - GetSearchIPCRouterSeqNo())); + GetSearchIPCRouter().SearchBoxUndoAllMostVisitedDeletions( + GetSearchIPCRouterSeqNo()); } TEST_F(SearchIPCRouterTest, IgnoreMessageIfThePageIsNotActive) { @@ -483,38 +451,30 @@ EXPECT_CALL(*mock_delegate(), OnDeleteMostVisitedItem(item_url)).Times(0); EXPECT_CALL(*policy, ShouldProcessDeleteMostVisitedItem()).Times(0); - OnMessageReceived(ChromeViewHostMsg_SearchBoxDeleteMostVisitedItem( - contents->GetRenderViewHost()->GetRoutingID(), page_seq_no, item_url)); + GetSearchIPCRouter().SearchBoxDeleteMostVisitedItem(page_seq_no, item_url); EXPECT_CALL(*mock_delegate(), OnUndoMostVisitedDeletion(item_url)).Times(0); EXPECT_CALL(*policy, ShouldProcessUndoMostVisitedDeletion()).Times(0); - OnMessageReceived(ChromeViewHostMsg_SearchBoxUndoMostVisitedDeletion( - contents->GetRenderViewHost()->GetRoutingID(), page_seq_no, item_url)); + GetSearchIPCRouter().SearchBoxUndoMostVisitedDeletion(page_seq_no, item_url); EXPECT_CALL(*mock_delegate(), OnUndoAllMostVisitedDeletions()).Times(0); EXPECT_CALL(*policy, ShouldProcessUndoAllMostVisitedDeletions()).Times(0); - OnMessageReceived(ChromeViewHostMsg_SearchBoxUndoAllMostVisitedDeletions( - contents->GetRenderViewHost()->GetRoutingID(), page_seq_no)); + GetSearchIPCRouter().SearchBoxUndoAllMostVisitedDeletions(page_seq_no); EXPECT_CALL(*mock_delegate(), FocusOmnibox(OMNIBOX_FOCUS_VISIBLE)).Times(0); EXPECT_CALL(*policy, ShouldProcessFocusOmnibox(is_active_tab)).Times(0); - OnMessageReceived(ChromeViewHostMsg_FocusOmnibox( - contents->GetRenderViewHost()->GetRoutingID(), page_seq_no, - OMNIBOX_FOCUS_VISIBLE)); + GetSearchIPCRouter().FocusOmnibox(page_seq_no, OMNIBOX_FOCUS_VISIBLE); base::TimeDelta delta = base::TimeDelta::FromMilliseconds(123); EXPECT_CALL(*mock_delegate(), OnLogEvent(NTP_ALL_TILES_LOADED, delta)) .Times(0); EXPECT_CALL(*policy, ShouldProcessLogEvent()).Times(0); - OnMessageReceived( - ChromeViewHostMsg_LogEvent(contents->GetRenderViewHost()->GetRoutingID(), - page_seq_no, NTP_ALL_TILES_LOADED, delta)); + GetSearchIPCRouter().LogEvent(page_seq_no, NTP_ALL_TILES_LOADED, delta); base::string16 text; EXPECT_CALL(*mock_delegate(), PasteIntoOmnibox(text)).Times(0); EXPECT_CALL(*policy, ShouldProcessPasteIntoOmnibox(is_active_tab)).Times(0); - OnMessageReceived(ChromeViewHostMsg_PasteAndOpenDropdown( - contents->GetRenderViewHost()->GetRoutingID(), page_seq_no, text)); + GetSearchIPCRouter().PasteAndOpenDropdown(page_seq_no, text); } TEST_F(SearchIPCRouterTest, ProcessPasteAndOpenDropdownMsg) { @@ -530,9 +490,7 @@ EXPECT_CALL(*mock_delegate(), PasteIntoOmnibox(text)).Times(1); EXPECT_CALL(*policy, ShouldProcessPasteIntoOmnibox(is_active_tab)).Times(1) .WillOnce(testing::Return(true)); - OnMessageReceived(ChromeViewHostMsg_PasteAndOpenDropdown( - contents->GetRenderViewHost()->GetRoutingID(), GetSearchIPCRouterSeqNo(), - text)); + GetSearchIPCRouter().PasteAndOpenDropdown(GetSearchIPCRouterSeqNo(), text); } TEST_F(SearchIPCRouterTest, IgnorePasteAndOpenDropdownMsg) { @@ -549,9 +507,7 @@ EXPECT_CALL(*policy, ShouldProcessPasteIntoOmnibox(is_active_tab)).Times(1) .WillOnce(testing::Return(false)); - OnMessageReceived(ChromeViewHostMsg_PasteAndOpenDropdown( - contents->GetRenderViewHost()->GetRoutingID(), GetSearchIPCRouterSeqNo(), - text)); + GetSearchIPCRouter().PasteAndOpenDropdown(GetSearchIPCRouterSeqNo(), text); } TEST_F(SearchIPCRouterTest, SendSetSuggestionToPrefetch) { @@ -561,11 +517,9 @@ EXPECT_CALL(*policy, ShouldSendSetSuggestionToPrefetch()).Times(1) .WillOnce(testing::Return(true)); - process()->sink().ClearMessages(); content::WebContents* contents = web_contents(); + EXPECT_CALL(*mock_search_box(), SetSuggestionToPrefetch(_)); GetSearchTabHelper(contents)->SetSuggestionToPrefetch(InstantSuggestion()); - EXPECT_TRUE(MessageWasSent( - ChromeViewMsg_SearchBoxSetSuggestionToPrefetch::ID)); } TEST_F(SearchIPCRouterTest, DoNotSendSetSuggestionToPrefetch) { @@ -575,11 +529,9 @@ EXPECT_CALL(*policy, ShouldSendSetSuggestionToPrefetch()).Times(1) .WillOnce(testing::Return(false)); - process()->sink().ClearMessages(); content::WebContents* contents = web_contents(); + EXPECT_CALL(*mock_search_box(), SetSuggestionToPrefetch(_)).Times(0); GetSearchTabHelper(contents)->SetSuggestionToPrefetch(InstantSuggestion()); - EXPECT_FALSE(MessageWasSent( - ChromeViewMsg_SearchBoxSetSuggestionToPrefetch::ID)); } TEST_F(SearchIPCRouterTest, SendOmniboxFocusChange) { @@ -589,10 +541,9 @@ EXPECT_CALL(*policy, ShouldSendOmniboxFocusChanged()).Times(1) .WillOnce(testing::Return(true)); - process()->sink().ClearMessages(); + EXPECT_CALL(*mock_search_box(), FocusChanged(_, _)); GetSearchIPCRouter().OmniboxFocusChanged(OMNIBOX_FOCUS_NONE, OMNIBOX_FOCUS_CHANGE_EXPLICIT); - EXPECT_TRUE(MessageWasSent(ChromeViewMsg_SearchBoxFocusChanged::ID)); } TEST_F(SearchIPCRouterTest, DoNotSendOmniboxFocusChange) { @@ -602,10 +553,9 @@ EXPECT_CALL(*policy, ShouldSendOmniboxFocusChanged()).Times(1) .WillOnce(testing::Return(false)); - process()->sink().ClearMessages(); + EXPECT_CALL(*mock_search_box(), FocusChanged(_, _)).Times(0); GetSearchIPCRouter().OmniboxFocusChanged(OMNIBOX_FOCUS_NONE, OMNIBOX_FOCUS_CHANGE_EXPLICIT); - EXPECT_FALSE(MessageWasSent(ChromeViewMsg_SearchBoxFocusChanged::ID)); } TEST_F(SearchIPCRouterTest, SendSetInputInProgress) { @@ -615,9 +565,8 @@ EXPECT_CALL(*policy, ShouldSendSetInputInProgress(true)).Times(1) .WillOnce(testing::Return(true)); - process()->sink().ClearMessages(); + EXPECT_CALL(*mock_search_box(), SetInputInProgress(_)); GetSearchIPCRouter().SetInputInProgress(true); - EXPECT_TRUE(MessageWasSent(ChromeViewMsg_SearchBoxSetInputInProgress::ID)); } TEST_F(SearchIPCRouterTest, DoNotSendSetInputInProgress) { @@ -627,9 +576,8 @@ EXPECT_CALL(*policy, ShouldSendSetInputInProgress(true)).Times(1) .WillOnce(testing::Return(false)); - process()->sink().ClearMessages(); + EXPECT_CALL(*mock_search_box(), SetInputInProgress(_)).Times(0); GetSearchIPCRouter().SetInputInProgress(true); - EXPECT_FALSE(MessageWasSent(ChromeViewMsg_SearchBoxSetInputInProgress::ID)); } TEST_F(SearchIPCRouterTest, SendMostVisitedItemsMsg) { @@ -639,11 +587,9 @@ EXPECT_CALL(*policy, ShouldSendMostVisitedItems()).Times(1) .WillOnce(testing::Return(true)); - process()->sink().ClearMessages(); + EXPECT_CALL(*mock_search_box(), MostVisitedChanged(_)); GetSearchIPCRouter().SendMostVisitedItems( std::vector<InstantMostVisitedItem>()); - EXPECT_TRUE(MessageWasSent( - ChromeViewMsg_SearchBoxMostVisitedItemsChanged::ID)); } TEST_F(SearchIPCRouterTest, DoNotSendMostVisitedItemsMsg) { @@ -653,11 +599,9 @@ EXPECT_CALL(*policy, ShouldSendMostVisitedItems()).Times(1) .WillOnce(testing::Return(false)); - process()->sink().ClearMessages(); + EXPECT_CALL(*mock_search_box(), MostVisitedChanged(_)).Times(0); GetSearchIPCRouter().SendMostVisitedItems( std::vector<InstantMostVisitedItem>()); - EXPECT_FALSE(MessageWasSent( - ChromeViewMsg_SearchBoxMostVisitedItemsChanged::ID)); } TEST_F(SearchIPCRouterTest, SendThemeBackgroundInfoMsg) { @@ -667,9 +611,8 @@ EXPECT_CALL(*policy, ShouldSendThemeBackgroundInfo()).Times(1) .WillOnce(testing::Return(true)); - process()->sink().ClearMessages(); + EXPECT_CALL(*mock_search_box(), ThemeChanged(_)); GetSearchIPCRouter().SendThemeBackgroundInfo(ThemeBackgroundInfo()); - EXPECT_TRUE(MessageWasSent(ChromeViewMsg_SearchBoxThemeChanged::ID)); } TEST_F(SearchIPCRouterTest, DoNotSendThemeBackgroundInfoMsg) { @@ -679,9 +622,8 @@ EXPECT_CALL(*policy, ShouldSendThemeBackgroundInfo()).Times(1) .WillOnce(testing::Return(false)); - process()->sink().ClearMessages(); + EXPECT_CALL(*mock_search_box(), ThemeChanged(_)).Times(0); GetSearchIPCRouter().SendThemeBackgroundInfo(ThemeBackgroundInfo()); - EXPECT_FALSE(MessageWasSent(ChromeViewMsg_SearchBoxThemeChanged::ID)); } TEST_F(SearchIPCRouterTest, SendSubmitMsg) { @@ -691,9 +633,8 @@ EXPECT_CALL(*policy, ShouldSubmitQuery()).Times(1) .WillOnce(testing::Return(true)); - process()->sink().ClearMessages(); + EXPECT_CALL(*mock_search_box(), Submit(_, _)); GetSearchIPCRouter().Submit(base::string16(), EmbeddedSearchRequestParams()); - EXPECT_TRUE(MessageWasSent(ChromeViewMsg_SearchBoxSubmit::ID)); } TEST_F(SearchIPCRouterTest, DoNotSendSubmitMsg) { @@ -703,22 +644,6 @@ EXPECT_CALL(*policy, ShouldSubmitQuery()).Times(1) .WillOnce(testing::Return(false)); - process()->sink().ClearMessages(); + EXPECT_CALL(*mock_search_box(), Submit(_, _)).Times(0); GetSearchIPCRouter().Submit(base::string16(), EmbeddedSearchRequestParams()); - EXPECT_FALSE(MessageWasSent(ChromeViewMsg_SearchBoxSubmit::ID)); -} - -TEST_F(SearchIPCRouterTest, SpuriousMessageTypesIgnored) { - NavigateAndCommitActiveTab(GURL("chrome-search://foo/bar")); - SetupMockDelegateAndPolicy(); - const int routing_id = web_contents()->GetRenderViewHost()->GetRoutingID(); - - // Construct a series of synthetic messages for each valid IPC message type, - // ensuring the router ignores them all. - for (int i = 0; i < LastIPCMsgStart; ++i) { - const int message_id = i << 16; - ASSERT_EQ(IPC_MESSAGE_ID_CLASS(message_id), i); - IPC::Message msg(routing_id, message_id, IPC::Message::PRIORITY_LOW); - EXPECT_FALSE(OnSpuriousMessageReceived(msg)) << i; - } }
diff --git a/chrome/browser/ui/search/search_tab_helper.h b/chrome/browser/ui/search/search_tab_helper.h index 35c9ee7..96c5add 100644 --- a/chrome/browser/ui/search/search_tab_helper.h +++ b/chrome/browser/ui/search/search_tab_helper.h
@@ -83,6 +83,8 @@ void set_delegate(SearchTabHelperDelegate* delegate) { delegate_ = delegate; } + SearchIPCRouter& ipc_router_for_testing() { return ipc_router_; } + private: friend class content::WebContentsUserData<SearchTabHelper>; friend class SearchIPCRouterPolicyTest; @@ -167,9 +169,6 @@ // received. void DetermineIfPageSupportsInstant(); - // Used by unit tests. - SearchIPCRouter& ipc_router() { return ipc_router_; } - Profile* profile() const; // Returns whether input is in progress, i.e. if the omnibox has focus and the
diff --git a/chrome/browser/ui/search/search_tab_helper_unittest.cc b/chrome/browser/ui/search/search_tab_helper_unittest.cc index 231f650..6c6cd0d 100644 --- a/chrome/browser/ui/search/search_tab_helper_unittest.cc +++ b/chrome/browser/ui/search/search_tab_helper_unittest.cc
@@ -19,6 +19,7 @@ #include "chrome/browser/ui/search/search_ipc_router.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/common/render_messages.h" +#include "chrome/common/search/mock_searchbox.h" #include "chrome/common/url_constants.h" #include "chrome/grit/generated_resources.h" #include "chrome/test/base/browser_with_test_window_test.h" @@ -41,7 +42,9 @@ class OmniboxView; +using testing::Eq; using testing::Return; +using testing::_; namespace { @@ -66,13 +69,26 @@ MOCK_METHOD0(OnHistorySyncCheck, void()); }; +class MockSearchBoxClientFactory + : public SearchIPCRouter::SearchBoxClientFactory { + public: + MOCK_METHOD0(GetSearchBox, chrome::mojom::SearchBox*(void)); +}; + } // namespace class SearchTabHelperTest : public ChromeRenderViewHostTestHarness { public: + SearchTabHelperTest() {} + void SetUp() override { ChromeRenderViewHostTestHarness::SetUp(); SearchTabHelper::CreateForWebContents(web_contents()); + auto search_tab = SearchTabHelper::FromWebContents(web_contents()); + auto factory = base::MakeUnique<MockSearchBoxClientFactory>(); + ON_CALL(*factory, GetSearchBox()).WillByDefault(Return(&mock_search_box_)); + search_tab->ipc_router_for_testing() + .set_search_box_client_factory_for_testing(std::move(factory)); } content::BrowserContext* CreateBrowserContext() override { @@ -110,14 +126,13 @@ .WillRepeatedly(Return(result)); } - bool MessageWasSent(uint32_t id) { - return process()->sink().GetFirstMessageMatching(id) != NULL; - } - MockSearchIPCRouterDelegate* mock_delegate() { return &delegate_; } + MockSearchBox* mock_search_box() { return &mock_search_box_; } + private: MockSearchIPCRouterDelegate delegate_; + MockSearchBox mock_search_box_; }; TEST_F(SearchTabHelperTest, DetermineIfPageSupportsInstant_Local) { @@ -127,27 +142,26 @@ SearchTabHelper* search_tab_helper = SearchTabHelper::FromWebContents(web_contents()); ASSERT_NE(static_cast<SearchTabHelper*>(NULL), search_tab_helper); - search_tab_helper->ipc_router().set_delegate_for_testing(mock_delegate()); + search_tab_helper->ipc_router_for_testing().set_delegate_for_testing( + mock_delegate()); search_tab_helper->DetermineIfPageSupportsInstant(); } TEST_F(SearchTabHelperTest, DetermineIfPageSupportsInstant_NonLocal) { NavigateAndCommit(GURL("chrome-search://foo/bar")); - process()->sink().ClearMessages(); EXPECT_CALL(*mock_delegate(), OnInstantSupportDetermined(true)).Times(1); SearchTabHelper* search_tab_helper = SearchTabHelper::FromWebContents(web_contents()); ASSERT_NE(static_cast<SearchTabHelper*>(NULL), search_tab_helper); - search_tab_helper->ipc_router().set_delegate_for_testing(mock_delegate()); + search_tab_helper->ipc_router_for_testing().set_delegate_for_testing( + mock_delegate()); + EXPECT_CALL(*mock_search_box(), DetermineIfPageSupportsInstant()); search_tab_helper->DetermineIfPageSupportsInstant(); - ASSERT_TRUE(MessageWasSent(ChromeViewMsg_DetermineIfPageSupportsInstant::ID)); - std::unique_ptr<IPC::Message> response( - new ChromeViewHostMsg_InstantSupportDetermined( - web_contents()->GetRenderViewHost()->GetRoutingID(), - search_tab_helper->ipc_router().page_seq_no_for_testing(), true)); - search_tab_helper->ipc_router().OnMessageReceived(*response); + search_tab_helper->ipc_router_for_testing().InstantSupportDetermined( + search_tab_helper->ipc_router_for_testing().page_seq_no_for_testing(), + true); } TEST_F(SearchTabHelperTest, PageURLDoesntBelongToInstantRenderer) { @@ -155,16 +169,15 @@ // SearchTabHelper::DeterminerIfPageSupportsInstant() should return // immediately without dispatching any message to the renderer. NavigateAndCommit(GURL("http://www.example.com")); - process()->sink().ClearMessages(); EXPECT_CALL(*mock_delegate(), OnInstantSupportDetermined(false)).Times(0); SearchTabHelper* search_tab_helper = SearchTabHelper::FromWebContents(web_contents()); ASSERT_NE(static_cast<SearchTabHelper*>(NULL), search_tab_helper); - search_tab_helper->ipc_router().set_delegate_for_testing(mock_delegate()); + search_tab_helper->ipc_router_for_testing().set_delegate_for_testing( + mock_delegate()); + EXPECT_CALL(*mock_search_box(), DetermineIfPageSupportsInstant()).Times(0); search_tab_helper->DetermineIfPageSupportsInstant(); - ASSERT_FALSE(MessageWasSent( - ChromeViewMsg_DetermineIfPageSupportsInstant::ID)); } TEST_F(SearchTabHelperTest, OnChromeIdentityCheckMatch) { @@ -175,16 +188,9 @@ ASSERT_NE(static_cast<SearchTabHelper*>(NULL), search_tab_helper); const base::string16 test_identity = base::ASCIIToUTF16("foo@bar.com"); + EXPECT_CALL(*mock_search_box(), + ChromeIdentityCheckResult(Eq(test_identity), true)); search_tab_helper->OnChromeIdentityCheck(test_identity); - - const IPC::Message* message = process()->sink().GetUniqueMessageMatching( - ChromeViewMsg_ChromeIdentityCheckResult::ID); - ASSERT_TRUE(message != NULL); - - ChromeViewMsg_ChromeIdentityCheckResult::Param params; - ChromeViewMsg_ChromeIdentityCheckResult::Read(message, ¶ms); - EXPECT_EQ(test_identity, std::get<0>(params)); - ASSERT_TRUE(std::get<1>(params)); } TEST_F(SearchTabHelperTest, OnChromeIdentityCheckMatchSlightlyDifferentGmail) { @@ -198,16 +204,9 @@ // standard form. const base::string16 test_identity = base::ASCIIToUTF16("Foo.Bar.123@gmail.com"); + EXPECT_CALL(*mock_search_box(), + ChromeIdentityCheckResult(Eq(test_identity), true)); search_tab_helper->OnChromeIdentityCheck(test_identity); - - const IPC::Message* message = process()->sink().GetUniqueMessageMatching( - ChromeViewMsg_ChromeIdentityCheckResult::ID); - ASSERT_TRUE(message != NULL); - - ChromeViewMsg_ChromeIdentityCheckResult::Param params; - ChromeViewMsg_ChromeIdentityCheckResult::Read(message, ¶ms); - EXPECT_EQ(test_identity, std::get<0>(params)); - ASSERT_TRUE(std::get<1>(params)); } TEST_F(SearchTabHelperTest, OnChromeIdentityCheckMatchSlightlyDifferentGmail2) { @@ -222,16 +221,9 @@ // a standard form. const base::string16 test_identity = base::ASCIIToUTF16("chromeuser7forever@googlemail.com"); + EXPECT_CALL(*mock_search_box(), + ChromeIdentityCheckResult(Eq(test_identity), true)); search_tab_helper->OnChromeIdentityCheck(test_identity); - - const IPC::Message* message = process()->sink().GetUniqueMessageMatching( - ChromeViewMsg_ChromeIdentityCheckResult::ID); - ASSERT_TRUE(message != NULL); - - ChromeViewMsg_ChromeIdentityCheckResult::Param params; - ChromeViewMsg_ChromeIdentityCheckResult::Read(message, ¶ms); - EXPECT_EQ(test_identity, std::get<0>(params)); - ASSERT_TRUE(std::get<1>(params)); } TEST_F(SearchTabHelperTest, OnChromeIdentityCheckMismatch) { @@ -242,16 +234,9 @@ ASSERT_NE(static_cast<SearchTabHelper*>(NULL), search_tab_helper); const base::string16 test_identity = base::ASCIIToUTF16("bar@foo.com"); + EXPECT_CALL(*mock_search_box(), + ChromeIdentityCheckResult(Eq(test_identity), false)); search_tab_helper->OnChromeIdentityCheck(test_identity); - - const IPC::Message* message = process()->sink().GetUniqueMessageMatching( - ChromeViewMsg_ChromeIdentityCheckResult::ID); - ASSERT_TRUE(message != NULL); - - ChromeViewMsg_ChromeIdentityCheckResult::Param params; - ChromeViewMsg_ChromeIdentityCheckResult::Read(message, ¶ms); - EXPECT_EQ(test_identity, std::get<0>(params)); - ASSERT_FALSE(std::get<1>(params)); } TEST_F(SearchTabHelperTest, OnChromeIdentityCheckSignedOutMismatch) { @@ -262,16 +247,9 @@ ASSERT_NE(static_cast<SearchTabHelper*>(NULL), search_tab_helper); const base::string16 test_identity = base::ASCIIToUTF16("bar@foo.com"); + EXPECT_CALL(*mock_search_box(), + ChromeIdentityCheckResult(Eq(test_identity), false)); search_tab_helper->OnChromeIdentityCheck(test_identity); - - const IPC::Message* message = process()->sink().GetUniqueMessageMatching( - ChromeViewMsg_ChromeIdentityCheckResult::ID); - ASSERT_TRUE(message != NULL); - - ChromeViewMsg_ChromeIdentityCheckResult::Param params; - ChromeViewMsg_ChromeIdentityCheckResult::Read(message, ¶ms); - EXPECT_EQ(test_identity, std::get<0>(params)); - ASSERT_FALSE(std::get<1>(params)); } TEST_F(SearchTabHelperTest, OnHistorySyncCheckSyncing) { @@ -281,15 +259,8 @@ SearchTabHelper::FromWebContents(web_contents()); ASSERT_NE(static_cast<SearchTabHelper*>(NULL), search_tab_helper); + EXPECT_CALL(*mock_search_box(), HistorySyncCheckResult(true)); search_tab_helper->OnHistorySyncCheck(); - - const IPC::Message* message = process()->sink().GetUniqueMessageMatching( - ChromeViewMsg_HistorySyncCheckResult::ID); - ASSERT_TRUE(message != NULL); - - ChromeViewMsg_HistorySyncCheckResult::Param params; - ChromeViewMsg_HistorySyncCheckResult::Read(message, ¶ms); - ASSERT_TRUE(std::get<0>(params)); } TEST_F(SearchTabHelperTest, OnHistorySyncCheckNotSyncing) { @@ -299,15 +270,8 @@ SearchTabHelper::FromWebContents(web_contents()); ASSERT_NE(static_cast<SearchTabHelper*>(NULL), search_tab_helper); + EXPECT_CALL(*mock_search_box(), HistorySyncCheckResult(false)); search_tab_helper->OnHistorySyncCheck(); - - const IPC::Message* message = process()->sink().GetUniqueMessageMatching( - ChromeViewMsg_HistorySyncCheckResult::ID); - ASSERT_TRUE(message != NULL); - - ChromeViewMsg_HistorySyncCheckResult::Param params; - ChromeViewMsg_HistorySyncCheckResult::Read(message, ¶ms); - ASSERT_FALSE(std::get<0>(params)); } class TabTitleObserver : public content::WebContentsObserver {
diff --git a/chrome/common/BUILD.gn b/chrome/common/BUILD.gn index 2c5b6d8..e34b618 100644 --- a/chrome/common/BUILD.gn +++ b/chrome/common/BUILD.gn
@@ -92,6 +92,7 @@ "descriptors_android.h", "ini_parser.cc", "ini_parser.h", + "instant_type_traits.h", "logging_chrome.cc", "logging_chrome.h", "mac/app_shim_launch.h", @@ -623,13 +624,18 @@ # require many files from it. This makes linking more efficient. static_library("test_support") { testonly = true - visibility = [ "//chrome/test:test_support" ] + visibility = [ "//chrome/test:*" ] - sources = [] + sources = [ + "search/mock_searchbox.cc", + "search/mock_searchbox.h", + ] deps = [ ":common", + ":instant_mojom", "//base", + "//testing/gmock", "//testing/gtest", ] @@ -685,3 +691,14 @@ "//url/mojo:url_mojom_gurl", ] } + +mojom("instant_mojom") { + sources = [ + "instant.mojom", + ] + + public_deps = [ + "//mojo/common:common_custom_types", + "//url/mojo:url_mojom_gurl", + ] +}
diff --git a/chrome/common/instant.mojom b/chrome/common/instant.mojom new file mode 100644 index 0000000..3935339e --- /dev/null +++ b/chrome/common/instant.mojom
@@ -0,0 +1,104 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +module chrome.mojom; + +import "mojo/common/string16.mojom"; +import "mojo/common/time.mojom"; +import "url/mojo/url.mojom"; + +[Native] +enum NTPLoggingEventType; + +[Native] +enum OmniboxFocusState; + +[Native] +enum NTPTileSource; + +interface Instant { + // Tells InstantExtended whether the embedded search API is supported. + // See http://dev.chromium.org/embeddedsearch + InstantSupportDetermined(int32 page_seq_no, bool result); + + // Tells InstantExtended to set the omnibox focus state. + FocusOmnibox(int32 page_seq_no, OmniboxFocusState state); + + // TODO(treib): Remove the SearchBox prefix from these three methods. + // Tells InstantExtended to delete a most visited item. + SearchBoxDeleteMostVisitedItem(int32 page_seq_no, url.mojom.Url url); + + // Tells InstantExtended to undo all most visited item deletions. + SearchBoxUndoAllMostVisitedDeletions(int32 page_seq_no); + + // Tells InstantExtended to undo one most visited item deletion. + SearchBoxUndoMostVisitedDeletion(int32 page_seq_no, url.mojom.Url url); + + // Logs events from InstantExtended New Tab Pages. + LogEvent(int32 page_seq_no, + NTPLoggingEventType event, + mojo.common.mojom.TimeDelta time); + + // Logs an impression on one of the Most Visited tile on the InstantExtended + // New Tab Page. + LogMostVisitedImpression(int32 page_seq_no, int32 position, + NTPTileSource tile_source); + + // Logs a navigation on one of the Most Visited tile on the InstantExtended + // New Tab Page. + LogMostVisitedNavigation(int32 page_seq_no, int32 position, + NTPTileSource tile_source); + + // Tells InstantExtended to paste text into the omnibox. If text is empty, + // the clipboard contents will be pasted. This causes the omnibox dropdown to + // open. + PasteAndOpenDropdown(int32 page_seq_no, + mojo.common.mojom.String16 text_to_be_pasted); + + // The Instant page asks whether the user syncs its history. + HistorySyncCheck(int32 page_seq_no); + + // The Instant page asks for Chrome identity check against |identity|. + ChromeIdentityCheck(int32 page_seq_no, mojo.common.mojom.String16 identity); +}; + +[Native] +enum OmniboxFocusChangeReason; + +[Native] +struct InstantMostVisitedItem; + +[Native] +struct InstantSuggestion; + +[Native] +struct EmbeddedSearchRequestParams; + +[Native] +struct ThemeBackgroundInfo; + +interface SearchBox { + SetPageSequenceNumber(int32 page_seq_no); + + DetermineIfPageSupportsInstant(); + + FocusChanged(OmniboxFocusState new_focus_state, + OmniboxFocusChangeReason reason); + + MostVisitedChanged(array<InstantMostVisitedItem> items); + + SetInputInProgress(bool input_in_progress); + + SetSuggestionToPrefetch(InstantSuggestion suggestion); + + Submit(mojo.common.mojom.String16 value, + EmbeddedSearchRequestParams params); + + ThemeChanged(ThemeBackgroundInfo value); + + HistorySyncCheckResult(bool sync_history); + + ChromeIdentityCheckResult(mojo.common.mojom.String16 identity, + bool identity_match); +};
diff --git a/chrome/common/instant.typemap b/chrome/common/instant.typemap new file mode 100644 index 0000000..04878d87 --- /dev/null +++ b/chrome/common/instant.typemap
@@ -0,0 +1,26 @@ +# Copyright 2016 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +mojom = "//chrome/common/instant.mojom" +public_headers = [ + "//chrome/common/search/instant_types.h", + "//chrome/common/search/ntp_logging_events.h", + "//components/ntp_tiles/ntp_tile_source.h", + "//components/omnibox/common/omnibox_focus_state.h", +] +traits_headers = [ "//chrome/common/instant_type_traits.h" ] +deps = [ + "//chrome/common", + "//ipc", +] +type_mappings = [ + "chrome.mojom.NTPLoggingEventType=::NTPLoggingEventType", + "chrome.mojom.NTPTileSource=::ntp_tiles::NTPTileSource", + "chrome.mojom.OmniboxFocusState=::OmniboxFocusState", + "chrome.mojom.OmniboxFocusChangeReason=::OmniboxFocusChangeReason", + "chrome.mojom.InstantMostVisitedItem=::InstantMostVisitedItem", + "chrome.mojom.InstantSuggestion=::InstantSuggestion", + "chrome.mojom.EmbeddedSearchRequestParams=::EmbeddedSearchRequestParams", + "chrome.mojom.ThemeBackgroundInfo=::ThemeBackgroundInfo", +]
diff --git a/chrome/common/instant_type_traits.h b/chrome/common/instant_type_traits.h new file mode 100644 index 0000000..4b97866 --- /dev/null +++ b/chrome/common/instant_type_traits.h
@@ -0,0 +1,57 @@ +// Copyright (c) 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Multiply-included file, no traditional include guard. +#include "chrome/common/search/instant_types.h" +#include "chrome/common/search/ntp_logging_events.h" +#include "components/omnibox/common/omnibox_focus_state.h" +#include "ipc/ipc_message_macros.h" + +IPC_ENUM_TRAITS_MAX_VALUE(OmniboxFocusState, OMNIBOX_FOCUS_STATE_LAST) + +IPC_ENUM_TRAITS_MAX_VALUE(OmniboxFocusChangeReason, + OMNIBOX_FOCUS_CHANGE_REASON_LAST) + +IPC_ENUM_TRAITS_MAX_VALUE(NTPLoggingEventType, NTP_EVENT_TYPE_LAST) + +IPC_ENUM_TRAITS_MAX_VALUE(ntp_tiles::NTPTileSource, + ntp_tiles::NTPTileSource::LAST) + +IPC_STRUCT_TRAITS_BEGIN(InstantMostVisitedItem) + IPC_STRUCT_TRAITS_MEMBER(url) + IPC_STRUCT_TRAITS_MEMBER(title) + IPC_STRUCT_TRAITS_MEMBER(thumbnail) + IPC_STRUCT_TRAITS_MEMBER(favicon) + IPC_STRUCT_TRAITS_MEMBER(source) +IPC_STRUCT_TRAITS_END() + +IPC_STRUCT_TRAITS_BEGIN(InstantSuggestion) + IPC_STRUCT_TRAITS_MEMBER(text) + IPC_STRUCT_TRAITS_MEMBER(metadata) +IPC_STRUCT_TRAITS_END() + +IPC_STRUCT_TRAITS_BEGIN(EmbeddedSearchRequestParams) + IPC_STRUCT_TRAITS_MEMBER(search_query) + IPC_STRUCT_TRAITS_MEMBER(original_query) + IPC_STRUCT_TRAITS_MEMBER(rlz_parameter_value) + IPC_STRUCT_TRAITS_MEMBER(input_encoding) + IPC_STRUCT_TRAITS_MEMBER(assisted_query_stats) +IPC_STRUCT_TRAITS_END() + +IPC_STRUCT_TRAITS_BEGIN(ThemeBackgroundInfo) + IPC_STRUCT_TRAITS_MEMBER(using_default_theme) + IPC_STRUCT_TRAITS_MEMBER(background_color) + IPC_STRUCT_TRAITS_MEMBER(text_color) + IPC_STRUCT_TRAITS_MEMBER(link_color) + IPC_STRUCT_TRAITS_MEMBER(text_color_light) + IPC_STRUCT_TRAITS_MEMBER(header_color) + IPC_STRUCT_TRAITS_MEMBER(section_border_color) + IPC_STRUCT_TRAITS_MEMBER(theme_id) + IPC_STRUCT_TRAITS_MEMBER(image_horizontal_alignment) + IPC_STRUCT_TRAITS_MEMBER(image_vertical_alignment) + IPC_STRUCT_TRAITS_MEMBER(image_tiling) + IPC_STRUCT_TRAITS_MEMBER(image_height) + IPC_STRUCT_TRAITS_MEMBER(has_attribution) + IPC_STRUCT_TRAITS_MEMBER(logo_alternate) +IPC_STRUCT_TRAITS_END()
diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h index b8242b3..4ebddb8 100644 --- a/chrome/common/render_messages.h +++ b/chrome/common/render_messages.h
@@ -12,6 +12,7 @@ #include "base/time/time.h" #include "build/build_config.h" #include "chrome/common/features.h" +#include "chrome/common/instant_type_traits.h" #include "chrome/common/search/instant_types.h" #include "chrome/common/search/ntp_logging_events.h" #include "chrome/common/web_application_info.h" @@ -80,9 +81,6 @@ IPC_ENUM_TRAITS_MAX_VALUE(ChromeViewHostMsg_GetPluginInfo_Status, ChromeViewHostMsg_GetPluginInfo_Status::kUnauthorized) -IPC_ENUM_TRAITS_MAX_VALUE(OmniboxFocusChangeReason, - OMNIBOX_FOCUS_CHANGE_REASON_LAST) -IPC_ENUM_TRAITS_MAX_VALUE(OmniboxFocusState, OMNIBOX_FOCUS_STATE_LAST) IPC_ENUM_TRAITS_MAX_VALUE(ThemeBackgroundImageAlignment, THEME_BKGRND_IMAGE_ALIGN_LAST) IPC_ENUM_TRAITS_MAX_VALUE(ThemeBackgroundImageTiling, THEME_BKGRND_IMAGE_LAST) @@ -119,27 +117,6 @@ IPC_STRUCT_TRAITS_MEMBER(incognito) IPC_STRUCT_TRAITS_END() -IPC_STRUCT_TRAITS_BEGIN(EmbeddedSearchRequestParams) - IPC_STRUCT_TRAITS_MEMBER(search_query) - IPC_STRUCT_TRAITS_MEMBER(original_query) - IPC_STRUCT_TRAITS_MEMBER(rlz_parameter_value) - IPC_STRUCT_TRAITS_MEMBER(input_encoding) - IPC_STRUCT_TRAITS_MEMBER(assisted_query_stats) -IPC_STRUCT_TRAITS_END() - -IPC_STRUCT_TRAITS_BEGIN(InstantSuggestion) - IPC_STRUCT_TRAITS_MEMBER(text) - IPC_STRUCT_TRAITS_MEMBER(metadata) -IPC_STRUCT_TRAITS_END() - -IPC_STRUCT_TRAITS_BEGIN(InstantMostVisitedItem) - IPC_STRUCT_TRAITS_MEMBER(url) - IPC_STRUCT_TRAITS_MEMBER(title) - IPC_STRUCT_TRAITS_MEMBER(thumbnail) - IPC_STRUCT_TRAITS_MEMBER(favicon) - IPC_STRUCT_TRAITS_MEMBER(source) -IPC_STRUCT_TRAITS_END() - IPC_STRUCT_TRAITS_BEGIN(RendererContentSettingRules) IPC_STRUCT_TRAITS_MEMBER(image_rules) IPC_STRUCT_TRAITS_MEMBER(script_rules) @@ -153,29 +130,6 @@ IPC_STRUCT_TRAITS_MEMBER(a) IPC_STRUCT_TRAITS_END() -IPC_STRUCT_TRAITS_BEGIN(ThemeBackgroundInfo) - IPC_STRUCT_TRAITS_MEMBER(using_default_theme) - IPC_STRUCT_TRAITS_MEMBER(background_color) - IPC_STRUCT_TRAITS_MEMBER(text_color) - IPC_STRUCT_TRAITS_MEMBER(link_color) - IPC_STRUCT_TRAITS_MEMBER(text_color_light) - IPC_STRUCT_TRAITS_MEMBER(header_color) - IPC_STRUCT_TRAITS_MEMBER(section_border_color) - IPC_STRUCT_TRAITS_MEMBER(theme_id) - IPC_STRUCT_TRAITS_MEMBER(image_horizontal_alignment) - IPC_STRUCT_TRAITS_MEMBER(image_vertical_alignment) - IPC_STRUCT_TRAITS_MEMBER(image_tiling) - IPC_STRUCT_TRAITS_MEMBER(image_height) - IPC_STRUCT_TRAITS_MEMBER(has_attribution) - IPC_STRUCT_TRAITS_MEMBER(logo_alternate) -IPC_STRUCT_TRAITS_END() - -IPC_ENUM_TRAITS_MAX_VALUE(NTPLoggingEventType, - NTP_EVENT_TYPE_LAST) - -IPC_ENUM_TRAITS_MAX_VALUE(ntp_tiles::NTPTileSource, - ntp_tiles::NTPTileSource::LAST) - IPC_ENUM_TRAITS_MAX_VALUE(WebApplicationInfo::MobileCapable, WebApplicationInfo::MOBILE_CAPABLE_APPLE) @@ -220,38 +174,6 @@ std::string /* field trial name */, std::string /* group name that was assigned. */) -IPC_MESSAGE_ROUTED1(ChromeViewMsg_SetPageSequenceNumber, - int /* page_seq_no */) - -IPC_MESSAGE_ROUTED0(ChromeViewMsg_DetermineIfPageSupportsInstant) - -IPC_MESSAGE_ROUTED2(ChromeViewMsg_SearchBoxFocusChanged, - OmniboxFocusState /* new_focus_state */, - OmniboxFocusChangeReason /* reason */) - -IPC_MESSAGE_ROUTED1(ChromeViewMsg_SearchBoxMostVisitedItemsChanged, - std::vector<InstantMostVisitedItem> /* items */) - -IPC_MESSAGE_ROUTED1(ChromeViewMsg_SearchBoxSetInputInProgress, - bool /* input_in_progress */) - -IPC_MESSAGE_ROUTED1(ChromeViewMsg_SearchBoxSetSuggestionToPrefetch, - InstantSuggestion /* suggestion */) - -IPC_MESSAGE_ROUTED2(ChromeViewMsg_SearchBoxSubmit, - base::string16 /* value */, - EmbeddedSearchRequestParams /* params */) - -IPC_MESSAGE_ROUTED1(ChromeViewMsg_SearchBoxThemeChanged, - ThemeBackgroundInfo /* value */) - -IPC_MESSAGE_ROUTED1(ChromeViewMsg_HistorySyncCheckResult, - bool /* sync_history */) - -IPC_MESSAGE_ROUTED2(ChromeViewMsg_ChromeIdentityCheckResult, - base::string16 /* identity */, - bool /* identity_match */) - // Sent to allow or forbid the running of insecure mixed-content. IPC_MESSAGE_ROUTED1(ChromeViewMsg_SetAllowRunningInsecureContent, bool /* allowed */) @@ -518,67 +440,6 @@ IPC_MESSAGE_ROUTED1(ChromeViewHostMsg_DidGetWebApplicationInfo, WebApplicationInfo) -// Logs events from InstantExtended New Tab Pages. -IPC_MESSAGE_ROUTED3(ChromeViewHostMsg_LogEvent, - int /* page_seq_no */, - NTPLoggingEventType /* event */, - base::TimeDelta /* time */) - -// Logs an impression on one of the Most Visited tile on the InstantExtended -// New Tab Page. -IPC_MESSAGE_ROUTED3(ChromeViewHostMsg_LogMostVisitedImpression, - int /* page_seq_no */, - int /* position */, - ntp_tiles::NTPTileSource /* tile_source */) - -// Logs a navigation on one of the Most Visited tile on the InstantExtended -// New Tab Page. -IPC_MESSAGE_ROUTED3(ChromeViewHostMsg_LogMostVisitedNavigation, - int /* page_seq_no */, - int /* position */, - ntp_tiles::NTPTileSource /* tile_source */) - -// The Instant page asks whether the user syncs its history. -IPC_MESSAGE_ROUTED1(ChromeViewHostMsg_HistorySyncCheck, - int /* page_seq_no */) - -// The Instant page asks for Chrome identity check against |identity|. -IPC_MESSAGE_ROUTED2(ChromeViewHostMsg_ChromeIdentityCheck, - int /* page_seq_no */, - base::string16 /* identity */) - -// Tells InstantExtended to set the omnibox focus state. -IPC_MESSAGE_ROUTED2(ChromeViewHostMsg_FocusOmnibox, - int /* page_seq_no */, - OmniboxFocusState /* state */) - -// Tells InstantExtended to paste text into the omnibox. If text is empty, -// the clipboard contents will be pasted. This causes the omnibox dropdown to -// open. -IPC_MESSAGE_ROUTED2(ChromeViewHostMsg_PasteAndOpenDropdown, - int /* page_seq_no */, - base::string16 /* text to be pasted */) - -// Tells InstantExtended whether the embedded search API is supported. -// See http://dev.chromium.org/embeddedsearch -IPC_MESSAGE_ROUTED2(ChromeViewHostMsg_InstantSupportDetermined, - int /* page_seq_no */, - bool /* result */) - -// Tells InstantExtended to delete a most visited item. -IPC_MESSAGE_ROUTED2(ChromeViewHostMsg_SearchBoxDeleteMostVisitedItem, - int /* page_seq_no */, - GURL /* url */) - -// Tells InstantExtended to undo all most visited item deletions. -IPC_MESSAGE_ROUTED1(ChromeViewHostMsg_SearchBoxUndoAllMostVisitedDeletions, - int /* page_seq_no */) - -// Tells InstantExtended to undo one most visited item deletion. -IPC_MESSAGE_ROUTED2(ChromeViewHostMsg_SearchBoxUndoMostVisitedDeletion, - int /* page_seq_no */, - GURL /* url */) - // Tells the renderer a list of URLs which should be bounced back to the browser // process so that they can be assigned to an Instant renderer. IPC_MESSAGE_CONTROL2(ChromeViewMsg_SetSearchURLs,
diff --git a/chrome/common/search/mock_searchbox.cc b/chrome/common/search/mock_searchbox.cc new file mode 100644 index 0000000..a6412a3 --- /dev/null +++ b/chrome/common/search/mock_searchbox.cc
@@ -0,0 +1,8 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/common/search/mock_searchbox.h" + +MockSearchBox::MockSearchBox() = default; +MockSearchBox::~MockSearchBox() = default;
diff --git a/chrome/common/search/mock_searchbox.h b/chrome/common/search/mock_searchbox.h new file mode 100644 index 0000000..baaf455 --- /dev/null +++ b/chrome/common/search/mock_searchbox.h
@@ -0,0 +1,31 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_COMMON_SEARCH_MOCK_SEARCHBOX_H_ +#define CHROME_COMMON_SEARCH_MOCK_SEARCHBOX_H_ + +#include "chrome/common/instant.mojom.h" +#include "testing/gmock/include/gmock/gmock.h" + +class MockSearchBox : public chrome::mojom::SearchBox { + public: + MockSearchBox(); + ~MockSearchBox(); + + MOCK_METHOD1(SetPageSequenceNumber, void(int)); + MOCK_METHOD2(ChromeIdentityCheckResult, void(const base::string16&, bool)); + MOCK_METHOD0(DetermineIfPageSupportsInstant, void()); + MOCK_METHOD2(FocusChanged, void(OmniboxFocusState, OmniboxFocusChangeReason)); + MOCK_METHOD1(HistorySyncCheckResult, void(bool)); + MOCK_METHOD1(MostVisitedChanged, + void(const std::vector<InstantMostVisitedItem>&)); + MOCK_METHOD1(SetInputInProgress, void(bool)); + MOCK_METHOD1(SetSuggestionToPrefetch, void(const InstantSuggestion&)); + MOCK_METHOD2(Submit, + void(const base::string16&, const EmbeddedSearchRequestParams&)); + MOCK_METHOD1(ThemeChanged, void(const ThemeBackgroundInfo&)); +}; + + +#endif // CHROME_COMMON_SEARCH_MOCK_SEARCHBOX_H_
diff --git a/chrome/renderer/BUILD.gn b/chrome/renderer/BUILD.gn index fe84d48f..e3783b3a 100644 --- a/chrome/renderer/BUILD.gn +++ b/chrome/renderer/BUILD.gn
@@ -92,6 +92,7 @@ "//chrome:resources", "//chrome:strings", "//chrome/common", + "//chrome/common:instant_mojom", "//chrome/common:mojo_bindings", "//chrome/common/net", "//components/autofill/content/renderer", @@ -127,6 +128,7 @@ "//net", "//ppapi/features", "//printing/features", + "//services/service_manager/public/cpp", "//skia", "//storage/common", "//third_party/WebKit/public:blink", @@ -377,6 +379,7 @@ deps = [ ":renderer", + "//chrome/common:instant_mojom", "//content/test:test_support", "//extensions/features", "//testing/gmock",
diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc index 25add39..6728fc9 100644 --- a/chrome/renderer/chrome_content_renderer_client.cc +++ b/chrome/renderer/chrome_content_renderer_client.cc
@@ -520,6 +520,11 @@ new subresource_filter::SubresourceFilterAgent( render_frame, subresource_filter_ruleset_dealer_.get()); } + + if (command_line->HasSwitch(switches::kInstantProcess) && + render_frame->IsMainFrame()) { + new SearchBox(render_frame); + } } void ChromeContentRendererClient::RenderViewCreated( @@ -532,10 +537,6 @@ #endif new prerender::PrerendererClient(render_view); - base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); - if (command_line->HasSwitch(switches::kInstantProcess)) - new SearchBox(render_view); - new ChromeRenderViewObserver(render_view, web_cache_impl_.get()); new password_manager::CredentialManagerClient(render_view); @@ -1118,9 +1119,8 @@ if (!url.protocolIs(chrome::kChromeSearchScheme)) return false; - const content::RenderView* render_view = - content::RenderView::FromWebView(frame->view()); - SearchBox* search_box = SearchBox::Get(render_view); + SearchBox* search_box = + SearchBox::Get(content::RenderFrame::FromWebFrame(frame)); if (search_box) { // Note: this GURL copy could be avoided if host() were added to WebURL. GURL gurl(url);
diff --git a/chrome/renderer/searchbox/searchbox.cc b/chrome/renderer/searchbox/searchbox.cc index 6262ed56..41a44b6 100644 --- a/chrome/renderer/searchbox/searchbox.cc +++ b/chrome/renderer/searchbox/searchbox.cc
@@ -16,6 +16,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/time/time.h" #include "chrome/common/chrome_switches.h" +#include "chrome/common/instant.mojom.h" #include "chrome/common/render_messages.h" #include "chrome/common/url_constants.h" #include "chrome/renderer/searchbox/searchbox_extension.h" @@ -24,6 +25,8 @@ #include "components/favicon_base/favicon_url_parser.h" #include "components/favicon_base/large_icon_url_parser.h" #include "components/omnibox/common/omnibox_focus_state.h" +#include "content/public/common/associated_interface_provider.h" +#include "content/public/common/associated_interface_registry.h" #include "content/public/renderer/render_frame.h" #include "content/public/renderer/render_view.h" #include "net/base/escape.h" @@ -123,7 +126,7 @@ } int SearchBoxIconURLHelper::GetViewID() const { - return search_box_->render_view()->GetRoutingID(); + return search_box_->render_frame()->GetRenderView()->GetRoutingID(); } std::string SearchBoxIconURLHelper::GetURLStringFromRestrictedID( @@ -231,69 +234,60 @@ SearchBox::IconURLHelper::~IconURLHelper() { } -SearchBox::SearchBox(content::RenderView* render_view) - : content::RenderViewObserver(render_view), - content::RenderViewObserverTracker<SearchBox>(render_view), - page_seq_no_(0), - is_focused_(false), - is_input_in_progress_(false), - is_key_capture_enabled_(false), - most_visited_items_cache_(kMaxInstantMostVisitedItemCacheSize), - query_() { +SearchBox::SearchBox(content::RenderFrame* render_frame) + : content::RenderFrameObserver(render_frame), + content::RenderFrameObserverTracker<SearchBox>(render_frame), + page_seq_no_(0), + is_focused_(false), + is_input_in_progress_(false), + is_key_capture_enabled_(false), + most_visited_items_cache_(kMaxInstantMostVisitedItemCacheSize), + query_(), + binding_(this) { + render_frame->GetRemoteAssociatedInterfaces()->GetInterface( + &instant_service_); + render_frame->GetAssociatedInterfaceRegistry()->AddInterface( + base::Bind(&SearchBox::Bind, base::Unretained(this))); } SearchBox::~SearchBox() { } void SearchBox::LogEvent(NTPLoggingEventType event) { - // The main frame for the current RenderView may be out-of-process, in which - // case it won't have performance(). Use the default delta of 0 in this - // case. - base::TimeDelta delta; - if (render_view()->GetWebView()->mainFrame()->isWebLocalFrame()) { - // navigation_start in ms. - uint64_t start = 1000 * (render_view() - ->GetMainRenderFrame() - ->GetWebFrame() - ->performance() - .navigationStart()); - uint64_t now = (base::TimeTicks::Now() - base::TimeTicks::UnixEpoch()) - .InMilliseconds(); - DCHECK(now >= start); - delta = base::TimeDelta::FromMilliseconds(now - start); - } - render_view()->Send(new ChromeViewHostMsg_LogEvent( - render_view()->GetRoutingID(), page_seq_no_, event, delta)); + // navigation_start in ms. + uint64_t start = + 1000 * (render_frame()->GetWebFrame()->performance().navigationStart()); + uint64_t now = + (base::TimeTicks::Now() - base::TimeTicks::UnixEpoch()).InMilliseconds(); + DCHECK(now >= start); + base::TimeDelta delta = base::TimeDelta::FromMilliseconds(now - start); + instant_service_->LogEvent(page_seq_no_, event, delta); } void SearchBox::LogMostVisitedImpression(int position, ntp_tiles::NTPTileSource tile_source) { - render_view()->Send(new ChromeViewHostMsg_LogMostVisitedImpression( - render_view()->GetRoutingID(), page_seq_no_, position, tile_source)); + instant_service_->LogMostVisitedImpression(page_seq_no_, position, + tile_source); } void SearchBox::LogMostVisitedNavigation(int position, ntp_tiles::NTPTileSource tile_source) { - render_view()->Send(new ChromeViewHostMsg_LogMostVisitedNavigation( - render_view()->GetRoutingID(), page_seq_no_, position, tile_source)); + instant_service_->LogMostVisitedNavigation(page_seq_no_, position, + tile_source); } void SearchBox::CheckIsUserSignedInToChromeAs(const base::string16& identity) { - render_view()->Send(new ChromeViewHostMsg_ChromeIdentityCheck( - render_view()->GetRoutingID(), page_seq_no_, identity)); + instant_service_->ChromeIdentityCheck(page_seq_no_, identity); } void SearchBox::CheckIsUserSyncingHistory() { - render_view()->Send(new ChromeViewHostMsg_HistorySyncCheck( - render_view()->GetRoutingID(), page_seq_no_)); + instant_service_->HistorySyncCheck(page_seq_no_); } void SearchBox::DeleteMostVisitedItem( InstantRestrictedID most_visited_item_id) { - render_view()->Send(new ChromeViewHostMsg_SearchBoxDeleteMostVisitedItem( - render_view()->GetRoutingID(), - page_seq_no_, - GetURLForMostVisitedItem(most_visited_item_id))); + instant_service_->SearchBoxDeleteMostVisitedItem( + page_seq_no_, GetURLForMostVisitedItem(most_visited_item_id)); } bool SearchBox::GenerateImageURLFromTransientURL(const GURL& transient_url, @@ -324,83 +318,46 @@ } void SearchBox::Paste(const base::string16& text) { - render_view()->Send(new ChromeViewHostMsg_PasteAndOpenDropdown( - render_view()->GetRoutingID(), page_seq_no_, text)); + instant_service_->PasteAndOpenDropdown(page_seq_no_, text); } void SearchBox::StartCapturingKeyStrokes() { - render_view()->Send(new ChromeViewHostMsg_FocusOmnibox( - render_view()->GetRoutingID(), page_seq_no_, OMNIBOX_FOCUS_INVISIBLE)); + instant_service_->FocusOmnibox(page_seq_no_, OMNIBOX_FOCUS_INVISIBLE); } void SearchBox::StopCapturingKeyStrokes() { - render_view()->Send(new ChromeViewHostMsg_FocusOmnibox( - render_view()->GetRoutingID(), page_seq_no_, OMNIBOX_FOCUS_NONE)); + instant_service_->FocusOmnibox(page_seq_no_, OMNIBOX_FOCUS_NONE); } void SearchBox::UndoAllMostVisitedDeletions() { - render_view()->Send( - new ChromeViewHostMsg_SearchBoxUndoAllMostVisitedDeletions( - render_view()->GetRoutingID(), page_seq_no_)); + instant_service_->SearchBoxUndoAllMostVisitedDeletions(page_seq_no_); } void SearchBox::UndoMostVisitedDeletion( InstantRestrictedID most_visited_item_id) { - render_view()->Send(new ChromeViewHostMsg_SearchBoxUndoMostVisitedDeletion( - render_view()->GetRoutingID(), page_seq_no_, - GetURLForMostVisitedItem(most_visited_item_id))); + instant_service_->SearchBoxUndoMostVisitedDeletion( + page_seq_no_, GetURLForMostVisitedItem(most_visited_item_id)); } -bool SearchBox::OnMessageReceived(const IPC::Message& message) { - bool handled = true; - IPC_BEGIN_MESSAGE_MAP(SearchBox, message) - IPC_MESSAGE_HANDLER(ChromeViewMsg_SetPageSequenceNumber, - OnSetPageSequenceNumber) - IPC_MESSAGE_HANDLER(ChromeViewMsg_ChromeIdentityCheckResult, - OnChromeIdentityCheckResult) - IPC_MESSAGE_HANDLER(ChromeViewMsg_DetermineIfPageSupportsInstant, - OnDetermineIfPageSupportsInstant) - IPC_MESSAGE_HANDLER(ChromeViewMsg_HistorySyncCheckResult, - OnHistorySyncCheckResult) - IPC_MESSAGE_HANDLER(ChromeViewMsg_SearchBoxFocusChanged, OnFocusChanged) - IPC_MESSAGE_HANDLER(ChromeViewMsg_SearchBoxMostVisitedItemsChanged, - OnMostVisitedChanged) - IPC_MESSAGE_HANDLER(ChromeViewMsg_SearchBoxSetInputInProgress, - OnSetInputInProgress) - IPC_MESSAGE_HANDLER(ChromeViewMsg_SearchBoxSetSuggestionToPrefetch, - OnSetSuggestionToPrefetch) - IPC_MESSAGE_HANDLER(ChromeViewMsg_SearchBoxSubmit, OnSubmit) - IPC_MESSAGE_HANDLER(ChromeViewMsg_SearchBoxThemeChanged, - OnThemeChanged) - IPC_MESSAGE_UNHANDLED(handled = false) - IPC_END_MESSAGE_MAP() - return handled; -} - -void SearchBox::OnSetPageSequenceNumber(int page_seq_no) { +void SearchBox::SetPageSequenceNumber(int page_seq_no) { page_seq_no_ = page_seq_no; } -void SearchBox::OnChromeIdentityCheckResult(const base::string16& identity, - bool identity_match) { - if (render_view()->GetWebView() && render_view()->GetWebView()->mainFrame()) { - extensions_v8::SearchBoxExtension::DispatchChromeIdentityCheckResult( - render_view()->GetWebView()->mainFrame(), identity, identity_match); - } +void SearchBox::ChromeIdentityCheckResult(const base::string16& identity, + bool identity_match) { + extensions_v8::SearchBoxExtension::DispatchChromeIdentityCheckResult( + render_frame()->GetWebFrame(), identity, identity_match); } -void SearchBox::OnDetermineIfPageSupportsInstant() { - if (render_view()->GetWebView() && render_view()->GetWebView()->mainFrame()) { - bool result = extensions_v8::SearchBoxExtension::PageSupportsInstant( - render_view()->GetWebView()->mainFrame()); - DVLOG(1) << render_view() << " PageSupportsInstant: " << result; - render_view()->Send(new ChromeViewHostMsg_InstantSupportDetermined( - render_view()->GetRoutingID(), page_seq_no_, result)); - } +void SearchBox::DetermineIfPageSupportsInstant() { + bool result = extensions_v8::SearchBoxExtension::PageSupportsInstant( + render_frame()->GetWebFrame()); + DVLOG(1) << render_frame() << " PageSupportsInstant: " << result; + instant_service_->InstantSupportDetermined(page_seq_no_, result); } -void SearchBox::OnFocusChanged(OmniboxFocusState new_focus_state, - OmniboxFocusChangeReason reason) { +void SearchBox::FocusChanged(OmniboxFocusState new_focus_state, + OmniboxFocusChangeReason reason) { bool key_capture_enabled = new_focus_state == OMNIBOX_FOCUS_INVISIBLE; if (key_capture_enabled != is_key_capture_enabled_) { // Tell the page if the key capture mode changed unless the focus state @@ -411,98 +368,81 @@ // onkeycapturechange before the corresponding onchange, and the page would // have no way of telling whether the keycapturechange happened because of // some actual user action or just because they started typing.) - if (reason != OMNIBOX_FOCUS_CHANGE_TYPING && - render_view()->GetWebView() && - render_view()->GetWebView()->mainFrame()) { + if (reason != OMNIBOX_FOCUS_CHANGE_TYPING) { is_key_capture_enabled_ = key_capture_enabled; - DVLOG(1) << render_view() << " OnKeyCaptureChange"; + DVLOG(1) << render_frame() << " KeyCaptureChange"; extensions_v8::SearchBoxExtension::DispatchKeyCaptureChange( - render_view()->GetWebView()->mainFrame()); + render_frame()->GetWebFrame()); } } bool is_focused = new_focus_state == OMNIBOX_FOCUS_VISIBLE; if (is_focused != is_focused_) { is_focused_ = is_focused; - DVLOG(1) << render_view() << " OnFocusChange"; - if (render_view()->GetWebView() && - render_view()->GetWebView()->mainFrame()) { - extensions_v8::SearchBoxExtension::DispatchFocusChange( - render_view()->GetWebView()->mainFrame()); - } + DVLOG(1) << render_frame() << " FocusChange"; + extensions_v8::SearchBoxExtension::DispatchFocusChange( + render_frame()->GetWebFrame()); } } -void SearchBox::OnHistorySyncCheckResult(bool sync_history) { - if (render_view()->GetWebView() && render_view()->GetWebView()->mainFrame()) { - extensions_v8::SearchBoxExtension::DispatchHistorySyncCheckResult( - render_view()->GetWebView()->mainFrame(), sync_history); - } +void SearchBox::HistorySyncCheckResult(bool sync_history) { + extensions_v8::SearchBoxExtension::DispatchHistorySyncCheckResult( + render_frame()->GetWebFrame(), sync_history); } -void SearchBox::OnMostVisitedChanged( +void SearchBox::MostVisitedChanged( const std::vector<InstantMostVisitedItem>& items) { std::vector<InstantMostVisitedItemIDPair> last_known_items; GetMostVisitedItems(&last_known_items); - if (AreMostVisitedItemsEqual(last_known_items, items)) + if (AreMostVisitedItemsEqual(last_known_items, items)) { return; // Do not send duplicate onmostvisitedchange events. + } most_visited_items_cache_.AddItems(items); - if (render_view()->GetWebView() && render_view()->GetWebView()->mainFrame()) { - extensions_v8::SearchBoxExtension::DispatchMostVisitedChanged( - render_view()->GetWebView()->mainFrame()); - } + extensions_v8::SearchBoxExtension::DispatchMostVisitedChanged( + render_frame()->GetWebFrame()); } -void SearchBox::OnSetInputInProgress(bool is_input_in_progress) { +void SearchBox::SetInputInProgress(bool is_input_in_progress) { if (is_input_in_progress_ != is_input_in_progress) { is_input_in_progress_ = is_input_in_progress; - DVLOG(1) << render_view() << " OnSetInputInProgress"; - if (render_view()->GetWebView() && - render_view()->GetWebView()->mainFrame()) { - if (is_input_in_progress_) { - extensions_v8::SearchBoxExtension::DispatchInputStart( - render_view()->GetWebView()->mainFrame()); - } else { - extensions_v8::SearchBoxExtension::DispatchInputCancel( - render_view()->GetWebView()->mainFrame()); - } + DVLOG(1) << render_frame() << " SetInputInProgress"; + if (is_input_in_progress_) { + extensions_v8::SearchBoxExtension::DispatchInputStart( + render_frame()->GetWebFrame()); + } else { + extensions_v8::SearchBoxExtension::DispatchInputCancel( + render_frame()->GetWebFrame()); } } } -void SearchBox::OnSetSuggestionToPrefetch(const InstantSuggestion& suggestion) { +void SearchBox::SetSuggestionToPrefetch(const InstantSuggestion& suggestion) { suggestion_ = suggestion; - if (render_view()->GetWebView() && render_view()->GetWebView()->mainFrame()) { - DVLOG(1) << render_view() << " OnSetSuggestionToPrefetch"; - extensions_v8::SearchBoxExtension::DispatchSuggestionChange( - render_view()->GetWebView()->mainFrame()); - } + DVLOG(1) << render_frame() << " SetSuggestionToPrefetch"; + extensions_v8::SearchBoxExtension::DispatchSuggestionChange( + render_frame()->GetWebFrame()); } -void SearchBox::OnSubmit(const base::string16& query, - const EmbeddedSearchRequestParams& params) { +void SearchBox::Submit(const base::string16& query, + const EmbeddedSearchRequestParams& params) { query_ = query; embedded_search_request_params_ = params; - if (render_view()->GetWebView() && render_view()->GetWebView()->mainFrame()) { - DVLOG(1) << render_view() << " OnSubmit"; - extensions_v8::SearchBoxExtension::DispatchSubmit( - render_view()->GetWebView()->mainFrame()); - } + DVLOG(1) << render_frame() << " Submit"; + extensions_v8::SearchBoxExtension::DispatchSubmit( + render_frame()->GetWebFrame()); if (!query.empty()) Reset(); } -void SearchBox::OnThemeChanged(const ThemeBackgroundInfo& theme_info) { +void SearchBox::ThemeChanged(const ThemeBackgroundInfo& theme_info) { // Do not send duplicate notifications. if (theme_info_ == theme_info) return; theme_info_ = theme_info; - if (render_view()->GetWebView() && render_view()->GetWebView()->mainFrame()) { - extensions_v8::SearchBoxExtension::DispatchThemeChange( - render_view()->GetWebView()->mainFrame()); - } + extensions_v8::SearchBoxExtension::DispatchThemeChange( + render_frame()->GetWebFrame()); } GURL SearchBox::GetURLForMostVisitedItem(InstantRestrictedID item_id) const { @@ -510,6 +450,10 @@ return GetMostVisitedItemWithID(item_id, &item) ? item.url : GURL(); } +void SearchBox::Bind(chrome::mojom::SearchBoxAssociatedRequest request) { + binding_.Bind(std::move(request)); +} + void SearchBox::Reset() { query_.clear(); embedded_search_request_params_ = EmbeddedSearchRequestParams();
diff --git a/chrome/renderer/searchbox/searchbox.h b/chrome/renderer/searchbox/searchbox.h index 50e89ec..5781157 100644 --- a/chrome/renderer/searchbox/searchbox.h +++ b/chrome/renderer/searchbox/searchbox.h
@@ -10,22 +10,21 @@ #include "base/macros.h" #include "base/strings/string16.h" +#include "chrome/common/instant.mojom.h" #include "chrome/common/search/instant_types.h" #include "chrome/common/search/ntp_logging_events.h" #include "chrome/renderer/instant_restricted_id_cache.h" #include "components/ntp_tiles/ntp_tile_source.h" #include "components/omnibox/common/omnibox_focus_state.h" -#include "content/public/renderer/render_view_observer.h" -#include "content/public/renderer/render_view_observer_tracker.h" +#include "content/public/renderer/render_frame_observer.h" +#include "content/public/renderer/render_frame_observer_tracker.h" +#include "mojo/public/cpp/bindings/associated_binding.h" #include "ui/base/window_open_disposition.h" #include "url/gurl.h" -namespace content { -class RenderView; -} - -class SearchBox : public content::RenderViewObserver, - public content::RenderViewObserverTracker<SearchBox> { +class SearchBox : public content::RenderFrameObserver, + public content::RenderFrameObserverTracker<SearchBox>, + public chrome::mojom::SearchBox { public: enum ImageSourceType { NONE = -1, @@ -48,27 +47,27 @@ const = 0; }; - explicit SearchBox(content::RenderView* render_view); + explicit SearchBox(content::RenderFrame* render_frame); ~SearchBox() override; - // Sends ChromeViewHostMsg_LogEvent to the browser. + // Sends LogEvent to the browser. void LogEvent(NTPLoggingEventType event); - // Sends ChromeViewHostMsg_LogMostVisitedImpression to the browser. + // Sends LogMostVisitedImpression to the browser. void LogMostVisitedImpression(int position, ntp_tiles::NTPTileSource tile_source); - // Sends ChromeViewHostMsg_LogMostVisitedNavigation to the browser. + // Sends LogMostVisitedNavigation to the browser. void LogMostVisitedNavigation(int position, ntp_tiles::NTPTileSource tile_source); - // Sends ChromeViewHostMsg_ChromeIdentityCheck to the browser. + // Sends ChromeIdentityCheck to the browser. void CheckIsUserSignedInToChromeAs(const base::string16& identity); - // Sends ChromeViewHostMsg_HistorySyncCheck to the browser. + // Sends HistorySyncCheck to the browser. void CheckIsUserSyncingHistory(); - // Sends ChromeViewHostMsg_SearchBoxDeleteMostVisitedItem to the browser. + // Sends SearchBoxDeleteMostVisitedItem to the browser. void DeleteMostVisitedItem(InstantRestrictedID most_visited_item_id); // Generates the image URL of |type| for the most visited item specified in @@ -110,7 +109,7 @@ bool GetMostVisitedItemWithID(InstantRestrictedID most_visited_item_id, InstantMostVisitedItem* item) const; - // Sends ChromeViewHostMsg_SearchBoxPaste to the browser. + // Sends SearchBoxPaste to the browser. void Paste(const base::string16& text); const ThemeBackgroundInfo& GetThemeBackgroundInfo(); @@ -122,11 +121,10 @@ // Sends ChromeViewHostMsg_StopCapturingKeyStrokes to the browser. void StopCapturingKeyStrokes(); - // Sends ChromeViewHostMsg_SearchBoxUndoAllMostVisitedDeletions to the - // browser. + // Sends SearchBoxUndoAllMostVisitedDeletions to the browser. void UndoAllMostVisitedDeletions(); - // Sends ChromeViewHostMsg_SearchBoxUndoMostVisitedDeletion to the browser. + // Sends SearchBoxUndoMostVisitedDeletion to the browser. void UndoMostVisitedDeletion(InstantRestrictedID most_visited_item_id); bool is_focused() const { return is_focused_; } @@ -136,24 +134,24 @@ const InstantSuggestion& suggestion() const { return suggestion_; } private: - // Overridden from content::RenderViewObserver: - bool OnMessageReceived(const IPC::Message& message) override; + // Overridden from content::RenderFrameObserver: void OnDestruct() override; - void OnSetPageSequenceNumber(int page_seq_no); - void OnChromeIdentityCheckResult(const base::string16& identity, - bool identity_match); - void OnDetermineIfPageSupportsInstant(); - void OnFocusChanged(OmniboxFocusState new_focus_state, - OmniboxFocusChangeReason reason); - void OnHistorySyncCheckResult(bool sync_history); - void OnMostVisitedChanged( - const std::vector<InstantMostVisitedItem>& items); - void OnSetInputInProgress(bool input_in_progress); - void OnSetSuggestionToPrefetch(const InstantSuggestion& suggestion); - void OnSubmit(const base::string16& query, - const EmbeddedSearchRequestParams& params); - void OnThemeChanged(const ThemeBackgroundInfo& theme_info); + // Overridden from chrome::mojom::SearchBox: + void SetPageSequenceNumber(int page_seq_no) override; + void ChromeIdentityCheckResult(const base::string16& identity, + bool identity_match) override; + void DetermineIfPageSupportsInstant() override; + void FocusChanged(OmniboxFocusState new_focus_state, + OmniboxFocusChangeReason reason) override; + void HistorySyncCheckResult(bool sync_history) override; + void MostVisitedChanged( + const std::vector<InstantMostVisitedItem>& items) override; + void SetInputInProgress(bool input_in_progress) override; + void SetSuggestionToPrefetch(const InstantSuggestion& suggestion) override; + void Submit(const base::string16& query, + const EmbeddedSearchRequestParams& params) override; + void ThemeChanged(const ThemeBackgroundInfo& theme_info) override; // Returns the current zoom factor of the render view or 1 on failure. double GetZoom() const; @@ -164,6 +162,8 @@ // Returns the URL of the Most Visited item specified by the |item_id|. GURL GetURLForMostVisitedItem(InstantRestrictedID item_id) const; + void Bind(chrome::mojom::SearchBoxAssociatedRequest request); + int page_seq_no_; bool is_focused_; bool is_input_in_progress_; @@ -173,6 +173,8 @@ base::string16 query_; EmbeddedSearchRequestParams embedded_search_request_params_; InstantSuggestion suggestion_; + chrome::mojom::InstantAssociatedPtr instant_service_; + mojo::AssociatedBinding<chrome::mojom::SearchBox> binding_; DISALLOW_COPY_AND_ASSIGN(SearchBox); };
diff --git a/chrome/renderer/searchbox/searchbox_extension.cc b/chrome/renderer/searchbox/searchbox_extension.cc index d86f66e..dd6221e4 100644 --- a/chrome/renderer/searchbox/searchbox_extension.cc +++ b/chrome/renderer/searchbox/searchbox_extension.cc
@@ -23,6 +23,7 @@ #include "chrome/renderer/searchbox/searchbox.h" #include "components/crx_file/id_util.h" #include "components/ntp_tiles/ntp_tile_source.h" +#include "content/public/renderer/render_frame.h" #include "content/public/renderer/render_view.h" #include "third_party/WebKit/public/platform/WebURLRequest.h" #include "third_party/WebKit/public/web/WebDocument.h" @@ -211,27 +212,24 @@ return obj; } -// Returns the render view for the current JS context if it matches |origin|, -// otherwise returns NULL. Used to restrict methods that access suggestions and -// most visited data to pages with origin chrome-search://most-visited and -// chrome-search://suggestions. -content::RenderView* GetRenderViewWithCheckedOrigin(const GURL& origin) { +// Returns the main frame of the render frame for the current JS context if it +// matches |origin|, otherwise returns NULL. Used to restrict methods that +// access suggestions and most visited data to pages with origin +// chrome-search://most-visited and chrome-search://suggestions. +content::RenderFrame* GetRenderFrameWithCheckedOrigin(const GURL& origin) { blink::WebLocalFrame* webframe = blink::WebLocalFrame::frameForCurrentContext(); if (!webframe) return NULL; - blink::WebView* webview = webframe->view(); - if (!webview) - return NULL; // Can happen during closing. - content::RenderView* render_view = content::RenderView::FromWebView(webview); - if (!render_view) + auto* main_frame = content::RenderFrame::FromWebFrame(webframe->localRoot()); + if (!main_frame || !main_frame->IsMainFrame()) return NULL; GURL url(webframe->document().url()); if (url.GetOrigin() != origin.GetOrigin()) return NULL; - return render_view; + return main_frame; } } // namespace @@ -398,8 +396,8 @@ v8::Isolate*, v8::Local<v8::String> name) override; - // Helper function to find the RenderView. May return NULL. - static content::RenderView* GetRenderView(); + // Helper function to find the main RenderFrame. May return NULL. + static content::RenderFrame* GetRenderFrame(); // Sends a Chrome identity check to the browser. static void CheckIsUserSignedInToChromeAs( @@ -418,7 +416,7 @@ const v8::FunctionCallbackInfo<v8::Value>& args); // Gets the raw data for a most visited item including its raw URL. - // GetRenderViewWithCheckedOrigin() enforces that only code in the origin + // GetRenderFrameWithCheckedOrigin() enforces that only code in the origin // chrome-search://most-visited can call this function. static void GetMostVisitedItemData( const v8::FunctionCallbackInfo<v8::Value>& args); @@ -629,70 +627,74 @@ } // static -content::RenderView* SearchBoxExtensionWrapper::GetRenderView() { +content::RenderFrame* SearchBoxExtensionWrapper::GetRenderFrame() { blink::WebLocalFrame* webframe = blink::WebLocalFrame::frameForCurrentContext(); if (!webframe) return NULL; - blink::WebView* webview = webframe->view(); - if (!webview) return NULL; // can happen during closing + auto* main_frame = content::RenderFrame::FromWebFrame(webframe->localRoot()); + if (!main_frame || !main_frame->IsMainFrame()) + return NULL; - return content::RenderView::FromWebView(webview); + return main_frame; } // static void SearchBoxExtensionWrapper::CheckIsUserSignedInToChromeAs( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderView(); - if (!render_view) return; + content::RenderFrame* render_frame = GetRenderFrame(); + if (!render_frame) + return; if (!args.Length() || args[0]->IsUndefined()) { ThrowInvalidParameters(args); return; } - DVLOG(1) << render_view << " CheckIsUserSignedInToChromeAs"; + DVLOG(1) << render_frame << " CheckIsUserSignedInToChromeAs"; - SearchBox::Get(render_view)->CheckIsUserSignedInToChromeAs( - V8ValueToUTF16(args[0])); + SearchBox::Get(render_frame) + ->CheckIsUserSignedInToChromeAs(V8ValueToUTF16(args[0])); } // static void SearchBoxExtensionWrapper::CheckIsUserSyncingHistory( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderView(); - if (!render_view) return; + content::RenderFrame* render_frame = GetRenderFrame(); + if (!render_frame) + return; - DVLOG(1) << render_view << " CheckIsUserSyncingHistory"; - SearchBox::Get(render_view)->CheckIsUserSyncingHistory(); + DVLOG(1) << render_frame << " CheckIsUserSyncingHistory"; + SearchBox::Get(render_frame)->CheckIsUserSyncingHistory(); } // static void SearchBoxExtensionWrapper::DeleteMostVisitedItem( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderView(); - if (!render_view) return; + content::RenderFrame* render_frame = GetRenderFrame(); + if (!render_frame) + return; if (!args.Length()) { ThrowInvalidParameters(args); return; } - DVLOG(1) << render_view + DVLOG(1) << render_frame << " DeleteMostVisitedItem: " << args[0]->ToInteger()->Value(); - SearchBox::Get(render_view)-> - DeleteMostVisitedItem(args[0]->ToInteger()->Value()); + SearchBox::Get(render_frame) + ->DeleteMostVisitedItem(args[0]->ToInteger()->Value()); } // static void SearchBoxExtensionWrapper::GetMostVisitedItems( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderView(); - if (!render_view) + content::RenderFrame* render_frame = GetRenderFrame(); + if (!render_frame) return; - DVLOG(1) << render_view << " GetMostVisitedItems"; + DVLOG(1) << render_frame << " GetMostVisitedItems"; - const SearchBox* search_box = SearchBox::Get(render_view); + const SearchBox* search_box = SearchBox::Get(render_frame); std::vector<InstantMostVisitedItemIDPair> instant_mv_items; search_box->GetMostVisitedItems(&instant_mv_items); @@ -700,11 +702,10 @@ v8::Local<v8::Array> v8_mv_items = v8::Array::New(isolate, instant_mv_items.size()); for (size_t i = 0; i < instant_mv_items.size(); ++i) { - v8_mv_items->Set(i, - GenerateMostVisitedItem(isolate, - render_view->GetRoutingID(), - instant_mv_items[i].first, - instant_mv_items[i].second)); + v8_mv_items->Set( + i, GenerateMostVisitedItem( + isolate, render_frame->GetRenderView()->GetRoutingID(), + instant_mv_items[i].first, instant_mv_items[i].second)); } args.GetReturnValue().Set(v8_mv_items); } @@ -712,9 +713,10 @@ // static void SearchBoxExtensionWrapper::GetMostVisitedItemData( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderViewWithCheckedOrigin( + content::RenderFrame* render_frame = GetRenderFrameWithCheckedOrigin( GURL(chrome::kChromeSearchMostVisitedUrl)); - if (!render_view) return; + if (!render_frame) + return; // Need an rid argument. if (!args.Length() || !args[0]->IsNumber()) { @@ -722,25 +724,27 @@ return; } - DVLOG(1) << render_view << " GetMostVisitedItem"; + DVLOG(1) << render_frame << " GetMostVisitedItem"; InstantRestrictedID restricted_id = args[0]->IntegerValue(); InstantMostVisitedItem mv_item; - if (!SearchBox::Get(render_view)->GetMostVisitedItemWithID( - restricted_id, &mv_item)) { + if (!SearchBox::Get(render_frame) + ->GetMostVisitedItemWithID(restricted_id, &mv_item)) { return; } v8::Isolate* isolate = args.GetIsolate(); args.GetReturnValue().Set(GenerateMostVisitedItem( - isolate, render_view->GetRoutingID(), restricted_id, mv_item)); + isolate, render_frame->GetRenderView()->GetRoutingID(), restricted_id, + mv_item)); } // static void SearchBoxExtensionWrapper::GetQuery( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderView(); - if (!render_view) return; - const base::string16& query = SearchBox::Get(render_view)->query(); - DVLOG(1) << render_view << " GetQuery: '" << query << "'"; + content::RenderFrame* render_frame = GetRenderFrame(); + if (!render_frame) + return; + const base::string16& query = SearchBox::Get(render_frame)->query(); + DVLOG(1) << render_frame << " GetQuery: '" << query << "'"; v8::Isolate* isolate = args.GetIsolate(); args.GetReturnValue().Set(UTF16ToV8String(isolate, query)); } @@ -754,11 +758,12 @@ // static void SearchBoxExtensionWrapper::GetSearchRequestParams( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderView(); - if (!render_view) return; + content::RenderFrame* render_frame = GetRenderFrame(); + if (!render_frame) + return; const EmbeddedSearchRequestParams& params = - SearchBox::Get(render_view)->GetEmbeddedSearchRequestParams(); + SearchBox::Get(render_frame)->GetEmbeddedSearchRequestParams(); v8::Isolate* isolate = args.GetIsolate(); v8::Local<v8::Object> data = v8::Object::New(isolate); if (!params.search_query.empty()) { @@ -787,11 +792,12 @@ // static void SearchBoxExtensionWrapper::GetSuggestionToPrefetch( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderView(); - if (!render_view) return; + content::RenderFrame* render_frame = GetRenderFrame(); + if (!render_frame) + return; const InstantSuggestion& suggestion = - SearchBox::Get(render_view)->suggestion(); + SearchBox::Get(render_frame)->suggestion(); v8::Isolate* isolate = args.GetIsolate(); v8::Local<v8::Object> data = v8::Object::New(isolate); data->Set(v8::String::NewFromUtf8(isolate, "text"), @@ -804,12 +810,13 @@ // static void SearchBoxExtensionWrapper::GetThemeBackgroundInfo( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderView(); - if (!render_view) return; + content::RenderFrame* render_frame = GetRenderFrame(); + if (!render_frame) + return; - DVLOG(1) << render_view << " GetThemeBackgroundInfo"; + DVLOG(1) << render_frame << " GetThemeBackgroundInfo"; const ThemeBackgroundInfo& theme_info = - SearchBox::Get(render_view)->GetThemeBackgroundInfo(); + SearchBox::Get(render_frame)->GetThemeBackgroundInfo(); v8::Isolate* isolate = args.GetIsolate(); v8::Local<v8::Object> info = v8::Object::New(isolate); @@ -959,152 +966,162 @@ // static void SearchBoxExtensionWrapper::IsFocused( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderView(); - if (!render_view) return; + content::RenderFrame* render_frame = GetRenderFrame(); + if (!render_frame) + return; - bool is_focused = SearchBox::Get(render_view)->is_focused(); - DVLOG(1) << render_view << " IsFocused: " << is_focused; + bool is_focused = SearchBox::Get(render_frame)->is_focused(); + DVLOG(1) << render_frame << " IsFocused: " << is_focused; args.GetReturnValue().Set(is_focused); } // static void SearchBoxExtensionWrapper::IsInputInProgress( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderView(); - if (!render_view) return; + content::RenderFrame* render_frame = GetRenderFrame(); + if (!render_frame) + return; bool is_input_in_progress = - SearchBox::Get(render_view)->is_input_in_progress(); - DVLOG(1) << render_view << " IsInputInProgress: " << is_input_in_progress; + SearchBox::Get(render_frame)->is_input_in_progress(); + DVLOG(1) << render_frame << " IsInputInProgress: " << is_input_in_progress; args.GetReturnValue().Set(is_input_in_progress); } // static void SearchBoxExtensionWrapper::IsKeyCaptureEnabled( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderView(); - if (!render_view) return; + content::RenderFrame* render_frame = GetRenderFrame(); + if (!render_frame) + return; - args.GetReturnValue().Set(SearchBox::Get(render_view)-> - is_key_capture_enabled()); + args.GetReturnValue().Set( + SearchBox::Get(render_frame)->is_key_capture_enabled()); } // static void SearchBoxExtensionWrapper::LogEvent( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderViewWithCheckedOrigin( + content::RenderFrame* render_frame = GetRenderFrameWithCheckedOrigin( GURL(chrome::kChromeSearchMostVisitedUrl)); - if (!render_view) return; + if (!render_frame) + return; if (!args.Length() || !args[0]->IsNumber()) { ThrowInvalidParameters(args); return; } - DVLOG(1) << render_view << " LogEvent"; + DVLOG(1) << render_frame << " LogEvent"; if (args[0]->Uint32Value() <= NTP_EVENT_TYPE_LAST) { NTPLoggingEventType event = static_cast<NTPLoggingEventType>(args[0]->Uint32Value()); - SearchBox::Get(render_view)->LogEvent(event); + SearchBox::Get(render_frame)->LogEvent(event); } } // static void SearchBoxExtensionWrapper::LogMostVisitedImpression( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderViewWithCheckedOrigin( + content::RenderFrame* render_frame = GetRenderFrameWithCheckedOrigin( GURL(chrome::kChromeSearchMostVisitedUrl)); - if (!render_view) return; + if (!render_frame) + return; if (args.Length() < 2 || !args[0]->IsNumber() || !args[1]->IsNumber()) { ThrowInvalidParameters(args); return; } - DVLOG(1) << render_view << " LogMostVisitedImpression"; + DVLOG(1) << render_frame << " LogMostVisitedImpression"; if (args[1]->Uint32Value() <= static_cast<int>(ntp_tiles::NTPTileSource::LAST)) { ntp_tiles::NTPTileSource tile_source = static_cast<ntp_tiles::NTPTileSource>(args[1]->Uint32Value()); - SearchBox::Get(render_view)->LogMostVisitedImpression( - args[0]->IntegerValue(), tile_source); + SearchBox::Get(render_frame) + ->LogMostVisitedImpression(args[0]->IntegerValue(), tile_source); } } // static void SearchBoxExtensionWrapper::LogMostVisitedNavigation( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderViewWithCheckedOrigin( + content::RenderFrame* render_frame = GetRenderFrameWithCheckedOrigin( GURL(chrome::kChromeSearchMostVisitedUrl)); - if (!render_view) return; + if (!render_frame) + return; if (args.Length() < 2 || !args[0]->IsNumber() || !args[1]->IsNumber()) { ThrowInvalidParameters(args); return; } - DVLOG(1) << render_view << " LogMostVisitedNavigation"; + DVLOG(1) << render_frame << " LogMostVisitedNavigation"; if (args[1]->Uint32Value() <= static_cast<int>(ntp_tiles::NTPTileSource::LAST)) { ntp_tiles::NTPTileSource tile_source = static_cast<ntp_tiles::NTPTileSource>(args[1]->Uint32Value()); - SearchBox::Get(render_view)->LogMostVisitedNavigation( - args[0]->IntegerValue(), tile_source); + SearchBox::Get(render_frame) + ->LogMostVisitedNavigation(args[0]->IntegerValue(), tile_source); } } // static void SearchBoxExtensionWrapper::Paste( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderView(); - if (!render_view) return; + content::RenderFrame* render_frame = GetRenderFrame(); + if (!render_frame) + return; base::string16 text; if (!args[0]->IsUndefined()) text = V8ValueToUTF16(args[0]); - DVLOG(1) << render_view << " Paste: " << text; - SearchBox::Get(render_view)->Paste(text); + DVLOG(1) << render_frame << " Paste: " << text; + SearchBox::Get(render_frame)->Paste(text); } // static void SearchBoxExtensionWrapper::StartCapturingKeyStrokes( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderView(); - if (!render_view) return; + content::RenderFrame* render_frame = GetRenderFrame(); + if (!render_frame) + return; - DVLOG(1) << render_view << " StartCapturingKeyStrokes"; - SearchBox::Get(render_view)->StartCapturingKeyStrokes(); + DVLOG(1) << render_frame << " StartCapturingKeyStrokes"; + SearchBox::Get(render_frame)->StartCapturingKeyStrokes(); } // static void SearchBoxExtensionWrapper::StopCapturingKeyStrokes( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderView(); - if (!render_view) return; + content::RenderFrame* render_frame = GetRenderFrame(); + if (!render_frame) + return; - DVLOG(1) << render_view << " StopCapturingKeyStrokes"; - SearchBox::Get(render_view)->StopCapturingKeyStrokes(); + DVLOG(1) << render_frame << " StopCapturingKeyStrokes"; + SearchBox::Get(render_frame)->StopCapturingKeyStrokes(); } // static void SearchBoxExtensionWrapper::UndoAllMostVisitedDeletions( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderView(); - if (!render_view) return; + content::RenderFrame* render_frame = GetRenderFrame(); + if (!render_frame) + return; - DVLOG(1) << render_view << " UndoAllMostVisitedDeletions"; - SearchBox::Get(render_view)->UndoAllMostVisitedDeletions(); + DVLOG(1) << render_frame << " UndoAllMostVisitedDeletions"; + SearchBox::Get(render_frame)->UndoAllMostVisitedDeletions(); } // static void SearchBoxExtensionWrapper::UndoMostVisitedDeletion( const v8::FunctionCallbackInfo<v8::Value>& args) { - content::RenderView* render_view = GetRenderView(); - if (!render_view) { + content::RenderFrame* render_frame = GetRenderFrame(); + if (!render_frame) { return; } if (!args.Length()) { @@ -1112,8 +1129,8 @@ return; } - DVLOG(1) << render_view << " UndoMostVisitedDeletion"; - SearchBox::Get(render_view) + DVLOG(1) << render_frame << " UndoMostVisitedDeletion"; + SearchBox::Get(render_frame) ->UndoMostVisitedDeletion(args[0]->ToInteger()->Value()); }
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index f7c7431c..d5d16ed 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn
@@ -3440,6 +3440,7 @@ ":test_support", ":test_support_unit", "//base/test:test_support", + "//chrome/common:test_support", "//components/browser_sync:test_support", "//components/content_settings/core/test:test_support", "//components/resources",
diff --git a/chrome/typemaps.gni b/chrome/typemaps.gni new file mode 100644 index 0000000..9a20fe5 --- /dev/null +++ b/chrome/typemaps.gni
@@ -0,0 +1,5 @@ +# Copyright 2016 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +typemaps = [ "//chrome/common/instant.typemap" ]
diff --git a/components/arc/ime/arc_ime_service.cc b/components/arc/ime/arc_ime_service.cc index 7d49a17..e2ffb34 100644 --- a/components/arc/ime/arc_ime_service.cc +++ b/components/arc/ime/arc_ime_service.cc
@@ -21,17 +21,36 @@ namespace arc { -ArcImeService::ArcWindowDetector::~ArcWindowDetector() = default; +namespace { -bool ArcImeService::ArcWindowDetector::IsArcWindow( - const aura::Window* window) const { - return exo::Surface::AsSurface(window); -} +class ArcWindowDelegateImpl : public ArcImeService::ArcWindowDelegate { + public: + explicit ArcWindowDelegateImpl(ArcImeService* ime_service) + : ime_service_(ime_service) {} -bool ArcImeService::ArcWindowDetector::IsArcTopLevelWindow( - const aura::Window* window) const { - return exo::ShellSurface::GetMainSurface(window); -} + ~ArcWindowDelegateImpl() override = default; + + bool IsArcWindow( + const aura::Window* window) const override { + return exo::Surface::AsSurface(window) || + exo::ShellSurface::GetMainSurface(window); + } + + void RegisterFocusObserver() override { + exo::WMHelper::GetInstance()->AddFocusObserver(ime_service_); + } + + void UnregisterFocusObserver() override { + exo::WMHelper::GetInstance()->RemoveFocusObserver(ime_service_); + } + + private: + ArcImeService* const ime_service_; + + DISALLOW_COPY_AND_ASSIGN(ArcWindowDelegateImpl); +}; + +} // anonymous namespace //////////////////////////////////////////////////////////////////////////////// // ArcImeService main implementation: @@ -39,7 +58,7 @@ ArcImeService::ArcImeService(ArcBridgeService* bridge_service) : ArcService(bridge_service), ime_bridge_(new ArcImeBridgeImpl(this, bridge_service)), - arc_window_detector_(new ArcWindowDetector()), + arc_window_delegate_(new ArcWindowDelegateImpl(this)), ime_type_(ui::TEXT_INPUT_TYPE_NONE), has_composition_text_(false), keyboard_controller_(nullptr), @@ -55,8 +74,8 @@ if (input_method) input_method->DetachTextInputClient(this); - if (is_focus_observer_installed_ && exo::WMHelper::GetInstance()) - exo::WMHelper::GetInstance()->RemoveFocusObserver(this); + if (is_focus_observer_installed_) + arc_window_delegate_->UnregisterFocusObserver(); aura::Env* env = aura::Env::GetInstanceDontCreate(); if (env) env->RemoveObserver(this); @@ -78,9 +97,9 @@ test_input_method_ = test_input_method; } -void ArcImeService::SetArcWindowDetectorForTesting( - std::unique_ptr<ArcWindowDetector> detector) { - arc_window_detector_ = std::move(detector); +void ArcImeService::SetArcWindowDelegateForTesting( + std::unique_ptr<ArcWindowDelegate> delegate) { + arc_window_delegate_= std::move(delegate); } ui::InputMethod* ArcImeService::GetInputMethod() { @@ -96,9 +115,9 @@ // Overridden from aura::EnvObserver: void ArcImeService::OnWindowInitialized(aura::Window* new_window) { - if (arc_window_detector_->IsArcWindow(new_window)) { + if (arc_window_delegate_->IsArcWindow(new_window)) { if (!is_focus_observer_installed_) { - exo::WMHelper::GetInstance()->AddFocusObserver(this); + arc_window_delegate_->RegisterFocusObserver(); is_focus_observer_installed_ = true; } } @@ -116,16 +135,12 @@ void ArcImeService::OnWindowFocused(aura::Window* gained_focus, aura::Window* lost_focus) { - // The Aura focus may or may not be on sub-window of the toplevel ARC++ frame. - // To handle all cases, judge the state by always climbing up to the toplevel. - gained_focus = gained_focus ? gained_focus->GetToplevelWindow() : nullptr; - lost_focus = lost_focus ? lost_focus->GetToplevelWindow() : nullptr; if (lost_focus == gained_focus) return; const bool detach = (lost_focus && focused_arc_window_.Contains(lost_focus)); const bool attach = - (gained_focus && arc_window_detector_->IsArcTopLevelWindow(gained_focus)); + (gained_focus && arc_window_delegate_->IsArcWindow(gained_focus)); // TODO(kinaba): Implicit dependency in GetInputMethod as described below is // confusing. Consider getting InputMethod directly from lost_ or gained_focus @@ -288,7 +303,11 @@ cursor_rect_, 1 / window->layer()->device_scale_factor()); // Add the offset of the window showing the ARC app. - converted.Offset(window->GetBoundsInScreen().OffsetFromOrigin()); + // TODO(yoshiki): Support for non-arc toplevel window. The following code do + // not work correctly with arc windows inside non-arc toplevel window (eg. + // notification). + converted.Offset( + window->GetToplevelWindow()->GetBoundsInScreen().OffsetFromOrigin()); return converted; }
diff --git a/components/arc/ime/arc_ime_service.h b/components/arc/ime/arc_ime_service.h index 12610a8b..86784b7 100644 --- a/components/arc/ime/arc_ime_service.h +++ b/components/arc/ime/arc_ime_service.h
@@ -45,11 +45,12 @@ explicit ArcImeService(ArcBridgeService* bridge_service); ~ArcImeService() override; - class ArcWindowDetector { + class ArcWindowDelegate { public: - virtual ~ArcWindowDetector(); - virtual bool IsArcTopLevelWindow(const aura::Window* window) const; - virtual bool IsArcWindow(const aura::Window* window) const; + virtual ~ArcWindowDelegate() {} + virtual bool IsArcWindow(const aura::Window* window) const = 0; + virtual void RegisterFocusObserver() = 0; + virtual void UnregisterFocusObserver() = 0; }; // Injects the custom IPC bridge object for testing purpose only. @@ -58,9 +59,9 @@ // Injects the custom IME for testing purpose only. void SetInputMethodForTesting(ui::InputMethod* test_input_method); - // Injects the custom detector for ARC windows, for testing purpose only. - void SetArcWindowDetectorForTesting( - std::unique_ptr<ArcWindowDetector> detector); + // Injects the custom delegate for ARC windows, for testing purpose only. + void SetArcWindowDelegateForTesting( + std::unique_ptr<ArcWindowDelegate> delegate); // Overridden from aura::EnvObserver: void OnWindowInitialized(aura::Window* new_window) override; @@ -118,7 +119,7 @@ ui::InputMethod* GetInputMethod(); std::unique_ptr<ArcImeBridge> ime_bridge_; - std::unique_ptr<ArcWindowDetector> arc_window_detector_; + std::unique_ptr<ArcWindowDelegate> arc_window_delegate_; ui::TextInputType ime_type_; gfx::Rect cursor_rect_; bool has_composition_text_;
diff --git a/components/arc/ime/arc_ime_service_unittest.cc b/components/arc/ime/arc_ime_service_unittest.cc index 086bea93..1b2ea0e3 100644 --- a/components/arc/ime/arc_ime_service_unittest.cc +++ b/components/arc/ime/arc_ime_service_unittest.cc
@@ -98,23 +98,25 @@ // Helper class for testing the window focus tracking feature of ArcImeService, // not depending on the full setup of Exo and Ash. -class FakeArcWindowDetector : public ArcImeService::ArcWindowDetector { +class FakeArcWindowDelegate : public ArcImeService::ArcWindowDelegate { public: - FakeArcWindowDetector() : next_id_(0) {} + FakeArcWindowDelegate() : next_id_(0) {} - bool IsArcWindow(const aura::Window* window) const override { return false; } - bool IsArcTopLevelWindow(const aura::Window* window) const override { - return arc_toplevel_window_id_.count(window->id()); + bool IsArcWindow(const aura::Window* window) const override { + return arc_window_id_.count(window->id()); } - std::unique_ptr<aura::Window> CreateFakeArcTopLevelWindow() { + void RegisterFocusObserver() override {} + void UnregisterFocusObserver() override {} + + std::unique_ptr<aura::Window> CreateFakeArcWindow() { const int id = next_id_++; - arc_toplevel_window_id_.insert(id); + arc_window_id_.insert(id); return base::WrapUnique(aura::test::CreateTestWindowWithDelegate( &dummy_delegate_, id, gfx::Rect(), nullptr)); } - std::unique_ptr<aura::Window> CreateFakeNonArcTopLevelWindow() { + std::unique_ptr<aura::Window> CreateFakeNonArcWindow() { const int id = next_id_++; return base::WrapUnique(aura::test::CreateTestWindowWithDelegate( &dummy_delegate_, id, gfx::Rect(), nullptr)); @@ -123,14 +125,14 @@ private: aura::test::TestWindowDelegate dummy_delegate_; int next_id_; - std::set<int> arc_toplevel_window_id_; + std::set<int> arc_window_id_; }; } // namespace class ArcImeServiceTest : public testing::Test { public: - ArcImeServiceTest() {} + ArcImeServiceTest() = default; protected: std::unique_ptr<ArcBridgeService> arc_bridge_service_; @@ -138,7 +140,7 @@ std::unique_ptr<ArcImeService> instance_; FakeArcImeBridge* fake_arc_ime_bridge_; // Owned by |instance_| - FakeArcWindowDetector* fake_window_detector_; // Owned by |instance_| + FakeArcWindowDelegate* fake_window_delegate_; // Owned by |instance_| std::unique_ptr<aura::Window> arc_win_; private: @@ -151,15 +153,15 @@ fake_input_method_ = base::MakeUnique<FakeInputMethod>(); instance_->SetInputMethodForTesting(fake_input_method_.get()); - fake_window_detector_ = new FakeArcWindowDetector(); - instance_->SetArcWindowDetectorForTesting( - base::WrapUnique(fake_window_detector_)); - arc_win_ = fake_window_detector_->CreateFakeArcTopLevelWindow(); + fake_window_delegate_ = new FakeArcWindowDelegate(); + instance_->SetArcWindowDelegateForTesting( + base::WrapUnique(fake_window_delegate_)); + arc_win_ = fake_window_delegate_->CreateFakeArcWindow(); } void TearDown() override { arc_win_.reset(); - fake_window_detector_ = nullptr; + fake_window_delegate_ = nullptr; fake_arc_ime_bridge_ = nullptr; instance_.reset(); arc_bridge_service_.reset(); @@ -233,9 +235,9 @@ TEST_F(ArcImeServiceTest, WindowFocusTracking) { std::unique_ptr<aura::Window> arc_win2 = - fake_window_detector_->CreateFakeArcTopLevelWindow(); + fake_window_delegate_->CreateFakeArcWindow(); std::unique_ptr<aura::Window> nonarc_win = - fake_window_detector_->CreateFakeNonArcTopLevelWindow(); + fake_window_delegate_->CreateFakeNonArcWindow(); // ARC window is focused. ArcImeService is set as the text input client. instance_->OnWindowFocused(arc_win_.get(), nullptr);
diff --git a/components/arc/metrics/arc_metrics_service.h b/components/arc/metrics/arc_metrics_service.h index 7ad1a0be3..df59122 100644 --- a/components/arc/metrics/arc_metrics_service.h +++ b/components/arc/metrics/arc_metrics_service.h
@@ -64,6 +64,8 @@ void OnInstanceClosed() override; ArcMetricsService* arc_metrics_service_; + + DISALLOW_COPY_AND_ASSIGN(ProcessObserver); }; mojo::Binding<mojom::MetricsHost> binding_;
diff --git a/content/browser/android/content_view_core_impl.cc b/content/browser/android/content_view_core_impl.cc index 6dc7a550..9537ec6 100644 --- a/content/browser/android/content_view_core_impl.cc +++ b/content/browser/android/content_view_core_impl.cc
@@ -1300,7 +1300,7 @@ int composition_start, int composition_end, bool show_ime_if_needed, - bool is_non_ime_change) { + bool reply_to_request) { JNIEnv* env = AttachCurrentThread(); ScopedJavaLocalRef<jobject> obj = java_ref_.get(env); if (obj.is_null()) @@ -1310,8 +1310,7 @@ Java_ContentViewCore_updateImeAdapter( env, obj, native_ime_adapter, text_input_type, text_input_flags, text_input_mode, jstring_text, selection_start, selection_end, - composition_start, composition_end, show_ime_if_needed, - is_non_ime_change); + composition_start, composition_end, show_ime_if_needed, reply_to_request); } void ContentViewCoreImpl::SetAccessibilityEnabled(
diff --git a/content/browser/android/content_view_core_impl.h b/content/browser/android/content_view_core_impl.h index f76b041..18d55d4 100644 --- a/content/browser/android/content_view_core_impl.h +++ b/content/browser/android/content_view_core_impl.h
@@ -334,7 +334,7 @@ int composition_start, int composition_end, bool show_ime_if_needed, - bool is_non_ime_change); + bool reply_to_request); void OnBackgroundColorChanged(SkColor color); bool HasFocus();
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc index b1dbc1a4..fc8f8812 100644 --- a/content/browser/frame_host/render_frame_host_impl.cc +++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -601,7 +601,7 @@ } else { // The channel may not be initialized in some tests environments. In this // case we set up a dummy interface provider. - mojo::GetDummyProxyForTesting(&remote_interfaces); + mojo::GetIsolatedProxy(&remote_interfaces); } remote_associated_interfaces_.reset(new AssociatedInterfaceProviderImpl( std::move(remote_interfaces)));
diff --git a/content/browser/renderer_host/render_widget_host_view_android.cc b/content/browser/renderer_host/render_widget_host_view_android.cc index 6d4b39c..a016d8f 100644 --- a/content/browser/renderer_host/render_widget_host_view_android.cc +++ b/content/browser/renderer_host/render_widget_host_view_android.cc
@@ -783,7 +783,7 @@ GetNativeImeAdapter(), static_cast<int>(state.type), state.flags, state.mode, state.value, state.selection_start, state.selection_end, state.composition_start, state.composition_end, state.show_ime_if_needed, - state.is_non_ime_change); + state.reply_to_request); } void RenderWidgetHostViewAndroid::OnImeCompositionRangeChanged(
diff --git a/content/common/text_input_state.cc b/content/common/text_input_state.cc index ec5fbfe9..f6044bf 100644 --- a/content/common/text_input_state.cc +++ b/content/common/text_input_state.cc
@@ -16,7 +16,7 @@ composition_end(-1), can_compose_inline(true), show_ime_if_needed(false), - is_non_ime_change(true) {} + reply_to_request(false) {} TextInputState::TextInputState(const TextInputState& other) = default;
diff --git a/content/common/text_input_state.h b/content/common/text_input_state.h index 0fdc32b..16b4994 100644 --- a/content/common/text_input_state.h +++ b/content/common/text_input_state.h
@@ -54,9 +54,8 @@ // TEXT_INPUT_TYPE_NONE). bool show_ime_if_needed; - // Whether this change is originated from non-IME (e.g., Javascript, - // Autofill). - bool is_non_ime_change; + // Whether or not this is a reply to a request from IME. + bool reply_to_request; }; } // namespace content
diff --git a/content/common/view_messages.h b/content/common/view_messages.h index 7bf0b14b..60e2313f 100644 --- a/content/common/view_messages.h +++ b/content/common/view_messages.h
@@ -286,7 +286,7 @@ IPC_STRUCT_TRAITS_MEMBER(composition_end) IPC_STRUCT_TRAITS_MEMBER(can_compose_inline) IPC_STRUCT_TRAITS_MEMBER(show_ime_if_needed) - IPC_STRUCT_TRAITS_MEMBER(is_non_ime_change) + IPC_STRUCT_TRAITS_MEMBER(reply_to_request) IPC_STRUCT_TRAITS_END() IPC_STRUCT_BEGIN(ViewHostMsg_CreateWorker_Params)
diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java index af892bf..9f97f23 100644 --- a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java +++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
@@ -1914,7 +1914,7 @@ private void updateImeAdapter(long nativeImeAdapterAndroid, int textInputType, int textInputFlags, int textInputMode, String text, int selectionStart, int selectionEnd, int compositionStart, int compositionEnd, boolean showImeIfNeeded, - boolean isNonImeChange) { + boolean replyToRequest) { try { TraceEvent.begin("ContentViewCore.updateImeAdapter"); boolean focusedNodeEditable = (textInputType != TextInputType.NONE); @@ -1924,7 +1924,7 @@ mImeAdapter.updateKeyboardVisibility( textInputType, textInputFlags, textInputMode, showImeIfNeeded); mImeAdapter.updateState(text, selectionStart, selectionEnd, compositionStart, - compositionEnd, isNonImeChange); + compositionEnd, replyToRequest); boolean editableToggled = (focusedNodeEditable != isFocusedNodeEditable()); mSelectionPopupController.updateSelectionState(focusedNodeEditable,
diff --git a/content/public/android/java/src/org/chromium/content/browser/input/ChromiumBaseInputConnection.java b/content/public/android/java/src/org/chromium/content/browser/input/ChromiumBaseInputConnection.java index eaedcac..4e10479 100644 --- a/content/public/android/java/src/org/chromium/content/browser/input/ChromiumBaseInputConnection.java +++ b/content/public/android/java/src/org/chromium/content/browser/input/ChromiumBaseInputConnection.java
@@ -46,10 +46,10 @@ * composition. * @param compositionEnd The character offset of the composition end, or -1 if there is no * selection. - * @param isNonImeChange True when the update was caused by non-IME (e.g. Javascript). + * @param replyToRequest True when the update was made in a reply to IME's request. */ void updateStateOnUiThread(String text, int selectionStart, int selectionEnd, - int compositionStart, int compositionEnd, boolean singleLine, boolean isNonImeChange); + int compositionStart, int compositionEnd, boolean singleLine, boolean replyToRequest); /** * Send key event on UI thread.
diff --git a/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java b/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java index fd5833f..df58b85a1 100644 --- a/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java +++ b/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
@@ -304,10 +304,10 @@ * composition. * @param compositionEnd The character offset of the composition end, or -1 if there is no * selection. - * @param isNonImeChange True when the update was caused by non-IME (e.g. Javascript). + * @param replyToRequest True when the update was requested by IME. */ public void updateState(String text, int selectionStart, int selectionEnd, int compositionStart, - int compositionEnd, boolean isNonImeChange) { + int compositionEnd, boolean replyToRequest) { if (mCursorAnchorInfoController != null && (!TextUtils.equals(mLastText, text) || mLastSelectionStart != selectionStart || mLastSelectionEnd != selectionEnd || mLastCompositionStart != compositionStart @@ -324,7 +324,7 @@ boolean singleLine = mTextInputType != TextInputType.TEXT_AREA && mTextInputType != TextInputType.CONTENT_EDITABLE; mInputConnection.updateStateOnUiThread(text, selectionStart, selectionEnd, compositionStart, - compositionEnd, singleLine, isNonImeChange); + compositionEnd, singleLine, replyToRequest); } /**
diff --git a/content/public/android/java/src/org/chromium/content/browser/input/TextInputState.java b/content/public/android/java/src/org/chromium/content/browser/input/TextInputState.java index 38684df..d205485 100644 --- a/content/public/android/java/src/org/chromium/content/browser/input/TextInputState.java +++ b/content/public/android/java/src/org/chromium/content/browser/input/TextInputState.java
@@ -17,10 +17,10 @@ private final Range mSelection; private final Range mComposition; private final boolean mSingleLine; - private final boolean mFromIme; + private final boolean mReplyToRequest; public TextInputState(CharSequence text, Range selection, Range composition, boolean singleLine, - boolean fromIme) { + boolean replyToRequest) { selection.clamp(0, text.length()); if (composition.start() != -1 || composition.end() != -1) { composition.clamp(0, text.length()); @@ -29,7 +29,7 @@ mSelection = selection; mComposition = composition; mSingleLine = singleLine; - mFromIme = fromIme; + mReplyToRequest = replyToRequest; } public CharSequence text() { @@ -48,8 +48,8 @@ return mSingleLine; } - public boolean fromIme() { - return mFromIme; + public boolean replyToRequest() { + return mReplyToRequest; } public CharSequence getSelectedText() { @@ -74,13 +74,13 @@ if (t == this) return true; return TextUtils.equals(mText, t.mText) && mSelection.equals(t.mSelection) && mComposition.equals(t.mComposition) && mSingleLine == t.mSingleLine - && mFromIme == t.mFromIme; + && mReplyToRequest == t.mReplyToRequest; } @Override public int hashCode() { return mText.hashCode() * 7 + mSelection.hashCode() * 11 + mComposition.hashCode() * 13 - + (mSingleLine ? 19 : 0) + (mFromIme ? 23 : 0); + + (mSingleLine ? 19 : 0) + (mReplyToRequest ? 23 : 0); } @SuppressWarnings("unused") @@ -90,8 +90,8 @@ @Override public String toString() { - return String.format(Locale.US, "TextInputState {[%s] SEL%s COM%s %s %s}", mText, - mSelection, mComposition, mSingleLine ? "SIN" : "MUL", - mFromIme ? "fromIME" : "NOTfromIME"); + return String.format(Locale.US, "TextInputState {[%s] SEL%s COM%s %s%s}", mText, mSelection, + mComposition, mSingleLine ? "SIN" : "MUL", + mReplyToRequest ? " ReplyToRequest" : ""); } } \ No newline at end of file
diff --git a/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnection.java b/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnection.java index 92c24c983..330863f770 100644 --- a/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnection.java +++ b/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnection.java
@@ -114,22 +114,21 @@ } @Override - public void updateStateOnUiThread(String text, final int selectionStart, - final int selectionEnd, final int compositionStart, final int compositionEnd, - boolean singleLine, final boolean isNonImeChange) { + public void updateStateOnUiThread(String text, final int selectionStart, final int selectionEnd, + final int compositionStart, final int compositionEnd, boolean singleLine, + final boolean replyToRequest) { ImeUtils.checkOnUiThread(); // crbug.com/663880: Non-breaking spaces can cause the IME to get confused. Replace with // normal spaces. text = text.replace('\u00A0', ' '); - mCachedTextInputState = - new TextInputState(text, new Range(selectionStart, selectionEnd), - new Range(compositionStart, compositionEnd), singleLine, !isNonImeChange); + mCachedTextInputState = new TextInputState(text, new Range(selectionStart, selectionEnd), + new Range(compositionStart, compositionEnd), singleLine, replyToRequest); if (DEBUG_LOGS) Log.w(TAG, "updateState: %s", mCachedTextInputState); addToQueueOnUiThread(mCachedTextInputState); - if (isNonImeChange) { + if (!replyToRequest) { mHandler.post(mProcessPendingInputStatesRunnable); } } @@ -270,7 +269,7 @@ if (state.shouldUnblock()) { if (DEBUG_LOGS) Log.w(TAG, "blockAndGetStateUpdate: unblocked"); return null; - } else if (state.fromIme()) { + } else if (state.replyToRequest()) { if (shouldUpdateSelection) updateSelection(state); if (DEBUG_LOGS) Log.w(TAG, "blockAndGetStateUpdate done: %d", mQueue.size()); return state;
diff --git a/content/public/android/junit/src/org/chromium/content/browser/input/ThreadedInputConnectionTest.java b/content/public/android/junit/src/org/chromium/content/browser/input/ThreadedInputConnectionTest.java index b452b80..a6c32806 100644 --- a/content/public/android/junit/src/org/chromium/content/browser/input/ThreadedInputConnectionTest.java +++ b/content/public/android/junit/src/org/chromium/content/browser/input/ThreadedInputConnectionTest.java
@@ -80,12 +80,12 @@ mInOrder.verify(mImeAdapter).sendCompositionToNative("hello", 1, false, 0); // Renderer updates states asynchronously. - mConnection.updateStateOnUiThread("hello", 5, 5, 0, 5, true, true); + mConnection.updateStateOnUiThread("hello", 5, 5, 0, 5, true, false); mInOrder.verify(mImeAdapter).updateSelection(5, 5, 0, 5); assertEquals(0, mConnection.getQueueForTest().size()); // Prepare to call requestTextInputStateUpdate. - mConnection.updateStateOnUiThread("hello", 5, 5, 0, 5, true, false); + mConnection.updateStateOnUiThread("hello", 5, 5, 0, 5, true, true); assertEquals(1, mConnection.getQueueForTest().size()); when(mImeAdapter.requestTextInputStateUpdate()).thenReturn(true); @@ -95,11 +95,11 @@ // IME app calls finishComposingText(). mConnection.finishComposingText(); mInOrder.verify(mImeAdapter).finishComposingText(); - mConnection.updateStateOnUiThread("hello", 5, 5, -1, -1, true, true); + mConnection.updateStateOnUiThread("hello", 5, 5, -1, -1, true, false); mInOrder.verify(mImeAdapter).updateSelection(5, 5, -1, -1); // Prepare to call requestTextInputStateUpdate. - mConnection.updateStateOnUiThread("hello", 5, 5, -1, -1, true, false); + mConnection.updateStateOnUiThread("hello", 5, 5, -1, -1, true, true); assertEquals(1, mConnection.getQueueForTest().size()); when(mImeAdapter.requestTextInputStateUpdate()).thenReturn(true); @@ -124,7 +124,7 @@ @Feature({"TextInput"}) public void testRenderChangeUpdatesSelection() { // User moves the cursor. - mConnection.updateStateOnUiThread("hello", 4, 4, -1, -1, true, true); + mConnection.updateStateOnUiThread("hello", 4, 4, -1, -1, true, false); mInOrder.verify(mImeAdapter).updateSelection(4, 4, -1, -1); assertEquals(0, mConnection.getQueueForTest().size()); } @@ -139,7 +139,7 @@ mInOrder.verify(mImeAdapter).sendCompositionToNative("hello", 1, true, 0); // Renderer updates states asynchronously. - mConnection.updateStateOnUiThread("hello", 5, 5, -1, -1, true, true); + mConnection.updateStateOnUiThread("hello", 5, 5, -1, -1, true, false); mInOrder.verify(mImeAdapter, never()).updateSelection(5, 5, -1, -1); assertEquals(0, mConnection.getQueueForTest().size()); @@ -154,7 +154,7 @@ mInOrder.verify(mImeAdapter, never()).updateSelection(4, 4, -1, -1); // Prepare to call requestTextInputStateUpdate. - mConnection.updateStateOnUiThread("hello", 4, 4, -1, -1, true, false); + mConnection.updateStateOnUiThread("hello", 4, 4, -1, -1, true, true); assertEquals(1, mConnection.getQueueForTest().size()); when(mImeAdapter.requestTextInputStateUpdate()).thenReturn(true); @@ -212,7 +212,7 @@ } })); // Or it could be. - mConnection.updateStateOnUiThread("hello", 5, 5, -1, -1, true, true); + mConnection.updateStateOnUiThread("hello", 5, 5, -1, -1, true, false); assertEquals("hello", ThreadUtils.runOnUiThreadBlockingNoException(new Callable<CharSequence>() { @Override
diff --git a/content/public/test/mock_render_process_host.cc b/content/public/test/mock_render_process_host.cc index c7ba2ec..d0af148f 100644 --- a/content/public/test/mock_render_process_host.cc +++ b/content/public/test/mock_render_process_host.cc
@@ -310,7 +310,7 @@ mojom::Renderer* MockRenderProcessHost::GetRendererInterface() { if (!renderer_interface_) { renderer_interface_.reset(new mojom::RendererAssociatedPtr); - mojo::GetDummyProxyForTesting(renderer_interface_.get()); + mojo::GetIsolatedProxy(renderer_interface_.get()); } return renderer_interface_->get(); }
diff --git a/content/public/test/text_input_test_utils.cc b/content/public/test/text_input_test_utils.cc index 6e734b9..e37b5dc 100644 --- a/content/public/test/text_input_test_utils.cc +++ b/content/public/test/text_input_test_utils.cc
@@ -396,10 +396,6 @@ text_input_state_->show_ime_if_needed = show_ime_if_needed; } -void TextInputStateSender::SetIsNonImeChange(bool is_non_ime_change) { - text_input_state_->is_non_ime_change = is_non_ime_change; -} - TestInputMethodObserver::TestInputMethodObserver() {} TestInputMethodObserver::~TestInputMethodObserver() {}
diff --git a/content/public/test/text_input_test_utils.h b/content/public/test/text_input_test_utils.h index a552dd4..6c0c2bc 100644 --- a/content/public/test/text_input_test_utils.h +++ b/content/public/test/text_input_test_utils.h
@@ -160,7 +160,6 @@ void SetFlags(int flags); void SetCanComposeInline(bool can_compose_inline); void SetShowImeIfNeeded(bool show_ime_if_needed); - void SetIsNonImeChange(bool is_non_ime_change); private: std::unique_ptr<TextInputState> text_input_state_;
diff --git a/content/renderer/ime_event_guard.cc b/content/renderer/ime_event_guard.cc index b33183f..b2d78c6b 100644 --- a/content/renderer/ime_event_guard.cc +++ b/content/renderer/ime_event_guard.cc
@@ -13,7 +13,7 @@ // it from other updates so that we can wait for it safely. So it is false by // default. ImeEventGuard::ImeEventGuard(RenderWidget* widget) - : widget_(widget), show_ime_(false), from_ime_(false) { + : widget_(widget), show_virtual_keyboard_(false), reply_to_request_(false) { widget_->OnImeEventGuardStart(this); }
diff --git a/content/renderer/ime_event_guard.h b/content/renderer/ime_event_guard.h index 6d69912..1df77f34 100644 --- a/content/renderer/ime_event_guard.h +++ b/content/renderer/ime_event_guard.h
@@ -15,15 +15,14 @@ explicit ImeEventGuard(RenderWidget* widget); ~ImeEventGuard(); - bool show_ime() const { return show_ime_; } - bool from_ime() const { return from_ime_; } - void set_show_ime(bool show_ime) { show_ime_ = show_ime; } - void set_from_ime(bool from_ime) { from_ime_ = from_ime; } + bool show_virtual_keyboard() const { return show_virtual_keyboard_; } + bool reply_to_request() const { return reply_to_request_; } + void set_show_virtual_keyboard(bool show) { show_virtual_keyboard_ = show; } private: RenderWidget* widget_; - bool show_ime_; - bool from_ime_; + bool show_virtual_keyboard_; + bool reply_to_request_; }; }
diff --git a/content/renderer/input/render_widget_input_handler.cc b/content/renderer/input/render_widget_input_handler.cc index 6e1ad1e0..63e11df6 100644 --- a/content/renderer/input/render_widget_input_handler.cc +++ b/content/renderer/input/render_widget_input_handler.cc
@@ -279,7 +279,7 @@ static_cast<const WebKeyboardEvent&>(input_event); if (key_event.nativeKeyCode == AKEYCODE_DPAD_CENTER && widget_->GetTextInputType() != ui::TEXT_INPUT_TYPE_NONE) { - widget_->showImeIfNeeded(); + widget_->showVirtualKeyboard(); prevent_default = true; } #endif @@ -415,8 +415,7 @@ // of a processed touch end event. if (input_event.type() == WebInputEvent::TouchEnd && processed != WebInputEventResult::NotHandled) { - delegate_->UpdateTextInputState(ShowIme::IF_NEEDED, - ChangeSource::FROM_NON_IME); + delegate_->ShowVirtualKeyboard(); } #elif defined(USE_AURA) // Show the virtual keyboard if enabled and a user gesture triggers a focus @@ -424,7 +423,7 @@ if (processed != WebInputEventResult::NotHandled && (input_event.type() == WebInputEvent::TouchEnd || input_event.type() == WebInputEvent::MouseUp)) { - delegate_->UpdateTextInputState(ShowIme::IF_NEEDED, ChangeSource::FROM_IME); + delegate_->ShowVirtualKeyboard(); } #endif
diff --git a/content/renderer/input/render_widget_input_handler_delegate.h b/content/renderer/input/render_widget_input_handler_delegate.h index 4c245f2..d1fd030 100644 --- a/content/renderer/input/render_widget_input_handler_delegate.h +++ b/content/renderer/input/render_widget_input_handler_delegate.h
@@ -24,13 +24,6 @@ class RenderWidgetInputHandler; -enum class ShowIme { IF_NEEDED, HIDE_IME }; - -enum class ChangeSource { - FROM_NON_IME, - FROM_IME, -}; - // Consumers of RenderWidgetInputHandler implement this delegate in order to // transport input handling information across processes. class CONTENT_EXPORT RenderWidgetInputHandlerDelegate { @@ -68,15 +61,14 @@ // Notifies the delegate of the |input_handler| managing it. virtual void SetInputHandler(RenderWidgetInputHandler* input_handler) = 0; - // |show_ime| should be ShowIme::IF_NEEDED iff the update may cause the ime to - // be displayed, e.g. after a tap on an input field on mobile. - // |change_source| should be ChangeSource::FROM_NON_IME when the renderer has - // to wait for the browser to acknowledge the change before the renderer - // handles any more IME events. This is when the text change did not originate - // from the IME in the browser side, such as changes by JavaScript or - // autofill. - virtual void UpdateTextInputState(ShowIme show_ime, - ChangeSource change_source) = 0; + // Call this if virtual keyboard should be displayed, e.g. after a tap on an + // input field on mobile. Note that we do this just by setting a bit in + // text input state and update it to the browser such that browser process can + // be up to date when showing virtual keyboard. + virtual void ShowVirtualKeyboard() = 0; + + // Send an update of text input state to the browser process. + virtual void UpdateTextInputState() = 0; // Notifies that a gesture event is about to be handled. // Returns true if no further handling is needed. In that case, the event
diff --git a/content/renderer/mus/render_widget_mus_connection.cc b/content/renderer/mus/render_widget_mus_connection.cc index b416e755..24594cb 100644 --- a/content/renderer/mus/render_widget_mus_connection.cc +++ b/content/renderer/mus/render_widget_mus_connection.cc
@@ -129,9 +129,11 @@ input_handler_ = input_handler; } -void RenderWidgetMusConnection::UpdateTextInputState( - ShowIme show_ime, - ChangeSource change_source) { +void RenderWidgetMusConnection::ShowVirtualKeyboard() { + NOTIMPLEMENTED(); +} + +void RenderWidgetMusConnection::UpdateTextInputState() { NOTIMPLEMENTED(); }
diff --git a/content/renderer/mus/render_widget_mus_connection.h b/content/renderer/mus/render_widget_mus_connection.h index bcf2d1c..7cf7bc9 100644 --- a/content/renderer/mus/render_widget_mus_connection.h +++ b/content/renderer/mus/render_widget_mus_connection.h
@@ -57,8 +57,8 @@ void NotifyInputEventHandled(blink::WebInputEvent::Type handled_type, InputEventAckState ack_result) override; void SetInputHandler(RenderWidgetInputHandler* input_handler) override; - void UpdateTextInputState(ShowIme show_ime, - ChangeSource change_source) override; + void ShowVirtualKeyboard() override; + void UpdateTextInputState() override; bool WillHandleGestureEvent(const blink::WebGestureEvent& event) override; bool WillHandleMouseEvent(const blink::WebMouseEvent& event) override;
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index c0adcb2..ce8f2725 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc
@@ -1305,8 +1305,7 @@ if (instance != focused_pepper_plugin_) return; - GetRenderWidget()->UpdateTextInputState(ShowIme::HIDE_IME, - ChangeSource::FROM_NON_IME); + GetRenderWidget()->UpdateTextInputState(); FocusedNodeChangedForAccessibility(WebNode()); } @@ -4043,8 +4042,7 @@ // was changed, and SyncSelectionIfRequired may send SelectionChanged // to notify the selection was changed. Focus change should be notified // before selection change. - GetRenderWidget()->UpdateTextInputState(ShowIme::HIDE_IME, - ChangeSource::FROM_NON_IME); + GetRenderWidget()->UpdateTextInputState(); SyncSelectionIfRequired(); } @@ -6811,8 +6809,7 @@ GetRenderWidget()->set_focused_pepper_plugin(focused_pepper_plugin_); - GetRenderWidget()->UpdateTextInputState(ShowIme::HIDE_IME, - ChangeSource::FROM_NON_IME); + GetRenderWidget()->UpdateTextInputState(); GetRenderWidget()->UpdateSelectionBounds(); }
diff --git a/content/renderer/render_view_browsertest.cc b/content/renderer/render_view_browsertest.cc index af65af0..3970bb9e 100644 --- a/content/renderer/render_view_browsertest.cc +++ b/content/renderer/render_view_browsertest.cc
@@ -1092,7 +1092,7 @@ // Update the IME status and verify if our IME backend sends an IPC message // to activate IMEs. - view()->UpdateTextInputState(ShowIme::HIDE_IME, ChangeSource::FROM_NON_IME); + view()->UpdateTextInputState(); const IPC::Message* msg = render_thread_->sink().GetMessageAt(0); EXPECT_TRUE(msg != NULL); EXPECT_EQ(ViewHostMsg_TextInputStateChanged::ID, msg->type()); @@ -1113,7 +1113,7 @@ // Update the IME status and verify if our IME backend sends an IPC message // to de-activate IMEs. - view()->UpdateTextInputState(ShowIme::HIDE_IME, ChangeSource::FROM_NON_IME); + view()->UpdateTextInputState(); msg = render_thread_->sink().GetMessageAt(0); EXPECT_TRUE(msg != NULL); EXPECT_EQ(ViewHostMsg_TextInputStateChanged::ID, msg->type()); @@ -1136,8 +1136,7 @@ // Update the IME status and verify if our IME backend sends an IPC // message to activate IMEs. - view()->UpdateTextInputState(ShowIme::HIDE_IME, - ChangeSource::FROM_NON_IME); + view()->UpdateTextInputState(); ProcessPendingMessages(); const IPC::Message* msg = render_thread_->sink().GetMessageAt(0); EXPECT_TRUE(msg != NULL); @@ -1277,7 +1276,7 @@ // Update the status of our IME back-end. // TODO(hbono): we should verify messages to be sent from the back-end. - view()->UpdateTextInputState(ShowIme::HIDE_IME, ChangeSource::FROM_NON_IME); + view()->UpdateTextInputState(); ProcessPendingMessages(); render_thread_->sink().ClearMessages();
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc index 42a0bb7..7083f7d 100644 --- a/content/renderer/render_view_impl.cc +++ b/content/renderer/render_view_impl.cc
@@ -1904,8 +1904,8 @@ RenderWidget::setTouchAction(touchAction); } -void RenderViewImpl::showImeIfNeeded() { - RenderWidget::showImeIfNeeded(); +void RenderViewImpl::showVirtualKeyboard() { + RenderWidget::showVirtualKeyboard(); } void RenderViewImpl::showUnhandledTapUIIfNeeded(
diff --git a/content/renderer/render_view_impl.h b/content/renderer/render_view_impl.h index 7cdb321..3e062e1 100644 --- a/content/renderer/render_view_impl.h +++ b/content/renderer/render_view_impl.h
@@ -283,7 +283,7 @@ void setToolTipText(const blink::WebString&, blink::WebTextDirection hint) override; void setTouchAction(blink::WebTouchAction touchAction) override; - void showImeIfNeeded() override; + void showVirtualKeyboard() override; void showUnhandledTapUIIfNeeded(const blink::WebPoint& tappedPosition, const blink::WebNode& tappedNode, bool pageChanged) override;
diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc index fd245a41..7bd31d9 100644 --- a/content/renderer/render_widget.cc +++ b/content/renderer/render_widget.cc
@@ -932,7 +932,7 @@ // The UpdateTextInputState can result in further layout and possibly // enable GPU acceleration so they need to be called before any painting // is done. - UpdateTextInputState(ShowIme::HIDE_IME, ChangeSource::FROM_NON_IME); + UpdateTextInputState(); UpdateSelectionBounds(); for (auto& observer : render_frame_proxies_) @@ -1024,15 +1024,25 @@ DCHECK(!input_handler_); } -void RenderWidget::UpdateTextInputState(ShowIme show_ime, - ChangeSource change_source) { +void RenderWidget::ShowVirtualKeyboard() { + UpdateTextInputStateInternal(true, false); +} + +void RenderWidget::UpdateTextInputState() { + UpdateTextInputStateInternal(false, false); +} + +void RenderWidget::UpdateTextInputStateInternal(bool show_virtual_keyboard, + bool reply_to_request) { TRACE_EVENT0("renderer", "RenderWidget::UpdateTextInputState"); + if (ime_event_guard_) { - // show_ime should still be effective even if it was set inside the IME + DCHECK(!reply_to_request); + // show_virtual_keyboard should still be effective even if it was set inside + // the IME // event guard. - if (show_ime == ShowIme::IF_NEEDED) { - ime_event_guard_->set_show_ime(true); - } + if (show_virtual_keyboard) + ime_event_guard_->set_show_virtual_keyboard(true); return; } @@ -1050,8 +1060,7 @@ // Only sends text input params if they are changed or if the ime should be // shown. - if (show_ime == ShowIme::IF_NEEDED || - change_source == ChangeSource::FROM_IME || + if (show_virtual_keyboard || reply_to_request || text_input_type_ != new_type || text_input_mode_ != new_mode || text_input_info_ != new_info || can_compose_inline_ != new_can_compose_inline) { @@ -1065,10 +1074,10 @@ params.composition_start = new_info.compositionStart; params.composition_end = new_info.compositionEnd; params.can_compose_inline = new_can_compose_inline; - params.show_ime_if_needed = (show_ime == ShowIme::IF_NEEDED); -#if defined(OS_ANDROID) || defined(USE_AURA) - params.is_non_ime_change = (change_source == ChangeSource::FROM_NON_IME); -#endif + // TODO(changwan): change instances of show_ime_if_needed to + // show_virtual_keyboard. + params.show_ime_if_needed = show_virtual_keyboard; + params.reply_to_request = reply_to_request; Send(new ViewHostMsg_TextInputStateChanged(routing_id(), params)); text_input_info_ = new_info; @@ -1739,10 +1748,8 @@ static_cast<WebFrameWidget*>(GetWebWidget())->dragSourceSystemDragEnded(); } -void RenderWidget::showImeIfNeeded() { -#if defined(OS_ANDROID) || defined(USE_AURA) - UpdateTextInputState(ShowIme::IF_NEEDED, ChangeSource::FROM_NON_IME); -#endif +void RenderWidget::showVirtualKeyboard() { + ShowVirtualKeyboard(); // TODO(rouslan): Fix ChromeOS and Windows 8 behavior of autofill popup with // virtual keyboard. @@ -1814,7 +1821,7 @@ void RenderWidget::OnRequestTextInputStateUpdate() { DCHECK(!ime_event_guard_); UpdateSelectionBounds(); - UpdateTextInputState(ShowIme::HIDE_IME, ChangeSource::FROM_IME); + UpdateTextInputStateInternal(false, true /* reply_to_request */); } #endif @@ -1888,15 +1895,7 @@ void RenderWidget::OnImeEventGuardFinish(ImeEventGuard* guard) { if (ime_event_guard_ != guard) { -#if defined(OS_ANDROID) - // In case a from-IME event (e.g. touch) ends up in not-from-IME event - // (e.g. long press gesture), we want to treat it as not-from-IME event - // so that ReplicaInputConnection can make changes to its Editable model. - // Therefore, we want to mark this text state update as 'from IME' only - // when all the nested events are all originating from IME. - ime_event_guard_->set_from_ime( - ime_event_guard_->from_ime() && guard->from_ime()); -#endif + DCHECK(!ime_event_guard_->reply_to_request()); return; } ime_event_guard_ = nullptr; @@ -1906,9 +1905,10 @@ // ime event. UpdateSelectionBounds(); #if defined(OS_ANDROID) - UpdateTextInputState( - guard->show_ime() ? ShowIme::IF_NEEDED : ShowIme::HIDE_IME, - guard->from_ime() ? ChangeSource::FROM_IME : ChangeSource::FROM_NON_IME); + if (guard->show_virtual_keyboard()) + ShowVirtualKeyboard(); + else + UpdateTextInputState(); #endif } @@ -2123,14 +2123,14 @@ if (event_cancelled) return; if (event.type() == WebInputEvent::GestureTap) { - UpdateTextInputState(ShowIme::IF_NEEDED, ChangeSource::FROM_NON_IME); + ShowVirtualKeyboard(); } else if (event.type() == WebInputEvent::GestureLongPress) { DCHECK(GetWebWidget()); blink::WebInputMethodController* controller = GetInputMethodController(); if (!controller || controller->textInputInfo().value.isEmpty()) - UpdateTextInputState(ShowIme::HIDE_IME, ChangeSource::FROM_NON_IME); + UpdateTextInputState(); else - UpdateTextInputState(ShowIme::IF_NEEDED, ChangeSource::FROM_NON_IME); + ShowVirtualKeyboard(); } // TODO(ananta): Piggyback off existing IPCs to communicate this information, // crbug/420130.
diff --git a/content/renderer/render_widget.h b/content/renderer/render_widget.h index 7c71a0e..c446998 100644 --- a/content/renderer/render_widget.h +++ b/content/renderer/render_widget.h
@@ -264,8 +264,8 @@ void NotifyInputEventHandled(blink::WebInputEvent::Type handled_type, InputEventAckState ack_result) override; void SetInputHandler(RenderWidgetInputHandler* input_handler) override; - void UpdateTextInputState(ShowIme show_ime, - ChangeSource change_source) override; + void ShowVirtualKeyboard() override; + void UpdateTextInputState() override; bool WillHandleGestureEvent(const blink::WebGestureEvent& event) override; bool WillHandleMouseEvent(const blink::WebMouseEvent& event) override; @@ -297,7 +297,7 @@ const blink::WebFloatSize& accumulatedOverscroll, const blink::WebFloatPoint& position, const blink::WebFloatSize& velocity) override; - void showImeIfNeeded() override; + void showVirtualKeyboard() override; void convertViewportToWindow(blink::WebRect* rect) override; void convertWindowToViewport(blink::WebFloatRect* rect) override; bool requestPointerLock() override; @@ -809,6 +809,9 @@ // handle the event, or there is no page focus. bool ShouldHandleImeEvents() const; + void UpdateTextInputStateInternal(bool show_virtual_keyboard, + bool reply_to_request); + // Indicates whether this widget has focus. bool has_focus_;
diff --git a/content/renderer/screen_orientation/screen_orientation_dispatcher_browsertest.cc b/content/renderer/screen_orientation/screen_orientation_dispatcher_browsertest.cc index 102006b1..973c8e1f 100644 --- a/content/renderer/screen_orientation/screen_orientation_dispatcher_browsertest.cc +++ b/content/renderer/screen_orientation/screen_orientation_dispatcher_browsertest.cc
@@ -59,7 +59,7 @@ RenderViewTest::SetUp(); dispatcher_.reset(new ScreenOrientationDispatcher(nullptr)); ScreenOrientationAssociatedPtr screen_orientation; - mojo::GetDummyProxyForTesting(&screen_orientation); + mojo::GetIsolatedProxy(&screen_orientation); dispatcher_->SetScreenOrientationForTests(screen_orientation); }
diff --git a/content/test/layouttest_support.cc b/content/test/layouttest_support.cc index 764125ce..0107ded 100644 --- a/content/test/layouttest_support.cc +++ b/content/test/layouttest_support.cc
@@ -604,8 +604,7 @@ void ForceTextInputStateUpdateForRenderFrame(RenderFrame* frame) { if (auto* render_widget = static_cast<RenderFrameImpl*>(frame)->GetRenderWidget()) { - render_widget->UpdateTextInputState(ShowIme::IF_NEEDED, - ChangeSource::FROM_NON_IME); + render_widget->ShowVirtualKeyboard(); } }
diff --git a/google_apis/drive/drive_api_requests.cc b/google_apis/drive/drive_api_requests.cc index 607c657..f534efa0 100644 --- a/google_apis/drive/drive_api_requests.cc +++ b/google_apis/drive/drive_api_requests.cc
@@ -10,6 +10,7 @@ #include "base/callback.h" #include "base/json/json_writer.h" #include "base/location.h" +#include "base/memory/ptr_util.h" #include "base/metrics/histogram_macros.h" #include "base/metrics/sparse_histogram.h" #include "base/sequenced_task_runner.h" @@ -1226,7 +1227,7 @@ DCHECK(request); DCHECK(GetChildEntry(request) == child_requests_.end()); DCHECK(!committed_); - child_requests_.push_back(new BatchUploadChildEntry(request)); + child_requests_.push_back(base::MakeUnique<BatchUploadChildEntry>(request)); request->Prepare(base::Bind(&BatchUploadRequest::OnChildRequestPrepared, weak_ptr_factory_.GetWeakPtr(), request)); } @@ -1270,8 +1271,8 @@ // Obtains corresponding child entry of |request_id|. Returns NULL if the // entry is not found. -ScopedVector<BatchUploadChildEntry>::iterator BatchUploadRequest::GetChildEntry( - RequestID request_id) { +std::vector<std::unique_ptr<BatchUploadChildEntry>>::iterator +BatchUploadRequest::GetChildEntry(RequestID request_id) { for (auto it = child_requests_.begin(); it != child_requests_.end(); ++it) { if ((*it)->request.get() == request_id) return it; @@ -1282,7 +1283,7 @@ void BatchUploadRequest::MayCompletePrepare() { if (!committed_ || prepare_callback_.is_null()) return; - for (auto* child : child_requests_) { + for (const auto& child : child_requests_) { if (!child->prepared) return; } @@ -1290,7 +1291,7 @@ // Build multipart body here. int64_t total_size = 0; std::vector<ContentTypeAndData> parts; - for (auto* child : child_requests_) { + for (const auto& child : child_requests_) { std::string type; std::string data; const bool result = child->request->GetContentData(&type, &data); @@ -1409,7 +1410,7 @@ } void BatchUploadRequest::RunCallbackOnPrematureFailure(DriveApiErrorCode code) { - for (auto* child : child_requests_) + for (const auto& child : child_requests_) child->request->NotifyError(code); child_requests_.clear(); } @@ -1417,7 +1418,7 @@ void BatchUploadRequest::OnURLFetchUploadProgress(const net::URLFetcher* source, int64_t current, int64_t total) { - for (auto* child : child_requests_) { + for (const auto& child : child_requests_) { if (child->data_offset <= current && current <= child->data_offset + child->data_size) { child->request->NotifyUploadProgress(source, current - child->data_offset,
diff --git a/google_apis/drive/drive_api_requests.h b/google_apis/drive/drive_api_requests.h index 9ccd9fa7..f880fcf9 100644 --- a/google_apis/drive/drive_api_requests.h +++ b/google_apis/drive/drive_api_requests.h
@@ -15,7 +15,6 @@ #include "base/callback_forward.h" #include "base/location.h" #include "base/macros.h" -#include "base/memory/scoped_vector.h" #include "base/sequenced_task_runner.h" #include "base/task_runner_util.h" #include "base/time/time.h" @@ -1227,7 +1226,7 @@ typedef void* RequestID; // Obtains corresponding child entry of |request_id|. Returns NULL if the // entry is not found. - ScopedVector<BatchUploadChildEntry>::iterator GetChildEntry( + std::vector<std::unique_ptr<BatchUploadChildEntry>>::iterator GetChildEntry( RequestID request_id); // Called after child requests' |Prepare| method. @@ -1241,7 +1240,7 @@ RequestSender* const sender_; const DriveApiUrlGenerator url_generator_; - ScopedVector<BatchUploadChildEntry> child_requests_; + std::vector<std::unique_ptr<BatchUploadChildEntry>> child_requests_; PrepareCallback prepare_callback_; bool committed_;
diff --git a/google_apis/drive/test_util.cc b/google_apis/drive/test_util.cc index a6497ca..6c0202fc 100644 --- a/google_apis/drive/test_util.cc +++ b/google_apis/drive/test_util.cc
@@ -174,7 +174,7 @@ void TestGetContentCallback::OnGetContent(google_apis::DriveApiErrorCode error, std::unique_ptr<std::string> data) { - data_.push_back(data.release()); + data_.push_back(std::move(data)); } } // namespace test_util
diff --git a/google_apis/drive/test_util.h b/google_apis/drive/test_util.h index 79e07c4..8b17f4b 100644 --- a/google_apis/drive/test_util.h +++ b/google_apis/drive/test_util.h
@@ -16,7 +16,6 @@ #include "base/bind.h" #include "base/callback.h" #include "base/macros.h" -#include "base/memory/scoped_vector.h" #include "google_apis/drive/base_requests.h" #include "google_apis/drive/drive_api_error_codes.h" #include "google_apis/drive/task_util.h" @@ -282,8 +281,10 @@ ~TestGetContentCallback(); const GetContentCallback& callback() const { return callback_; } - const ScopedVector<std::string>& data() const { return data_; } - ScopedVector<std::string>* mutable_data() { return &data_; } + const std::vector<std::unique_ptr<std::string>>& data() const { + return data_; + } + std::vector<std::unique_ptr<std::string>>* mutable_data() { return &data_; } std::string GetConcatenatedData() const; private: @@ -291,7 +292,7 @@ std::unique_ptr<std::string> data); const GetContentCallback callback_; - ScopedVector<std::string> data_; + std::vector<std::unique_ptr<std::string>> data_; DISALLOW_COPY_AND_ASSIGN(TestGetContentCallback); };
diff --git a/mojo/public/cpp/bindings/associated_interface_ptr.h b/mojo/public/cpp/bindings/associated_interface_ptr.h index e760c7d..5d83e48 100644 --- a/mojo/public/cpp/bindings/associated_interface_ptr.h +++ b/mojo/public/cpp/bindings/associated_interface_ptr.h
@@ -204,7 +204,7 @@ // Creates an associated interface. The output |ptr| should be used locally // while the returned request should be passed through the message pipe endpoint -// referred to by |associated_group| to setup the corresponding asssociated +// referred to by |associated_group| to setup the corresponding associated // interface implementation at the remote side. // // NOTE: |ptr| should NOT be used to make calls before the request is sent. @@ -255,16 +255,41 @@ return request; } -// Creates an associated interface proxy which casts its messages into the void. +// Like |GetProxy|, but the interface is never associated with any other +// interface. The returned request can be bound directly to the corresponding +// associated interface implementation, without first passing it through a +// message pipe endpoint. +// +// This function has two main uses: +// +// * In testing, where the returned request is bound to e.g. a mock and there +// are no other interfaces involved. +// +// * When discarding messages sent on an interface, which can be done by +// discarding the returned request. template <typename Interface> -void GetDummyProxyForTesting(AssociatedInterfacePtr<Interface>* proxy) { +AssociatedInterfaceRequest<Interface> GetIsolatedProxy( + AssociatedInterfacePtr<Interface>* ptr) { MessagePipe pipe; - scoped_refptr<internal::MultiplexRouter> router = + scoped_refptr<internal::MultiplexRouter> router0 = new internal::MultiplexRouter(std::move(pipe.handle0), internal::MultiplexRouter::MULTI_INTERFACE, false, base::ThreadTaskRunnerHandle::Get()); - std::unique_ptr<AssociatedGroup> group = router->CreateAssociatedGroup(); - MakeRequest(proxy, group.get()); + scoped_refptr<internal::MultiplexRouter> router1 = + new internal::MultiplexRouter(std::move(pipe.handle1), + internal::MultiplexRouter::MULTI_INTERFACE, + true, base::ThreadTaskRunnerHandle::Get()); + + ScopedInterfaceEndpointHandle endpoint0, endpoint1; + router0->CreateEndpointHandlePair(&endpoint0, &endpoint1); + endpoint1 = router1->CreateLocalEndpointHandle(endpoint1.release()); + + ptr->Bind(AssociatedInterfacePtrInfo<Interface>(std::move(endpoint0), + Interface::Version_)); + + AssociatedInterfaceRequest<Interface> request; + request.Bind(std::move(endpoint1)); + return request; } } // namespace mojo
diff --git a/mojo/public/tools/bindings/chromium_bindings_configuration.gni b/mojo/public/tools/bindings/chromium_bindings_configuration.gni index 791bfa1d..f35a119 100644 --- a/mojo/public/tools/bindings/chromium_bindings_configuration.gni +++ b/mojo/public/tools/bindings/chromium_bindings_configuration.gni
@@ -8,6 +8,7 @@ "//chrome/browser/media/router/mojo/typemaps.gni", "//chrome/common/extensions/typemaps.gni", "//chrome/common/importer/typemaps.gni", + "//chrome/typemaps.gni", "//components/arc/common/typemaps.gni", "//components/metrics/public/cpp/typemaps.gni", "//components/typemaps.gni",
diff --git a/remoting/protocol/ice_transport_channel.h b/remoting/protocol/ice_transport_channel.h index f28ff82..2fbb33c 100644 --- a/remoting/protocol/ice_transport_channel.h +++ b/remoting/protocol/ice_transport_channel.h
@@ -99,7 +99,7 @@ void NotifyConnected(); // Signal handlers for cricket::IceTransportInternal. - void OnCandidateGathered(cricket::IceTransportInternal2* ice_transport, + void OnCandidateGathered(cricket::IceTransportInternal* ice_transport, const cricket::Candidate& candidate); void OnRouteChange(cricket::IceTransportInternal* ice_transport, const cricket::Candidate& candidate);
diff --git a/third_party/WebKit/LayoutTests/NeverFixTests b/third_party/WebKit/LayoutTests/NeverFixTests index 9ff65fe..fe3ec0e9 100644 --- a/third_party/WebKit/LayoutTests/NeverFixTests +++ b/third_party/WebKit/LayoutTests/NeverFixTests
@@ -208,6 +208,16 @@ webaudio/codec-tests/mp3 [ WontFix ] webaudio/codec-tests/aac [ WontFix ] +# WPT subdirectories without owners. +imported/wpt/accelerometer [ WontFix ] +imported/wpt/auxclick [ WontFix ] +imported/wpt/clear-site-data [ WontFix ] +imported/wpt/css-values [ WontFix ] +imported/wpt/gyroscope [ WontFix ] +imported/wpt/magnetometer [ WontFix ] +imported/wpt/preload [ WontFix ] +imported/wpt/upgrade-insecure-requests [ WontFix ] + # These tests require a DRM system that is not available with content_shell. imported/wpt/encrypted-media/drm-check-initdata-type.html [ WontFix ] imported/wpt/encrypted-media/drm-events.html [ WontFix ]
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index 01f68414..6b1c23e 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -1716,14 +1716,7 @@ crbug.com/626703 imported/wpt/html/semantics/interfaces.html [ Failure ] crbug.com/626703 imported/wpt/pointerevents/pointerevent_sequence_at_implicit_release_on_drag-manual.html [ Timeout ] crbug.com/626703 imported/wpt/pointerevents/pointerevent_sequence_at_implicit_release_on_click-manual.html [ Timeout ] -crbug.com/626703 imported/wpt/clear-site-data/navigation.html [ Timeout ] -crbug.com/626703 imported/wpt/auxclick/auxclick_event-manual.html [ Timeout ] crbug.com/626703 imported/wpt/pointerevents/pointerevent_boundary_events_in_capturing-manual.html [ Timeout ] -crbug.com/626703 imported/wpt/upgrade-insecure-requests/image-redirect-upgrade.https.html [ Timeout ] -crbug.com/626703 imported/wpt/upgrade-insecure-requests/websocket-upgrade.https.html [ Timeout ] -crbug.com/626703 imported/wpt/upgrade-insecure-requests/image-upgrade.https.html [ Timeout ] -crbug.com/626703 imported/wpt/upgrade-insecure-requests/iframe-upgrade.https.html [ Timeout ] -crbug.com/626703 imported/wpt/upgrade-insecure-requests/iframe-redirect-upgrade.https.html [ Timeout ] crbug.com/626703 imported/csswg-test/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-align-self-baseline-horiz-001a.xhtml [ Failure ] crbug.com/626703 imported/csswg-test/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-align-self-baseline-horiz-001b.xhtml [ Failure ] crbug.com/626703 imported/csswg-test/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-align-self-baseline-horiz-006.xhtml [ Failure ]
diff --git a/third_party/WebKit/LayoutTests/WPTServeExpectations b/third_party/WebKit/LayoutTests/WPTServeExpectations index af4f3396..e09a1b0e 100644 --- a/third_party/WebKit/LayoutTests/WPTServeExpectations +++ b/third_party/WebKit/LayoutTests/WPTServeExpectations
@@ -1,3 +1,18 @@ # Overriden expectations for imported tests when run with --enable-wptserve # This file is now obsolete and will be removed. See https://crbug.com/666111 + +# Needs rebaseline after enabling WPTServe. +imported/wpt/domparsing/DOMParser-parseFromString-html.html [ Failure ] +imported/wpt/domparsing/DOMParser-parseFromString-xml.html [ Failure ] +imported/wpt/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/dynamic-append.html [ Failure ] +imported/wpt/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/not-in-shadow-tree.html [ Failure ] +imported/wpt/html/webappapis/scripting/processing-model-2/compile-error-same-origin-with-hash.html [ Failure ] +imported/wpt/html/webappapis/scripting/processing-model-2/runtime-error-same-origin-with-hash.html [ Failure ] + +# Update expectation after enabling WPTServe +imported/wpt/domparsing/innerhtml-05.xhtml [ Crash Failure ] +imported/wpt/workers/opaque-origin.html [ Crash Failure ] +imported/wpt/workers/Worker_cross_origin_security_err.htm [ Timeout ] +imported/wpt/service-workers/service-worker/fetch-frame-resource.https.html [ Timeout ] +imported/wpt/service-workers/service-worker/update-after-oneday.https.html [ Timeout ]
diff --git a/third_party/WebKit/LayoutTests/fast/input/placeholder-wrongly-placed-if-too-tall-expected.txt b/third_party/WebKit/LayoutTests/fast/forms/text/placeholder-wrongly-placed-if-too-tall-expected.txt similarity index 100% rename from third_party/WebKit/LayoutTests/fast/input/placeholder-wrongly-placed-if-too-tall-expected.txt rename to third_party/WebKit/LayoutTests/fast/forms/text/placeholder-wrongly-placed-if-too-tall-expected.txt
diff --git a/third_party/WebKit/LayoutTests/fast/input/placeholder-wrongly-placed-if-too-tall.html b/third_party/WebKit/LayoutTests/fast/forms/text/placeholder-wrongly-placed-if-too-tall.html similarity index 93% rename from third_party/WebKit/LayoutTests/fast/input/placeholder-wrongly-placed-if-too-tall.html rename to third_party/WebKit/LayoutTests/fast/forms/text/placeholder-wrongly-placed-if-too-tall.html index d71cc1e..b3d2ca5 100644 --- a/third_party/WebKit/LayoutTests/fast/input/placeholder-wrongly-placed-if-too-tall.html +++ b/third_party/WebKit/LayoutTests/fast/forms/text/placeholder-wrongly-placed-if-too-tall.html
@@ -1,5 +1,5 @@ <!DOCTYPE html> -<script src="../../resources/js-test.js"></script> +<script src="../../../resources/js-test.js"></script> <style> input { height: 26px;
diff --git a/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp b/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp index 1c0ff85..7a2f548 100644 --- a/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp +++ b/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
@@ -1948,26 +1948,17 @@ const PropertyRegistry::Registration* registration = registry->registration(customPropertyName); if (registration) { - const CSSValue* result = nullptr; - if (registration->inherits()) { - if (StyleInheritedVariables* variables = style.inheritedVariables()) - result = variables->registeredVariable(customPropertyName); - } else { - if (StyleNonInheritedVariables* variables = - style.nonInheritedVariables()) - result = variables->registeredVariable(customPropertyName); - } + const CSSValue* result = style.getRegisteredVariable( + customPropertyName, registration->inherits()); if (result) return result; return registration->initial(); } } - StyleInheritedVariables* variables = style.inheritedVariables(); - if (!variables) - return nullptr; - - CSSVariableData* data = variables->getVariable(customPropertyName); + bool isInheritedProperty = true; + CSSVariableData* data = + style.getVariable(customPropertyName, isInheritedProperty); if (!data) return nullptr;
diff --git a/third_party/WebKit/Source/core/dom/Element.cpp b/third_party/WebKit/Source/core/dom/Element.cpp index fede332..d5ea539 100644 --- a/third_party/WebKit/Source/core/dom/Element.cpp +++ b/third_party/WebKit/Source/core/dom/Element.cpp
@@ -2666,7 +2666,7 @@ // gesture. Since tracking that across arbitrary boundaries (eg. // animations) is difficult, for now we match IE's heuristic and bring // up the keyboard if there's been any gesture since load. - document().page()->chromeClient().showImeIfNeeded(); + document().page()->chromeClient().showVirtualKeyboard(); } }
diff --git a/third_party/WebKit/Source/core/frame/BUILD.gn b/third_party/WebKit/Source/core/frame/BUILD.gn index d75dc18..cc8f8861 100644 --- a/third_party/WebKit/Source/core/frame/BUILD.gn +++ b/third_party/WebKit/Source/core/frame/BUILD.gn
@@ -21,8 +21,6 @@ "DOMWindowBase64.cpp", "DOMWindowBase64.h", "DOMWindowEventHandlers.h", - "DOMWindowProperty.cpp", - "DOMWindowProperty.h", "DOMWindowTimers.cpp", "DOMWindowTimers.h", "DeprecatedScheduleStyleRecalcDuringLayout.cpp",
diff --git a/third_party/WebKit/Source/core/frame/DOMWindow.cpp b/third_party/WebKit/Source/core/frame/DOMWindow.cpp index c5f9123..2e3704b 100644 --- a/third_party/WebKit/Source/core/frame/DOMWindow.cpp +++ b/third_party/WebKit/Source/core/frame/DOMWindow.cpp
@@ -155,9 +155,8 @@ } void DOMWindow::resetLocation() { - // Location needs to be reset manually because it doesn't inherit from - // DOMWindowProperty. DOMWindowProperty is local-only, and Location needs to - // support remote windows, too. + // Location needs to be reset manually so that it doesn't retain a stale + // Frame pointer. if (m_location) { m_location->reset(); m_location = nullptr;
diff --git a/third_party/WebKit/Source/core/frame/DOMWindowProperty.cpp b/third_party/WebKit/Source/core/frame/DOMWindowProperty.cpp deleted file mode 100644 index fef480ff..0000000 --- a/third_party/WebKit/Source/core/frame/DOMWindowProperty.cpp +++ /dev/null
@@ -1,56 +0,0 @@ -/* - * Copyright (C) 2011 Google, Inc. All Rights Reserved. - * Copyright (C) 2012 Apple Inc. All Rights Reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY GOOGLE INC. ``AS IS'' AND ANY - * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR - * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR - * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, - * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, - * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR - * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY - * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - -#include "core/frame/DOMWindowProperty.h" - -#include "core/frame/LocalDOMWindow.h" -#include "core/frame/LocalFrame.h" - -namespace blink { - -DOMWindowProperty::DOMWindowProperty(LocalFrame* frame) : m_frame(frame) { - // FIXME: For now it *is* acceptable for a DOMWindowProperty to be created - // with a null frame. See fast/dom/navigator-detached-no-crash.html for the - // recipe. We should fix that. <rdar://problem/11567132> - if (m_frame) { - // FIXME: Need to figure out what to do with DOMWindowProperties on - // remote DOM windows. - m_frame->domWindow()->registerProperty(this); - } -} - -void DOMWindowProperty::frameDestroyed() { - // If the property is getting this callback it must have been - // created with a LocalFrame and it should still have it. - ASSERT(m_frame); - m_frame = nullptr; -} - -DEFINE_TRACE(DOMWindowProperty) { - visitor->trace(m_frame); -} - -} // namespace blink
diff --git a/third_party/WebKit/Source/core/frame/DOMWindowProperty.h b/third_party/WebKit/Source/core/frame/DOMWindowProperty.h deleted file mode 100644 index 283f23b..0000000 --- a/third_party/WebKit/Source/core/frame/DOMWindowProperty.h +++ /dev/null
@@ -1,52 +0,0 @@ -/* - * Copyright (C) 2011 Google, Inc. All Rights Reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY GOOGLE INC. ``AS IS'' AND ANY - * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR - * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR - * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, - * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, - * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR - * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY - * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - -#ifndef DOMWindowProperty_h -#define DOMWindowProperty_h - -#include "core/CoreExport.h" -#include "platform/heap/Handle.h" - -namespace blink { - -class LocalFrame; - -class CORE_EXPORT DOMWindowProperty : public GarbageCollectedMixin { - public: - explicit DOMWindowProperty(LocalFrame*); - - void frameDestroyed(); - - LocalFrame* frame() const { return m_frame; } - - DECLARE_VIRTUAL_TRACE(); - - private: - Member<LocalFrame> m_frame; -}; - -} // namespace blink - -#endif // DOMWindowProperty_h
diff --git a/third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp b/third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp index 9a9bab1..62548b8 100644 --- a/third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp +++ b/third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp
@@ -53,7 +53,6 @@ #include "core/fetch/ResourceFetcher.h" #include "core/frame/BarProp.h" #include "core/frame/DOMVisualViewport.h" -#include "core/frame/DOMWindowProperty.h" #include "core/frame/EventHandlerRegistry.h" #include "core/frame/FrameConsole.h" #include "core/frame/FrameView.h" @@ -479,23 +478,11 @@ } void LocalDOMWindow::frameDestroyed() { - for (const auto& domWindowProperty : m_properties) - domWindowProperty->frameDestroyed(); - resetLocation(); - m_properties.clear(); removeAllEventListeners(); disconnectFromFrame(); } -void LocalDOMWindow::registerProperty(DOMWindowProperty* property) { - m_properties.add(property); -} - -void LocalDOMWindow::unregisterProperty(DOMWindowProperty* property) { - m_properties.remove(property); -} - void LocalDOMWindow::registerEventListenerObserver( EventListenerObserver* eventListenerObserver) { m_eventListenerObservers.add(eventListenerObserver); @@ -1609,7 +1596,6 @@ DEFINE_TRACE(LocalDOMWindow) { visitor->trace(m_document); - visitor->trace(m_properties); visitor->trace(m_screen); visitor->trace(m_history); visitor->trace(m_locationbar);
diff --git a/third_party/WebKit/Source/core/frame/LocalDOMWindow.h b/third_party/WebKit/Source/core/frame/LocalDOMWindow.h index ad9a4cb..2a12b79 100644 --- a/third_party/WebKit/Source/core/frame/LocalDOMWindow.h +++ b/third_party/WebKit/Source/core/frame/LocalDOMWindow.h
@@ -42,7 +42,6 @@ class CustomElementRegistry; class DOMWindowEventQueue; -class DOMWindowProperty; class DocumentInit; class EventQueue; class FrameConsole; @@ -172,9 +171,6 @@ CustomElementRegistry* customElements() const; CustomElementRegistry* maybeCustomElements() const; - void registerProperty(DOMWindowProperty*); - void unregisterProperty(DOMWindowProperty*); - void registerEventListenerObserver(EventListenerObserver*); void frameDestroyed(); @@ -267,8 +263,6 @@ bool m_shouldPrintWhenFinishedLoading; - HeapHashSet<WeakMember<DOMWindowProperty>> m_properties; - mutable Member<Screen> m_screen; mutable Member<History> m_history; mutable Member<BarProp> m_locationbar;
diff --git a/third_party/WebKit/Source/core/frame/Location.h b/third_party/WebKit/Source/core/frame/Location.h index bcf777c..2618f79 100644 --- a/third_party/WebKit/Source/core/frame/Location.h +++ b/third_party/WebKit/Source/core/frame/Location.h
@@ -33,7 +33,6 @@ #include "bindings/core/v8/ScriptWrappable.h" #include "core/CoreExport.h" #include "core/dom/DOMStringList.h" -#include "core/frame/DOMWindowProperty.h" #include "wtf/text/WTFString.h" namespace blink { @@ -44,11 +43,9 @@ class KURL; // This class corresponds to the JS Location API, which is the only DOM API -// besides Window that is operable in a RemoteFrame. Rather than making -// DOMWindowProperty support RemoteFrames and generating a lot code churn, -// Location is implemented as a one-off with some custom lifetime management -// code. Namely, it needs a manual call to reset() from DOMWindow::reset() to -// ensure it doesn't retain a stale Frame pointer. +// besides Window that is operable in a RemoteFrame. Location needs to be +// manually updated in DOMWindow::reset() to ensure it doesn't retain a stale +// Frame pointer. class CORE_EXPORT Location final : public GarbageCollected<Location>, public ScriptWrappable { DEFINE_WRAPPERTYPEINFO();
diff --git a/third_party/WebKit/Source/core/page/ChromeClient.h b/third_party/WebKit/Source/core/page/ChromeClient.h index 4788dfc..e1785e59 100644 --- a/third_party/WebKit/Source/core/page/ChromeClient.h +++ b/third_party/WebKit/Source/core/page/ChromeClient.h
@@ -311,7 +311,7 @@ virtual void didCancelCompositionOnSelectionChange() {} virtual void resetInputMethod() {} virtual void didUpdateTextOfFocusedElementByNonUserInput(LocalFrame&) {} - virtual void showImeIfNeeded() {} + virtual void showVirtualKeyboard() {} virtual void registerViewportLayers() const {}
diff --git a/third_party/WebKit/Source/modules/quota/WorkerNavigatorStorageQuota.h b/third_party/WebKit/Source/modules/quota/WorkerNavigatorStorageQuota.h index 2a7279b..581bfc3c 100644 --- a/third_party/WebKit/Source/modules/quota/WorkerNavigatorStorageQuota.h +++ b/third_party/WebKit/Source/modules/quota/WorkerNavigatorStorageQuota.h
@@ -31,7 +31,6 @@ #ifndef WorkerNavigatorStorageQuota_h #define WorkerNavigatorStorageQuota_h -#include "core/frame/DOMWindowProperty.h" #include "core/workers/WorkerNavigator.h" #include "modules/quota/DeprecatedStorageQuota.h" #include "platform/Supplementable.h"
diff --git a/third_party/WebKit/Source/web/ChromeClientImpl.cpp b/third_party/WebKit/Source/web/ChromeClientImpl.cpp index 3fa95b1..11d980e8 100644 --- a/third_party/WebKit/Source/web/ChromeClientImpl.cpp +++ b/third_party/WebKit/Source/web/ChromeClientImpl.cpp
@@ -1023,9 +1023,9 @@ m_webView->client()->resetInputMethod(); } -void ChromeClientImpl::showImeIfNeeded() { +void ChromeClientImpl::showVirtualKeyboard() { if (m_webView->client()) - m_webView->client()->showImeIfNeeded(); + m_webView->client()->showVirtualKeyboard(); } void ChromeClientImpl::showUnhandledTapUIIfNeeded(
diff --git a/third_party/WebKit/Source/web/ChromeClientImpl.h b/third_party/WebKit/Source/web/ChromeClientImpl.h index 33b1194..4cf59e7 100644 --- a/third_party/WebKit/Source/web/ChromeClientImpl.h +++ b/third_party/WebKit/Source/web/ChromeClientImpl.h
@@ -199,7 +199,7 @@ void didCancelCompositionOnSelectionChange() override; void resetInputMethod() override; - void showImeIfNeeded() override; + void showVirtualKeyboard() override; void registerViewportLayers() const override;
diff --git a/third_party/WebKit/Source/web/tests/ImeOnFocusTest.cpp b/third_party/WebKit/Source/web/tests/ImeOnFocusTest.cpp index 2d7d552..50f42c5 100644 --- a/third_party/WebKit/Source/web/tests/ImeOnFocusTest.cpp +++ b/third_party/WebKit/Source/web/tests/ImeOnFocusTest.cpp
@@ -25,18 +25,18 @@ class ImeRequestTrackingWebViewClient : public FrameTestHelpers::TestWebViewClient { public: - ImeRequestTrackingWebViewClient() : m_imeRequestCount(0) {} + ImeRequestTrackingWebViewClient() : m_virtualKeyboardRequestCount(0) {} // WebWidgetClient methods - void showImeIfNeeded() override { ++m_imeRequestCount; } + void showVirtualKeyboard() override { ++m_virtualKeyboardRequestCount; } // Local methds - void reset() { m_imeRequestCount = 0; } + void reset() { m_virtualKeyboardRequestCount = 0; } - int imeRequestCount() { return m_imeRequestCount; } + int virtualKeyboardRequestCount() { return m_virtualKeyboardRequestCount; } private: - int m_imeRequestCount; + int m_virtualKeyboardRequestCount; }; class ImeOnFocusTest : public ::testing::Test { @@ -85,7 +85,7 @@ } void ImeOnFocusTest::runImeOnFocusTest(std::string fileName, - int expectedImeRequestCount, + int expectedVirtualKeyboardRequestCount, IntPoint tapPoint, const AtomicString& focusElement, std::string frame) { @@ -100,7 +100,7 @@ if (!focusElement.isNull()) focus(focusElement); - EXPECT_EQ(0, client.imeRequestCount()); + EXPECT_EQ(0, client.virtualKeyboardRequestCount()); if (tapPoint.x() >= 0 && tapPoint.y() >= 0) sendGestureTap(webView, tapPoint); @@ -114,7 +114,8 @@ if (!focusElement.isNull()) focus(focusElement); - EXPECT_EQ(expectedImeRequestCount, client.imeRequestCount()); + EXPECT_EQ(expectedVirtualKeyboardRequestCount, + client.virtualKeyboardRequestCount()); m_webViewHelper.reset(); }
diff --git a/third_party/WebKit/public/web/WebViewClient.h b/third_party/WebKit/public/web/WebViewClient.h index 3f77867..0cafd7d 100644 --- a/third_party/WebKit/public/web/WebViewClient.h +++ b/third_party/WebKit/public/web/WebViewClient.h
@@ -276,7 +276,7 @@ void resetInputMethod() override {} WebScreenInfo screenInfo() override { return WebScreenInfo(); } void setTouchAction(WebTouchAction touchAction) override {} - void showImeIfNeeded() override {} + void showVirtualKeyboard() override {} void showUnhandledTapUIIfNeeded(const WebPoint& tappedPosition, const WebNode& tappedNode, bool pageChanged) override {}
diff --git a/third_party/WebKit/public/web/WebWidgetClient.h b/third_party/WebKit/public/web/WebWidgetClient.h index bd80104e..c4e2b44b 100644 --- a/third_party/WebKit/public/web/WebWidgetClient.h +++ b/third_party/WebKit/public/web/WebWidgetClient.h
@@ -143,8 +143,8 @@ // the embedder of the touch actions that are permitted for this touch. virtual void setTouchAction(WebTouchAction touchAction) {} - // Request the browser to show the IME for current input type. - virtual void showImeIfNeeded() {} + // Request the browser to show virtual keyboard for current input type. + virtual void showVirtualKeyboard() {} // Request that the browser show a UI for an unhandled tap, if needed. // Invoked during the handling of a GestureTap input event whenever the
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 82b7e74c3..5ff826f91 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml
@@ -19098,6 +19098,12 @@ <summary>The volume type where quick view is opened.</summary> </histogram> +<histogram name="FileBrowser.QuickView.WayToOpen" + enum="FileManagerQuickViewWayToOpen"> + <owner>oka@google.com</owner> + <summary>How quick view was opened.</summary> +</histogram> + <histogram name="FileBrowser.SuggestApps.Close" enum="SuggestAppsDialogCloseReason"> <owner>joshwoodward@google.com</owner> @@ -89859,6 +89865,11 @@ <int value="6" label="Error"/> </enum> +<enum name="FileManagerQuickViewWayToOpen" type="int"> + <int value="0" label="Context menu"/> + <int value="1" label="Space key"/> +</enum> + <enum name="FileManagerVolumeType" type="int"> <int value="0" label="Google Drive"/> <int value="1" label="Download Folder"/> @@ -89866,6 +89877,7 @@ <int value="3" label="Archive File"/> <int value="4" label="FileSystemProvider API"/> <int value="5" label="MTP (Media Transfer Protocol) Device"/> + <int value="6" label="Media View"/> </enum> <enum name="FileMetricsProviderAccessResult" type="int">
diff --git a/ui/accessibility/platform/ax_platform_node_mac.mm b/ui/accessibility/platform/ax_platform_node_mac.mm index 0cabd69..5bfa69c 100644 --- a/ui/accessibility/platform/ax_platform_node_mac.mm +++ b/ui/accessibility/platform/ax_platform_node_mac.mm
@@ -12,7 +12,9 @@ #include "ui/accessibility/ax_action_data.h" #include "ui/accessibility/ax_node_data.h" #include "ui/accessibility/platform/ax_platform_node_delegate.h" +#include "ui/base/l10n/l10n_util.h" #import "ui/gfx/mac/coordinate_conversion.h" +#include "ui/strings/grit/ui_strings.h" namespace { @@ -371,8 +373,7 @@ // Allow certain attributes to be written via an accessibility client. A // writable attribute will only appear as such if the accessibility element // has a value set for that attribute. - if ([attributeName isEqualToString:NSAccessibilitySelectedAttribute] || - [attributeName + if ([attributeName isEqualToString:NSAccessibilitySelectedChildrenAttribute] || [attributeName isEqualToString:NSAccessibilitySelectedTextRangeAttribute] || @@ -381,10 +382,18 @@ return NO; } + // Since tabs use the Radio Button role on Mac, the standard way to set them + // is via the value attribute rather than the selected attribute. + if ([attributeName isEqualToString:NSAccessibilityValueAttribute] && + node_->GetData().role == ui::AX_ROLE_TAB) { + return !node_->GetData().HasStateFlag(ui::AX_STATE_SELECTED); + } + if ([attributeName isEqualToString:NSAccessibilityValueAttribute] || - [attributeName isEqualToString:NSAccessibilitySelectedTextAttribute]) + [attributeName isEqualToString:NSAccessibilitySelectedTextAttribute]) { return !ui::AXNodeData::IsFlagSet(node_->GetData().state, ui::AX_STATE_READ_ONLY); + } if ([attributeName isEqualToString:NSAccessibilityFocusedAttribute]) { return ui::AXNodeData::IsFlagSet(node_->GetData().state, @@ -398,21 +407,26 @@ - (void)accessibilitySetValue:(id)value forAttribute:(NSString*)attribute { ui::AXActionData data; - if ([value isKindOfClass:[NSString class]]) { - data.value = base::SysNSStringToUTF16(value); - if ([attribute isEqualToString:NSAccessibilityValueAttribute]) { - data.action = ui::AX_ACTION_SET_VALUE; - } else if ([attribute - isEqualToString:NSAccessibilitySelectedTextAttribute]) { - data.action = ui::AX_ACTION_REPLACE_SELECTED_TEXT; - } - } else if ([value isKindOfClass:[NSNumber class]]) { - if ([attribute isEqualToString:NSAccessibilityFocusedAttribute]) { + + // Check for attributes first. Only the |data.action| should be set here - any + // type-specific information, if needed, should be set below. + if ([attribute isEqualToString:NSAccessibilityValueAttribute]) { + data.action = node_->GetData().role == ui::AX_ROLE_TAB + ? ui::AX_ACTION_SET_SELECTION + : ui::AX_ACTION_SET_VALUE; + } else if ([attribute isEqualToString:NSAccessibilitySelectedTextAttribute]) { + data.action = ui::AX_ACTION_REPLACE_SELECTED_TEXT; + } else if ([attribute isEqualToString:NSAccessibilityFocusedAttribute]) { + if ([value isKindOfClass:[NSNumber class]]) { data.action = [value boolValue] ? ui::AX_ACTION_FOCUS : ui::AX_ACTION_BLUR; } } + // Set type-specific information as necessary for actions set above. + if ([value isKindOfClass:[NSString class]]) + data.value = base::SysNSStringToUTF16(value); + if (data.action != ui::AX_ACTION_NONE) node_->GetDelegate()->AccessibilityPerformAction(data); @@ -437,6 +451,15 @@ } - (NSString*)AXRoleDescription { + switch (node_->GetData().role) { + case ui::AX_ROLE_TAB: + // There is no NSAccessibilityTabRole or similar (AXRadioButton is used + // instead). Do the same as NSTabView and put "tab" in the description. + return [l10n_util::GetNSStringWithFixup(IDS_ACCNAME_TAB_ROLE_DESCRIPTION) + lowercaseString]; + default: + break; + } return NSAccessibilityRoleDescription([self AXRole], [self AXSubrole]); } @@ -458,7 +481,9 @@ return [self getStringAttribute:ui::AX_ATTR_DESCRIPTION]; } -- (NSString*)AXValue { +- (id)AXValue { + if (node_->GetData().role == ui::AX_ROLE_TAB) + return [self AXSelected]; return [self getStringAttribute:ui::AX_ATTR_VALUE]; } @@ -514,6 +539,11 @@ // Misc attributes. +- (NSNumber*)AXSelected { + return [NSNumber + numberWithBool:node_->GetData().HasStateFlag(ui::AX_STATE_SELECTED)]; +} + - (NSString*)AXPlaceholderValue { return [self getStringAttribute:ui::AX_ATTR_PLACEHOLDER]; }
diff --git a/ui/file_manager/file_manager/background/js/volume_info_list_impl.js b/ui/file_manager/file_manager/background/js/volume_info_list_impl.js index 08b4f5ea..b3981cc 100644 --- a/ui/file_manager/file_manager/background/js/volume_info_list_impl.js +++ b/ui/file_manager/file_manager/background/js/volume_info_list_impl.js
@@ -37,13 +37,27 @@ VolumeManagerCommon.VolumeType.REMOVABLE, VolumeManagerCommon.VolumeType.MTP, VolumeManagerCommon.VolumeType.PROVIDED, + VolumeManagerCommon.VolumeType.MEDIA_VIEW, +]; + +/** + * The order of the media view roots. + * @type {Array<VolumeManagerCommon.MediaViewRootType>} + * @const + * @private + */ +VolumeInfoListImpl.MEDIA_VIEW_ROOT_ORDER_ = [ + VolumeManagerCommon.MediaViewRootType.IMAGES, + VolumeManagerCommon.MediaViewRootType.VIDEOS, + VolumeManagerCommon.MediaViewRootType.AUDIO, ]; /** * Orders two volumes by volumeType and volumeId. * * The volumes at first are compared by volume type in the order of - * volumeListOrder_. Then they are compared by volume ID. + * volumeListOrder_. Then they are compared by volume ID, except for media + * views which are sorted in a fixed order. * * @param {!VolumeInfo} volumeInfo1 Volume info to be compared. * @param {!VolumeInfo} volumeInfo2 Volume info to be compared. @@ -58,6 +72,17 @@ VolumeInfoListImpl.VOLUME_LIST_ORDER_.indexOf(volumeInfo2.volumeType); if (typeIndex1 !== typeIndex2) return typeIndex1 < typeIndex2 ? -1 : 1; + if (volumeInfo1.volumeType === VolumeManagerCommon.VolumeType.MEDIA_VIEW) { + var mediaTypeIndex1 = VolumeInfoListImpl.MEDIA_VIEW_ROOT_ORDER_.indexOf( + VolumeManagerCommon.getMediaViewRootTypeFromVolumeId( + volumeInfo1.volumeId)); + var mediaTypeIndex2 = VolumeInfoListImpl.MEDIA_VIEW_ROOT_ORDER_.indexOf( + VolumeManagerCommon.getMediaViewRootTypeFromVolumeId( + volumeInfo2.volumeId)); + if (mediaTypeIndex1 !== mediaTypeIndex2) + return mediaTypeIndex1 < mediaTypeIndex2 ? -1 : 1; + return 0; + } if (volumeInfo1.volumeId !== volumeInfo2.volumeId) return volumeInfo1.volumeId < volumeInfo2.volumeId ? -1 : 1; return 0;
diff --git a/ui/file_manager/file_manager/background/js/volume_manager_impl.js b/ui/file_manager/file_manager/background/js/volume_manager_impl.js index e6463bec..7288036 100644 --- a/ui/file_manager/file_manager/background/js/volume_manager_impl.js +++ b/ui/file_manager/file_manager/background/js/volume_manager_impl.js
@@ -331,6 +331,9 @@ case VolumeManagerCommon.VolumeType.PROVIDED: rootType = VolumeManagerCommon.RootType.PROVIDED; break; + case VolumeManagerCommon.VolumeType.MEDIA_VIEW: + rootType = VolumeManagerCommon.RootType.MEDIA_VIEW; + break; default: // Programming error, throw an exception. throw new Error('Invalid volume type: ' + volumeInfo.volumeType);
diff --git a/ui/file_manager/file_manager/background/js/volume_manager_util.js b/ui/file_manager/file_manager/background/js/volume_manager_util.js index ec6abc2..a02ce6a9 100644 --- a/ui/file_manager/file_manager/background/js/volume_manager_util.js +++ b/ui/file_manager/file_manager/background/js/volume_manager_util.js
@@ -63,6 +63,20 @@ case VolumeManagerCommon.VolumeType.DRIVE: localizedLabel = str('DRIVE_DIRECTORY_LABEL'); break; + case VolumeManagerCommon.VolumeType.MEDIA_VIEW: + switch (VolumeManagerCommon.getMediaViewRootTypeFromVolumeId( + volumeMetadata.volumeId)) { + case VolumeManagerCommon.MediaViewRootType.IMAGES: + localizedLabel = str('MEDIA_VIEW_IMAGES_ROOT_LABEL'); + break; + case VolumeManagerCommon.MediaViewRootType.VIDEOS: + localizedLabel = str('MEDIA_VIEW_VIDEOS_ROOT_LABEL'); + break; + case VolumeManagerCommon.MediaViewRootType.AUDIO: + localizedLabel = str('MEDIA_VIEW_AUDIO_ROOT_LABEL'); + break; + } + break; default: // TODO(mtomasz): Calculate volumeLabel for all types of volumes in the // C++ layer.
diff --git a/ui/file_manager/file_manager/common/js/util.js b/ui/file_manager/file_manager/common/js/util.js index f5a0455..d7b0b83 100644 --- a/ui/file_manager/file_manager/common/js/util.js +++ b/ui/file_manager/file_manager/common/js/util.js
@@ -940,6 +940,20 @@ return str('DRIVE_SHARED_WITH_ME_COLLECTION_LABEL'); case VolumeManagerCommon.RootType.DRIVE_RECENT: return str('DRIVE_RECENT_COLLECTION_LABEL'); + case VolumeManagerCommon.RootType.MEDIA_VIEW: + var mediaViewRootType = + VolumeManagerCommon.getMediaViewRootTypeFromVolumeId( + locationInfo.volumeInfo.volumeId); + switch (mediaViewRootType) { + case VolumeManagerCommon.MediaViewRootType.IMAGES: + return str('MEDIA_VIEW_IMAGES_ROOT_LABEL'); + case VolumeManagerCommon.MediaViewRootType.VIDEOS: + return str('MEDIA_VIEW_VIDEOS_ROOT_LABEL'); + case VolumeManagerCommon.MediaViewRootType.AUDIO: + return str('MEDIA_VIEW_AUDIO_ROOT_LABEL'); + } + console.error('Unsupported media view root type: ' + mediaViewRootType); + return locationInfo.volumeInfo.label; case VolumeManagerCommon.RootType.DRIVE_OTHER: case VolumeManagerCommon.RootType.ARCHIVE: case VolumeManagerCommon.RootType.REMOVABLE:
diff --git a/ui/file_manager/file_manager/common/js/volume_manager_common.js b/ui/file_manager/file_manager/common/js/volume_manager_common.js index 03b72e4..2085eac 100644 --- a/ui/file_manager/file_manager/common/js/volume_manager_common.js +++ b/ui/file_manager/file_manager/common/js/volume_manager_common.js
@@ -54,7 +54,10 @@ DRIVE_SHARED_WITH_ME: 'drive_shared_with_me', // Fake root for recent files on the drive. - DRIVE_RECENT: 'drive_recent' + DRIVE_RECENT: 'drive_recent', + + // Root for media views. + MEDIA_VIEW: 'media_view', }; Object.freeze(VolumeManagerCommon.RootType); @@ -132,7 +135,8 @@ REMOVABLE: 'removable', ARCHIVE: 'archive', MTP: 'mtp', - PROVIDED: 'provided' + PROVIDED: 'provided', + MEDIA_VIEW: 'media_view', }; /** @@ -184,6 +188,8 @@ return VolumeManagerCommon.VolumeType.MTP; case VolumeManagerCommon.RootType.PROVIDED: return VolumeManagerCommon.VolumeType.PROVIDED; + case VolumeManagerCommon.RootType.MEDIA_VIEW: + return VolumeManagerCommon.VolumeType.MEDIA_VIEW; } assertNotReached('Unknown root type: ' + rootType); }; @@ -212,6 +218,31 @@ VolumeManagerCommon.VolumeInfoProvider.prototype.getVolumeInfo; /** + * List of media view root types. + * + * Keep this in sync with constants in arc_media_view_util.cc. + * + * @enum {string} + * @const + */ +VolumeManagerCommon.MediaViewRootType = { + IMAGES: 'images_root', + VIDEOS: 'videos_root', + AUDIO: 'audio_root', +}; +Object.freeze(VolumeManagerCommon.MediaViewRootType); + +/** + * Obtains volume type from root type. + * @param {string} volumeId Volume ID. + * @return {VolumeManagerCommon.MediaViewRootType} + */ +VolumeManagerCommon.getMediaViewRootTypeFromVolumeId = function(volumeId) { + return /** @type {VolumeManagerCommon.MediaViewRootType} */ ( + volumeId.split(':', 2)[1]); +}; + +/** * Fake entries for Google Drive's virtual folders. * (OFFLINE, RECENT, and SHARED_WITH_ME) * @typedef {{
diff --git a/ui/file_manager/file_manager/foreground/css/file_types.css b/ui/file_manager/file_manager/foreground/css/file_types.css index 66ec654..151de60 100644 --- a/ui/file_manager/file_manager/foreground/css/file_types.css +++ b/ui/file_manager/file_manager/foreground/css/file_types.css
@@ -306,6 +306,45 @@ url(../images/volumes/2x/cd_active.png) 2x); } +[volume-type-icon='media_view'][volume-subtype='images_root'] { + background-image: -webkit-image-set( + url(../images/volumes/images.png) 1x, + url(../images/volumes/2x/images.png) 2x); +} + +.tree-row[selected] + [volume-type-icon='media_view'][volume-subtype='images_root'] { + background-image: -webkit-image-set( + url(../images/volumes/images_active.png) 1x, + url(../images/volumes/2x/images_active.png) 2x); +} + +[volume-type-icon='media_view'][volume-subtype='videos_root'] { + background-image: -webkit-image-set( + url(../images/volumes/videos.png) 1x, + url(../images/volumes/2x/videos.png) 2x); +} + +.tree-row[selected] + [volume-type-icon='media_view'][volume-subtype='videos_root'] { + background-image: -webkit-image-set( + url(../images/volumes/videos_active.png) 1x, + url(../images/volumes/2x/videos_active.png) 2x); +} + +[volume-type-icon='media_view'][volume-subtype='audio_root'] { + background-image: -webkit-image-set( + url(../images/volumes/audio.png) 1x, + url(../images/volumes/2x/audio.png) 2x); +} + +.tree-row[selected] + [volume-type-icon='media_view'][volume-subtype='audio_root'] { + background-image: -webkit-image-set( + url(../images/volumes/audio_active.png) 1x, + url(../images/volumes/2x/audio_active.png) 2x); +} + [volume-type-icon='mtp'] { background-image: -webkit-image-set( url(../images/volumes/phone.png) 1x,
diff --git a/ui/file_manager/file_manager/foreground/images/volumes/2x/audio.png b/ui/file_manager/file_manager/foreground/images/volumes/2x/audio.png new file mode 100644 index 0000000..94f018f --- /dev/null +++ b/ui/file_manager/file_manager/foreground/images/volumes/2x/audio.png Binary files differ
diff --git a/ui/file_manager/file_manager/foreground/images/volumes/2x/audio_active.png b/ui/file_manager/file_manager/foreground/images/volumes/2x/audio_active.png new file mode 100644 index 0000000..e4f9d4d --- /dev/null +++ b/ui/file_manager/file_manager/foreground/images/volumes/2x/audio_active.png Binary files differ
diff --git a/ui/file_manager/file_manager/foreground/images/volumes/2x/images.png b/ui/file_manager/file_manager/foreground/images/volumes/2x/images.png new file mode 100644 index 0000000..685ed43 --- /dev/null +++ b/ui/file_manager/file_manager/foreground/images/volumes/2x/images.png Binary files differ
diff --git a/ui/file_manager/file_manager/foreground/images/volumes/2x/images_active.png b/ui/file_manager/file_manager/foreground/images/volumes/2x/images_active.png new file mode 100644 index 0000000..f0a9a84 --- /dev/null +++ b/ui/file_manager/file_manager/foreground/images/volumes/2x/images_active.png Binary files differ
diff --git a/ui/file_manager/file_manager/foreground/images/volumes/2x/videos.png b/ui/file_manager/file_manager/foreground/images/volumes/2x/videos.png new file mode 100644 index 0000000..3c23e32f --- /dev/null +++ b/ui/file_manager/file_manager/foreground/images/volumes/2x/videos.png Binary files differ
diff --git a/ui/file_manager/file_manager/foreground/images/volumes/2x/videos_active.png b/ui/file_manager/file_manager/foreground/images/volumes/2x/videos_active.png new file mode 100644 index 0000000..e774fc15 --- /dev/null +++ b/ui/file_manager/file_manager/foreground/images/volumes/2x/videos_active.png Binary files differ
diff --git a/ui/file_manager/file_manager/foreground/images/volumes/audio.png b/ui/file_manager/file_manager/foreground/images/volumes/audio.png new file mode 100644 index 0000000..7ad30ec --- /dev/null +++ b/ui/file_manager/file_manager/foreground/images/volumes/audio.png Binary files differ
diff --git a/ui/file_manager/file_manager/foreground/images/volumes/audio_active.png b/ui/file_manager/file_manager/foreground/images/volumes/audio_active.png new file mode 100644 index 0000000..1e4ea54 --- /dev/null +++ b/ui/file_manager/file_manager/foreground/images/volumes/audio_active.png Binary files differ
diff --git a/ui/file_manager/file_manager/foreground/images/volumes/images.png b/ui/file_manager/file_manager/foreground/images/volumes/images.png new file mode 100644 index 0000000..7bb644f5 --- /dev/null +++ b/ui/file_manager/file_manager/foreground/images/volumes/images.png Binary files differ
diff --git a/ui/file_manager/file_manager/foreground/images/volumes/images_active.png b/ui/file_manager/file_manager/foreground/images/volumes/images_active.png new file mode 100644 index 0000000..92a67cf --- /dev/null +++ b/ui/file_manager/file_manager/foreground/images/volumes/images_active.png Binary files differ
diff --git a/ui/file_manager/file_manager/foreground/images/volumes/videos.png b/ui/file_manager/file_manager/foreground/images/volumes/videos.png new file mode 100644 index 0000000..bef8132 --- /dev/null +++ b/ui/file_manager/file_manager/foreground/images/volumes/videos.png Binary files differ
diff --git a/ui/file_manager/file_manager/foreground/images/volumes/videos_active.png b/ui/file_manager/file_manager/foreground/images/volumes/videos_active.png new file mode 100644 index 0000000..3bc025d --- /dev/null +++ b/ui/file_manager/file_manager/foreground/images/volumes/videos_active.png Binary files differ
diff --git a/ui/file_manager/file_manager/foreground/js/quick_view_controller.js b/ui/file_manager/file_manager/foreground/js/quick_view_controller.js index 25667c7..7435dc6 100644 --- a/ui/file_manager/file_manager/foreground/js/quick_view_controller.js +++ b/ui/file_manager/file_manager/foreground/js/quick_view_controller.js
@@ -90,7 +90,7 @@ 'keydown', this.onKeyDownToOpen_.bind(this)); this.listContainer_.element.addEventListener('command', function(event) { if(event.command.id === 'get-info') - this.display_(); + this.display_(QuickViewUma.WayToOpen.CONTEXT_MENU); }.bind(this)); } @@ -151,7 +151,7 @@ return; if (event.key === ' ') { event.preventDefault(); - this.display_(); + this.display_(QuickViewUma.WayToOpen.SPACE_KEY); } }; @@ -188,13 +188,15 @@ /** * Display quick view. * + * @param {QuickViewUma.WayToOpen=} opt_wayToOpen in which way opening of + * quick view was triggered. Can be omitted if quick view is already open. * @private */ -QuickViewController.prototype.display_ = function() { +QuickViewController.prototype.display_ = function(opt_wayToOpen) { this.updateQuickView_().then(function() { if (!this.quickView_.isOpened()) { this.quickView_.open(); - this.quickViewUma_.onOpened(this.entries_[0]); + this.quickViewUma_.onOpened(this.entries_[0], assert(opt_wayToOpen)); } }.bind(this)); };
diff --git a/ui/file_manager/file_manager/foreground/js/quick_view_uma.js b/ui/file_manager/file_manager/foreground/js/quick_view_uma.js index 66d7df66..1391070 100644 --- a/ui/file_manager/file_manager/foreground/js/quick_view_uma.js +++ b/ui/file_manager/file_manager/foreground/js/quick_view_uma.js
@@ -25,6 +25,27 @@ } /** + * In which way quick view was opened. + * @enum {string} + * @const + */ +QuickViewUma.WayToOpen = { + CONTEXT_MENU: 'contextMenu', + SPACE_KEY: 'spaceKey', +}; + +/** + * The order should be consistnet with the definition in histograms.xml. + * + * @const {!Array<QuickViewUma.WayToOpen>} + * @private + */ +QuickViewUma.WayToOpenValues_ = [ + QuickViewUma.WayToOpen.CONTEXT_MENU, + QuickViewUma.WayToOpen.SPACE_KEY, +]; + +/** * Keep the order of this in sync with FileManagerVolumeType in * tools/metrics/histograms/histograms.xml. * @@ -38,6 +59,7 @@ VolumeManagerCommon.VolumeType.ARCHIVE, VolumeManagerCommon.VolumeType.PROVIDED, VolumeManagerCommon.VolumeType.MTP, + VolumeManagerCommon.VolumeType.MEDIA_VIEW, ]; /** @@ -69,9 +91,13 @@ * Exports UMA based on the entry selected when Quick View is opened. * * @param {!FileEntry} entry + * @param {QuickViewUma.WayToOpen} wayToOpen */ -QuickViewUma.prototype.onOpened = function(entry) { +QuickViewUma.prototype.onOpened = function(entry, wayToOpen) { this.exportFileType_(entry, 'QuickView.FileTypeOnLaunch'); + metrics.recordEnum( + 'QuickView.WayToOpen', wayToOpen, QuickViewUma.WayToOpenValues_); + var volumeType = this.volumeManager_.getVolumeInfo(entry).volumeType; if (QuickViewUma.VolumeType.includes(volumeType)) { metrics.recordEnum(
diff --git a/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js b/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js index e51bae72..489ee96 100644 --- a/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js +++ b/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js
@@ -641,7 +641,14 @@ 'style', 'background-image: ' + backgroundImage); } icon.setAttribute('volume-type-icon', volumeInfo.volumeType); - icon.setAttribute('volume-subtype', volumeInfo.deviceType || ''); + if (volumeInfo.volumeType === VolumeManagerCommon.VolumeType.MEDIA_VIEW) { + icon.setAttribute( + 'volume-subtype', + VolumeManagerCommon.getMediaViewRootTypeFromVolumeId( + volumeInfo.volumeId)); + } else { + icon.setAttribute('volume-subtype', volumeInfo.deviceType || ''); + } }; /**
diff --git a/ui/strings/ui_strings.grd b/ui/strings/ui_strings.grd index 3160c1c..da3b2787 100644 --- a/ui/strings/ui_strings.grd +++ b/ui/strings/ui_strings.grd
@@ -344,6 +344,11 @@ </message> <!--Accessible name strings--> + <if expr="is_macosx"> + <message name="IDS_ACCNAME_TAB_ROLE_DESCRIPTION" desc="A generic description of a tab button's role." meaning="UI element"> + Tab + </message> + </if> <message name="IDS_APP_ACCNAME_CLOSE" desc="The accessible name for the Close button."> Close </message>
diff --git a/ui/views/BUILD.gn b/ui/views/BUILD.gn index 0c762ff..4b4ec7f 100644 --- a/ui/views/BUILD.gn +++ b/ui/views/BUILD.gn
@@ -845,6 +845,7 @@ "controls/scrollbar/scrollbar_unittest.cc", "controls/slider_unittest.cc", "controls/styled_label_unittest.cc", + "controls/tabbed_pane/tabbed_pane_accessibility_mac_unittest.mm", "controls/tabbed_pane/tabbed_pane_unittest.cc", "controls/table/table_utils_unittest.cc", "controls/table/table_view_unittest.cc",
diff --git a/ui/views/cocoa/bridged_content_view.h b/ui/views/cocoa/bridged_content_view.h index f6952a2..c04515dd 100644 --- a/ui/views/cocoa/bridged_content_view.h +++ b/ui/views/cocoa/bridged_content_view.h
@@ -39,6 +39,9 @@ // The keyDown event currently being handled, nil otherwise. NSEvent* keyDownEvent_; + // Whether there's an active key down event which is not handled yet. + BOOL hasUnhandledKeyDownEvent_; + // The last tooltip text, used to limit updates. base::string16 lastTooltipText_;
diff --git a/ui/views/cocoa/bridged_content_view.mm b/ui/views/cocoa/bridged_content_view.mm index 15ac350..3afe6232 100644 --- a/ui/views/cocoa/bridged_content_view.mm +++ b/ui/views/cocoa/bridged_content_view.mm
@@ -200,19 +200,6 @@ return substring; } -// Returns a character event corresponding to |event|. |event| must be a -// character event itself. -ui::KeyEvent GetCharacterEventFromNSEvent(NSEvent* event) { - DCHECK([event type] == NSKeyDown || [event type] == NSKeyUp); - DCHECK_EQ(1u, [[event characters] length]); - - // [NSEvent characters] already considers the pressed key modifiers. Hence - // send ui::EF_NONE as the key modifier to the KeyEvent constructor. - // E.g. For Alt+S, [NSEvent characters] is 'ß' and not 'S'. - return ui::KeyEvent([[event characters] characterAtIndex:0], - ui::KeyboardCodeFromNSEvent(event), ui::EF_NONE); -} - NSAttributedString* GetAttributedString( const gfx::DecoratedText& decorated_text) { base::scoped_nsobject<NSMutableAttributedString> str( @@ -462,12 +449,12 @@ } - (BOOL)handleUnhandledKeyDownAsKeyEvent { - if (!keyDownEvent_) + if (!hasUnhandledKeyDownEvent_) return NO; ui::KeyEvent event(keyDownEvent_); [self handleKeyEvent:&event]; - keyDownEvent_ = nil; + hasUnhandledKeyDownEvent_ = NO; return event.handled(); } @@ -506,6 +493,42 @@ text = [text string]; bool isCharacterEvent = keyDownEvent_ && [text length] == 1; + // Pass the character event to the View hierarchy. Cases this handles (non- + // exhaustive)- + // - Space key press on controls. Unlike Tab and newline which have + // corresponding action messages, an insertText: message is generated for + // the Space key (insertText:replacementRange: when there's an active + // input context). + // - Menu mnemonic selection. + // Note we create a custom character ui::KeyEvent (and not use the + // ui::KeyEvent(NSEvent*) constructor) since we can't just rely on the event + // key code to get the actual characters from the ui::KeyEvent. This for + // example is necessary for menu mnemonic selection of non-latin text. + + // Don't generate a key event when there is active composition text. These key + // down events should be consumed by the IME and not reach the Views layer. + // For example, on pressing Return to commit composition text, if we passed a + // synthetic key event to the View hierarchy, it will have the effect of + // performing the default action on the current dialog. We do not want this. + + // Also note that a single key down event can cause multiple + // insertText:replacementRange: action messages. Example, on pressing Alt+e, + // the accent (´) character is composed via setMarkedText:. Now on pressing + // the character 'r', two insertText:replacementRange: action messages are + // generated with the text value of accent (´) and 'r' respectively. The key + // down event will have characters field of length 2. The first of these + // insertText messages won't generate a KeyEvent since there'll be active + // marked text. However, a KeyEvent will be generated corresponding to 'r'. + + // Currently there seems to be no use case to pass non-character events routed + // from insertText: handlers to the View hierarchy. + if (isCharacterEvent && ![self hasMarkedText]) { + ui::KeyEvent charEvent([text characterAtIndex:0], + ui::KeyboardCodeFromNSEvent(keyDownEvent_), + ui::EF_NONE); + [self handleKeyEvent:&charEvent]; + hasUnhandledKeyDownEvent_ = NO; + } // Forward the |text| to |textInputClient_| if no menu is active. if (textInputClient_ && ![self activeMenuController]) { @@ -518,34 +541,17 @@ // modifiers. // Also, note we don't use |keyDownEvent_| to generate the synthetic - // ui::KeyEvent since for text inserted using an IME, [keyDownEvent_ - // characters] might not be the same as |text|. This is because - // |keyDownEvent_| will correspond to the event that caused the composition - // text to be confirmed, say, Return key press. + // ui::KeyEvent since for composed text, [keyDownEvent_ characters] might + // not be the same as |text|. This is because |keyDownEvent_| will + // correspond to the event that caused the composition text to be confirmed, + // say, Return key press. if (isCharacterEvent) { textInputClient_->InsertChar(ui::KeyEvent([text characterAtIndex:0], ui::VKEY_UNKNOWN, ui::EF_NONE)); } else { textInputClient_->InsertText(base::SysNSStringToUTF16(text)); } - - keyDownEvent_ = nil; // Handled. - return; - } - - // Only handle the case where no. of characters is 1. Cases not handled (not - // an exhaustive list): - // - |text| contains a unicode surrogate pair, i.e. a single grapheme which - // requires two 16 bit characters. Currently Views menu only supports - // mnemonics using a single 16 bit character, so it is ok to ignore this - // case. - // - Programmatically created events. - // - Input from IME. But this case should not occur since inputContext is - // nil. - if (isCharacterEvent) { - ui::KeyEvent charEvent = GetCharacterEventFromNSEvent(keyDownEvent_); - [self handleKeyEvent:&charEvent]; - keyDownEvent_ = nil; // Handled. + hasUnhandledKeyDownEvent_ = NO; } } @@ -804,12 +810,14 @@ - (void)keyDown:(NSEvent*)theEvent { // Convert the event into an action message, according to OSX key mappings. keyDownEvent_ = theEvent; + hasUnhandledKeyDownEvent_ = YES; [self interpretKeyEvents:@[ theEvent ]]; // If |keyDownEvent_| wasn't cleared during -interpretKeyEvents:, it wasn't // handled. Give Widget accelerators a chance to handle it. [self handleUnhandledKeyDownAsKeyEvent]; - DCHECK(!keyDownEvent_); + DCHECK(!hasUnhandledKeyDownEvent_); + keyDownEvent_ = nil; } - (void)keyUp:(NSEvent*)theEvent { @@ -1333,7 +1341,7 @@ if ([self respondsToSelector:selector]) { [self performSelector:selector withObject:nil]; - keyDownEvent_ = nil; + hasUnhandledKeyDownEvent_ = NO; return; } @@ -1412,7 +1420,7 @@ composition.underlines.push_back(ui::CompositionUnderline( 0, [text length], SK_ColorBLACK, false, SK_ColorTRANSPARENT)); textInputClient_->SetCompositionText(composition); - keyDownEvent_ = nil; // Handled. + hasUnhandledKeyDownEvent_ = NO; } - (void)unmarkText { @@ -1420,7 +1428,7 @@ return; textInputClient_->ConfirmCompositionText(); - keyDownEvent_ = nil; // Handled. + hasUnhandledKeyDownEvent_ = NO; } - (NSArray*)validAttributesForMarkedText {
diff --git a/ui/views/controls/combobox/combobox_unittest.cc b/ui/views/controls/combobox/combobox_unittest.cc index 927a76f7..1863286 100644 --- a/ui/views/controls/combobox/combobox_unittest.cc +++ b/ui/views/controls/combobox/combobox_unittest.cc
@@ -18,6 +18,7 @@ #include "ui/events/event_utils.h" #include "ui/events/keycodes/dom/dom_code.h" #include "ui/events/keycodes/keyboard_codes.h" +#include "ui/events/test/event_generator.h" #include "ui/views/controls/combobox/combobox_listener.h" #include "ui/views/test/combobox_test_api.h" #include "ui/views/test/views_test_base.h" @@ -206,7 +207,8 @@ combobox_->set_id(1); widget_ = new Widget; - Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); + Widget::InitParams params = + CreateParams(Widget::InitParams::TYPE_WINDOW_FRAMELESS); params.bounds = gfx::Rect(200, 200, 200, 200); widget_->Init(params); View* container = new View(); @@ -216,19 +218,19 @@ combobox_->RequestFocus(); combobox_->SizeToPreferredSize(); + + event_generator_ = + base::MakeUnique<ui::test::EventGenerator>(widget_->GetNativeWindow()); + event_generator_->set_target(ui::test::EventGenerator::Target::WINDOW); } protected: - void SendKeyEvent(ui::KeyboardCode key_code) { - SendKeyEventWithType(key_code, ui::ET_KEY_PRESSED); + void PressKey(ui::KeyboardCode key_code) { + event_generator_->PressKey(key_code, ui::EF_NONE); } - void SendKeyEventWithType(ui::KeyboardCode key_code, ui::EventType type) { - ui::KeyEvent event(type, key_code, ui::EF_NONE); - FocusManager* focus_manager = widget_->GetFocusManager(); - widget_->OnKeyEvent(&event); - if (!event.handled() && focus_manager) - focus_manager->OnKeyEvent(event); + void ReleaseKey(ui::KeyboardCode key_code) { + event_generator_->ReleaseKey(key_code, ui::EF_NONE); } View* GetFocusedView() { @@ -267,28 +269,30 @@ // The current menu show count. int menu_show_count_ = 0; + std::unique_ptr<ui::test::EventGenerator> event_generator_; + private: DISALLOW_COPY_AND_ASSIGN(ComboboxTest); }; TEST_F(ComboboxTest, KeyTest) { InitCombobox(nullptr, Combobox::STYLE_NORMAL); - SendKeyEvent(ui::VKEY_END); + PressKey(ui::VKEY_END); EXPECT_EQ(combobox_->selected_index() + 1, model_->GetItemCount()); - SendKeyEvent(ui::VKEY_HOME); + PressKey(ui::VKEY_HOME); EXPECT_EQ(combobox_->selected_index(), 0); - SendKeyEvent(ui::VKEY_DOWN); - SendKeyEvent(ui::VKEY_DOWN); + PressKey(ui::VKEY_DOWN); + PressKey(ui::VKEY_DOWN); EXPECT_EQ(combobox_->selected_index(), 2); - SendKeyEvent(ui::VKEY_RIGHT); + PressKey(ui::VKEY_RIGHT); EXPECT_EQ(combobox_->selected_index(), 2); - SendKeyEvent(ui::VKEY_LEFT); + PressKey(ui::VKEY_LEFT); EXPECT_EQ(combobox_->selected_index(), 2); - SendKeyEvent(ui::VKEY_UP); + PressKey(ui::VKEY_UP); EXPECT_EQ(combobox_->selected_index(), 1); - SendKeyEvent(ui::VKEY_PRIOR); + PressKey(ui::VKEY_PRIOR); EXPECT_EQ(combobox_->selected_index(), 0); - SendKeyEvent(ui::VKEY_NEXT); + PressKey(ui::VKEY_NEXT); EXPECT_EQ(combobox_->selected_index(), model_->GetItemCount() - 1); } @@ -302,7 +306,8 @@ combobox_->SetEnabled(false); widget_ = new Widget; - Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); + Widget::InitParams params = + CreateParams(Widget::InitParams::TYPE_WINDOW_FRAMELESS); params.bounds = gfx::Rect(100, 100, 100, 100); widget_->Init(params); View* container = new View(); @@ -318,17 +323,17 @@ separators.insert(2); InitCombobox(&separators, Combobox::STYLE_NORMAL); EXPECT_EQ(0, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_DOWN); + PressKey(ui::VKEY_DOWN); EXPECT_EQ(1, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_DOWN); + PressKey(ui::VKEY_DOWN); EXPECT_EQ(3, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_UP); + PressKey(ui::VKEY_UP); EXPECT_EQ(1, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_HOME); + PressKey(ui::VKEY_HOME); EXPECT_EQ(0, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_PRIOR); + PressKey(ui::VKEY_PRIOR); EXPECT_EQ(0, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_END); + PressKey(ui::VKEY_END); EXPECT_EQ(9, combobox_->selected_index()); } @@ -339,17 +344,17 @@ separators.insert(0); InitCombobox(&separators, Combobox::STYLE_NORMAL); EXPECT_EQ(1, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_DOWN); + PressKey(ui::VKEY_DOWN); EXPECT_EQ(2, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_DOWN); + PressKey(ui::VKEY_DOWN); EXPECT_EQ(3, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_UP); + PressKey(ui::VKEY_UP); EXPECT_EQ(2, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_HOME); + PressKey(ui::VKEY_HOME); EXPECT_EQ(1, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_PRIOR); + PressKey(ui::VKEY_PRIOR); EXPECT_EQ(1, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_END); + PressKey(ui::VKEY_END); EXPECT_EQ(9, combobox_->selected_index()); } @@ -360,11 +365,11 @@ separators.insert(TestComboboxModel::kItemCount - 1); InitCombobox(&separators, Combobox::STYLE_NORMAL); combobox_->SetSelectedIndex(8); - SendKeyEvent(ui::VKEY_DOWN); + PressKey(ui::VKEY_DOWN); EXPECT_EQ(8, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_UP); + PressKey(ui::VKEY_UP); EXPECT_EQ(7, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_END); + PressKey(ui::VKEY_END); EXPECT_EQ(8, combobox_->selected_index()); } @@ -378,17 +383,17 @@ separators.insert(2); InitCombobox(&separators, Combobox::STYLE_NORMAL); EXPECT_EQ(3, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_DOWN); + PressKey(ui::VKEY_DOWN); EXPECT_EQ(4, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_UP); + PressKey(ui::VKEY_UP); EXPECT_EQ(3, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_NEXT); + PressKey(ui::VKEY_NEXT); EXPECT_EQ(9, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_HOME); + PressKey(ui::VKEY_HOME); EXPECT_EQ(3, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_END); + PressKey(ui::VKEY_END); EXPECT_EQ(9, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_PRIOR); + PressKey(ui::VKEY_PRIOR); EXPECT_EQ(3, combobox_->selected_index()); } @@ -402,9 +407,9 @@ separators.insert(6); InitCombobox(&separators, Combobox::STYLE_NORMAL); combobox_->SetSelectedIndex(3); - SendKeyEvent(ui::VKEY_DOWN); + PressKey(ui::VKEY_DOWN); EXPECT_EQ(7, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_UP); + PressKey(ui::VKEY_UP); EXPECT_EQ(3, combobox_->selected_index()); } @@ -418,17 +423,17 @@ separators.insert(9); InitCombobox(&separators, Combobox::STYLE_NORMAL); combobox_->SetSelectedIndex(6); - SendKeyEvent(ui::VKEY_DOWN); + PressKey(ui::VKEY_DOWN); EXPECT_EQ(6, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_UP); + PressKey(ui::VKEY_UP); EXPECT_EQ(5, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_HOME); + PressKey(ui::VKEY_HOME); EXPECT_EQ(0, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_NEXT); + PressKey(ui::VKEY_NEXT); EXPECT_EQ(6, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_PRIOR); + PressKey(ui::VKEY_PRIOR); EXPECT_EQ(0, combobox_->selected_index()); - SendKeyEvent(ui::VKEY_END); + PressKey(ui::VKEY_END); EXPECT_EQ(6, combobox_->selected_index()); } @@ -540,7 +545,7 @@ combobox_->set_listener(&listener); // With STYLE_NORMAL, the click event is ignored. Instead the menu is shown. - SendKeyEvent(ui::VKEY_RETURN); + PressKey(ui::VKEY_RETURN); EXPECT_EQ(1, menu_show_count_); EXPECT_FALSE(listener.on_perform_action_called()); } @@ -552,7 +557,7 @@ combobox_->set_listener(&listener); // With STYLE_ACTION, the click event is notified and the menu is not shown. - SendKeyEvent(ui::VKEY_RETURN); + PressKey(ui::VKEY_RETURN); EXPECT_EQ(0, menu_show_count_); EXPECT_TRUE(listener.on_perform_action_called()); EXPECT_EQ(0, listener.perform_action_index()); @@ -565,11 +570,11 @@ combobox_->set_listener(&listener); // With STYLE_NORMAL, the click event is ignored. Instead the menu is shwon. - SendKeyEvent(ui::VKEY_SPACE); + PressKey(ui::VKEY_SPACE); EXPECT_EQ(1, menu_show_count_); EXPECT_FALSE(listener.on_perform_action_called()); - SendKeyEventWithType(ui::VKEY_SPACE, ui::ET_KEY_RELEASED); + ReleaseKey(ui::VKEY_SPACE); EXPECT_EQ(1, menu_show_count_); EXPECT_FALSE(listener.on_perform_action_called()); } @@ -582,11 +587,11 @@ // With STYLE_ACTION, the click event is notified after releasing and the menu // is not shown. - SendKeyEvent(ui::VKEY_SPACE); + PressKey(ui::VKEY_SPACE); EXPECT_EQ(0, menu_show_count_); EXPECT_FALSE(listener.on_perform_action_called()); - SendKeyEventWithType(ui::VKEY_SPACE, ui::ET_KEY_RELEASED); + ReleaseKey(ui::VKEY_SPACE); EXPECT_EQ(0, menu_show_count_); EXPECT_TRUE(listener.on_perform_action_called()); EXPECT_EQ(0, listener.perform_action_index());
diff --git a/ui/views/controls/tabbed_pane/tabbed_pane.cc b/ui/views/controls/tabbed_pane/tabbed_pane.cc index f36f68083..552c6ee 100644 --- a/ui/views/controls/tabbed_pane/tabbed_pane.cc +++ b/ui/views/controls/tabbed_pane/tabbed_pane.cc
@@ -8,6 +8,7 @@ #include "base/macros.h" #include "third_party/skia/include/core/SkPaint.h" #include "third_party/skia/include/core/SkPath.h" +#include "ui/accessibility/ax_action_data.h" #include "ui/accessibility/ax_node_data.h" #include "ui/base/default_style.h" #include "ui/base/material_design/material_design_controller.h" @@ -27,6 +28,8 @@ #include "ui/views/layout/layout_manager.h" #include "ui/views/widget/widget.h" +namespace views { + namespace { // TODO(markusheintz|msw): Use NativeTheme colors. @@ -42,9 +45,29 @@ const int kHarmonyTabStripTabHeight = 40; -} // namespace +// The View containing the text for each tab in the tab strip. +class TabLabel : public Label { + public: + explicit TabLabel(const base::string16& tab_title) + : Label(tab_title, + ui::ResourceBundle::GetSharedInstance().GetFontListWithDelta( + ui::kLabelFontSizeDelta, + gfx::Font::NORMAL, + kActiveWeight)) {} -namespace views { + // Label: + void GetAccessibleNodeData(ui::AXNodeData* data) override { + // views::Tab shouldn't expose any of its children in the a11y tree. + // Instead, it should provide the a11y information itself. Normally, + // non-keyboard-focusable children of keyboard-focusable parents are + // ignored, but Tabs only mark the currently selected tab as + // keyboard-focusable. This means all unselected Tabs expose their children + // to the a11y tree. To fix, manually ignore the children. + data->role = ui::AX_ROLE_IGNORED; + } +}; + +} // namespace // static const char TabbedPane::kViewClassName[] = "TabbedPane"; @@ -67,33 +90,6 @@ DISALLOW_COPY_AND_ASSIGN(MdTab); }; -// The tab strip shown above the tab contents. -class TabStrip : public View { - public: - // Internal class name. - static const char kViewClassName[]; - - TabStrip(); - ~TabStrip() override; - - // Called by TabStrip when the selected tab changes. This function is only - // called if |from_tab| is not null, i.e., there was a previously selected - // tab. - virtual void OnSelectedTabChanged(Tab* from_tab, Tab* to_tab); - - // Overridden from View: - const char* GetClassName() const override; - void OnPaintBorder(gfx::Canvas* canvas) override; - - Tab* GetSelectedTab() const; - Tab* GetTabAtDeltaFromSelected(int delta) const; - Tab* GetTabAtIndex(int index) const; - int GetSelectedTabIndex() const; - - private: - DISALLOW_COPY_AND_ASSIGN(TabStrip); -}; - // A subclass of TabStrip that implements the Harmony visual styling. This // class uses a BoxLayout to position tabs. class MdTabStrip : public TabStrip, public gfx::AnimationDelegate { @@ -132,12 +128,7 @@ Tab::Tab(TabbedPane* tabbed_pane, const base::string16& title, View* contents) : tabbed_pane_(tabbed_pane), - title_(new Label( - title, - ui::ResourceBundle::GetSharedInstance().GetFontListWithDelta( - ui::kLabelFontSizeDelta, - gfx::Font::NORMAL, - kActiveWeight))), + title_(new TabLabel(title)), tab_state_(TAB_ACTIVE), contents_(contents) { // Calculate this now while the font list is guaranteed to be bold. @@ -238,6 +229,24 @@ SchedulePaint(); } +void Tab::GetAccessibleNodeData(ui::AXNodeData* data) { + data->role = ui::AX_ROLE_TAB; + data->SetName(title()->text()); + data->AddStateFlag(ui::AX_STATE_SELECTABLE); + if (selected()) + data->AddStateFlag(ui::AX_STATE_SELECTED); +} + +bool Tab::HandleAccessibleAction(const ui::AXActionData& action_data) { + if (action_data.action != ui::AX_ACTION_SET_SELECTION || !enabled()) + return false; + + // It's not clear what should happen if a tab is 'deselected', so the + // AX_ACTION_SET_SELECTION action will always select the tab. + tabbed_pane_->SelectTab(this); + return true; +} + void Tab::OnFocus() { OnStateChanged(); // When the tab gains focus, send an accessibility event indicating that the
diff --git a/ui/views/controls/tabbed_pane/tabbed_pane.h b/ui/views/controls/tabbed_pane/tabbed_pane.h index 4ff3c692..aca7f69 100644 --- a/ui/views/controls/tabbed_pane/tabbed_pane.h +++ b/ui/views/controls/tabbed_pane/tabbed_pane.h
@@ -17,6 +17,11 @@ class TabbedPaneListener; class TabStrip; +namespace test { +class TabbedPaneAccessibilityMacTest; +class TabbedPaneTest; +} + // TabbedPane is a view that shows tabs. When the user clicks on a tab, the // associated view is displayed. class VIEWS_EXPORT TabbedPane : public View { @@ -61,11 +66,8 @@ friend class FocusTraversalTest; friend class Tab; friend class TabStrip; - FRIEND_TEST_ALL_PREFIXES(TabbedPaneTest, AddAndSelect); - FRIEND_TEST_ALL_PREFIXES(TabbedPaneTest, ArrowKeyBindings); - - // Get the Tab (the tabstrip view, not its content) at the valid |index|. - Tab* GetTabAt(int index); + friend class test::TabbedPaneTest; + friend class test::TabbedPaneAccessibilityMacTest; // Get the Tab (the tabstrip view, not its content) at the selected index. Tab* GetSelectedTab(); @@ -121,6 +123,8 @@ void OnGestureEvent(ui::GestureEvent* event) override; gfx::Size GetPreferredSize() const override; const char* GetClassName() const override; + void GetAccessibleNodeData(ui::AXNodeData* node_data) override; + bool HandleAccessibleAction(const ui::AXActionData& action_data) override; void OnFocus() override; void OnBlur() override; bool OnKeyPressed(const ui::KeyEvent& event) override; @@ -150,6 +154,33 @@ DISALLOW_COPY_AND_ASSIGN(Tab); }; +// The tab strip shown above the tab contents. +class TabStrip : public View { + public: + // Internal class name. + static const char kViewClassName[]; + + TabStrip(); + ~TabStrip() override; + + // Called by TabStrip when the selected tab changes. This function is only + // called if |from_tab| is not null, i.e., there was a previously selected + // tab. + virtual void OnSelectedTabChanged(Tab* from_tab, Tab* to_tab); + + // Overridden from View: + const char* GetClassName() const override; + void OnPaintBorder(gfx::Canvas* canvas) override; + + Tab* GetSelectedTab() const; + Tab* GetTabAtDeltaFromSelected(int delta) const; + Tab* GetTabAtIndex(int index) const; + int GetSelectedTabIndex() const; + + private: + DISALLOW_COPY_AND_ASSIGN(TabStrip); +}; + } // namespace views #endif // UI_VIEWS_CONTROLS_TABBED_PANE_TABBED_PANE_H_
diff --git a/ui/views/controls/tabbed_pane/tabbed_pane_accessibility_mac_unittest.mm b/ui/views/controls/tabbed_pane/tabbed_pane_accessibility_mac_unittest.mm new file mode 100644 index 0000000..d0ae166c --- /dev/null +++ b/ui/views/controls/tabbed_pane/tabbed_pane_accessibility_mac_unittest.mm
@@ -0,0 +1,164 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#import <Cocoa/Cocoa.h> + +#import "base/mac/scoped_nsobject.h" +#include "base/strings/utf_string_conversions.h" +#include "ui/gfx/geometry/point.h" +#import "ui/gfx/mac/coordinate_conversion.h" +#include "ui/views/controls/tabbed_pane/tabbed_pane.h" +#include "ui/views/test/widget_test.h" +#include "ui/views/widget/widget.h" +#import "testing/gtest_mac.h" + +namespace views { +namespace test { + +class TabbedPaneAccessibilityMacTest : public WidgetTest { + public: + TabbedPaneAccessibilityMacTest() {} + + // WidgetTest: + void SetUp() override { + WidgetTest::SetUp(); + widget_ = CreateTopLevelPlatformWidget(); + widget_->SetBounds(gfx::Rect(50, 50, 100, 100)); + tabbed_pane_ = new TabbedPane(); + tabbed_pane_->SetSize(gfx::Size(100, 100)); + + // Create two tabs and position/size them. + tabbed_pane_->AddTab(base::ASCIIToUTF16("Tab 1"), new View()); + tabbed_pane_->AddTab(base::ASCIIToUTF16("Tab 2"), new View()); + tabbed_pane_->Layout(); + + widget_->GetContentsView()->AddChildView(tabbed_pane_); + widget_->Show(); + } + + void TearDown() override { + widget_->CloseNow(); + WidgetTest::TearDown(); + } + + Tab* GetTabAt(int index) { + return static_cast<Tab*>(tabbed_pane_->tab_strip_->child_at(index)); + } + + id AttributeValueAtPoint(NSString* attribute, const gfx::Point& point) { + id value = + [A11yElementAtPoint(point) accessibilityAttributeValue:attribute]; + EXPECT_NE(nil, value); + return value; + } + + id A11yElementAtPoint(const gfx::Point& point) { + // Accessibility hit tests come in Cocoa screen coordinates. + NSPoint ns_point = gfx::ScreenPointToNSPoint(point); + return [widget_->GetNativeWindow() accessibilityHitTest:ns_point]; + } + + gfx::Point TabCenterPoint(int index) { + return GetTabAt(index)->GetBoundsInScreen().CenterPoint(); + } + + protected: + Widget* widget_ = nullptr; + TabbedPane* tabbed_pane_ = nullptr; + + private: + DISALLOW_COPY_AND_ASSIGN(TabbedPaneAccessibilityMacTest); +}; + +// Test the Tab's a11y information compared to a Cocoa NSTabViewItem. +TEST_F(TabbedPaneAccessibilityMacTest, AttributesMatchAppKit) { + // Create a Cocoa NSTabView to test against and select the first tab. + base::scoped_nsobject<NSTabView> cocoa_tab_group( + [[NSTabView alloc] initWithFrame:NSMakeRect(50, 50, 100, 100)]); + NSArray* cocoa_tabs = @[ + [[[NSTabViewItem alloc] init] autorelease], + [[[NSTabViewItem alloc] init] autorelease], + ]; + for (size_t i = 0; i < [cocoa_tabs count]; ++i) { + [cocoa_tabs[i] setLabel:[NSString stringWithFormat:@"Tab %zu", i + 1]]; + [cocoa_tab_group addTabViewItem:cocoa_tabs[i]]; + } + + // General a11y information. + EXPECT_NSEQ( + [cocoa_tabs[0] accessibilityAttributeValue:NSAccessibilityRoleAttribute], + AttributeValueAtPoint(NSAccessibilityRoleAttribute, TabCenterPoint(0))); + EXPECT_NSEQ( + [cocoa_tabs[0] + accessibilityAttributeValue:NSAccessibilityRoleDescriptionAttribute], + AttributeValueAtPoint(NSAccessibilityRoleDescriptionAttribute, + TabCenterPoint(0))); + EXPECT_NSEQ( + [cocoa_tabs[0] accessibilityAttributeValue:NSAccessibilityTitleAttribute], + AttributeValueAtPoint(NSAccessibilityTitleAttribute, TabCenterPoint(0))); + + // Compare the value attribute against native Cocoa and check it matches up + // with whether tabs are actually selected. + for (int i : {0, 1}) { + NSNumber* cocoa_value = [cocoa_tabs[i] + accessibilityAttributeValue:NSAccessibilityValueAttribute]; + // Verify that only the second tab is selected. + EXPECT_EQ(i ? 0 : 1, [cocoa_value intValue]); + EXPECT_NSEQ(cocoa_value, + AttributeValueAtPoint(NSAccessibilityValueAttribute, + TabCenterPoint(i))); + } + + // NSTabViewItem doesn't support NSAccessibilitySelectedAttribute, so don't + // compare against Cocoa here. + EXPECT_TRUE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute, + TabCenterPoint(0)) boolValue]); + EXPECT_FALSE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute, + TabCenterPoint(1)) boolValue]); +} + +// Make sure tabs can be selected by writing the value attribute. +TEST_F(TabbedPaneAccessibilityMacTest, WritableValue) { + id tab1_a11y = A11yElementAtPoint(TabCenterPoint(0)); + id tab2_a11y = A11yElementAtPoint(TabCenterPoint(1)); + + // Only unselected tabs should be writable. + EXPECT_FALSE([tab1_a11y + accessibilityIsAttributeSettable:NSAccessibilityValueAttribute]); + EXPECT_TRUE([tab2_a11y + accessibilityIsAttributeSettable:NSAccessibilityValueAttribute]); + + // Select the second tab. AXValue actually accepts any type, but for tabs, + // Cocoa uses an integer. Despite this, the Accessibility Inspector provides a + // textfield to set the value for a control, so test this with an NSString. + [tab2_a11y accessibilitySetValue:@"string" + forAttribute:NSAccessibilityValueAttribute]; + EXPECT_EQ(0, [AttributeValueAtPoint(NSAccessibilityValueAttribute, + TabCenterPoint(0)) intValue]); + EXPECT_EQ(1, [AttributeValueAtPoint(NSAccessibilityValueAttribute, + TabCenterPoint(1)) intValue]); + EXPECT_FALSE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute, + TabCenterPoint(0)) boolValue]); + EXPECT_TRUE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute, + TabCenterPoint(1)) boolValue]); + EXPECT_TRUE(GetTabAt(1)->selected()); + + // It doesn't make sense to 'deselect' a tab (i.e., without specifying another + // to select). So any value passed to -accessibilitySetValue: should select + // that tab. Try an empty string. + [tab1_a11y accessibilitySetValue:@"" + forAttribute:NSAccessibilityValueAttribute]; + EXPECT_EQ(1, [AttributeValueAtPoint(NSAccessibilityValueAttribute, + TabCenterPoint(0)) intValue]); + EXPECT_EQ(0, [AttributeValueAtPoint(NSAccessibilityValueAttribute, + TabCenterPoint(1)) intValue]); + EXPECT_TRUE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute, + TabCenterPoint(0)) boolValue]); + EXPECT_FALSE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute, + TabCenterPoint(1)) boolValue]); + EXPECT_TRUE(GetTabAt(0)->selected()); +} + +} // namespace test +} // namespace views
diff --git a/ui/views/controls/tabbed_pane/tabbed_pane_unittest.cc b/ui/views/controls/tabbed_pane/tabbed_pane_unittest.cc index e02a252..8761683 100644 --- a/ui/views/controls/tabbed_pane/tabbed_pane_unittest.cc +++ b/ui/views/controls/tabbed_pane/tabbed_pane_unittest.cc
@@ -10,121 +10,183 @@ #include "base/message_loop/message_loop.h" #include "base/strings/utf_string_conversions.h" #include "testing/gtest/include/gtest/gtest.h" +#include "ui/accessibility/ax_action_data.h" +#include "ui/accessibility/ax_enums.h" #include "ui/events/keycodes/keyboard_code_conversion.h" +#include "ui/views/accessibility/native_view_accessibility.h" +#include "ui/views/test/test_views.h" #include "ui/views/test/views_test_base.h" +#include "ui/views/widget/widget.h" using base::ASCIIToUTF16; namespace views { +namespace test { +namespace { -// A view for testing that takes a fixed preferred size upon construction. -class FixedSizeView : public View { +base::string16 DefaultTabTitle() { + return ASCIIToUTF16("tab"); +} + +} // namespace + +class TabbedPaneTest : public ViewsTestBase { public: - explicit FixedSizeView(const gfx::Size& size) - : size_(size) {} + TabbedPaneTest() { + tabbed_pane_ = base::MakeUnique<TabbedPane>(); + tabbed_pane_->set_owned_by_client(); + } - // Overridden from View: - gfx::Size GetPreferredSize() const override { return size_; } + protected: + Tab* GetTabAt(int index) { + return static_cast<Tab*>(tabbed_pane_->tab_strip_->child_at(index)); + } + + View* GetSelectedTabContentView() { + return tabbed_pane_->GetSelectedTabContentView(); + } + + void SendKeyPressToSelectedTab(ui::KeyboardCode keyboard_code) { + tabbed_pane_->GetSelectedTab()->OnKeyPressed( + ui::KeyEvent(ui::ET_KEY_PRESSED, keyboard_code, + ui::UsLayoutKeyboardCodeToDomCode(keyboard_code), 0)); + } + + std::unique_ptr<TabbedPane> tabbed_pane_; private: - const gfx::Size size_; - - DISALLOW_COPY_AND_ASSIGN(FixedSizeView); + DISALLOW_COPY_AND_ASSIGN(TabbedPaneTest); }; -typedef ViewsTestBase TabbedPaneTest; - // Tests TabbedPane::GetPreferredSize() and TabbedPane::Layout(). TEST_F(TabbedPaneTest, SizeAndLayout) { - std::unique_ptr<TabbedPane> tabbed_pane(new TabbedPane()); - View* child1 = new FixedSizeView(gfx::Size(20, 10)); - tabbed_pane->AddTab(ASCIIToUTF16("tab1"), child1); - View* child2 = new FixedSizeView(gfx::Size(5, 5)); - tabbed_pane->AddTab(ASCIIToUTF16("tab2"), child2); - tabbed_pane->SelectTabAt(0); + View* child1 = new StaticSizedView(gfx::Size(20, 10)); + tabbed_pane_->AddTab(ASCIIToUTF16("tab1"), child1); + View* child2 = new StaticSizedView(gfx::Size(5, 5)); + tabbed_pane_->AddTab(ASCIIToUTF16("tab2"), child2); + tabbed_pane_->SelectTabAt(0); - // The |tabbed_pane| implementation of Views has no border by default. + // The |tabbed_pane_| implementation of Views has no border by default. // Therefore it should be as wide as the widest tab. The native Windows // tabbed pane has a border that used up extra space. Therefore the preferred // width is larger than the largest child. - gfx::Size pref(tabbed_pane->GetPreferredSize()); + gfx::Size pref(tabbed_pane_->GetPreferredSize()); EXPECT_GE(pref.width(), 20); EXPECT_GT(pref.height(), 10); // The bounds of our children should be smaller than the tabbed pane's bounds. - tabbed_pane->SetBounds(0, 0, 100, 200); + tabbed_pane_->SetBounds(0, 0, 100, 200); RunPendingMessages(); gfx::Rect bounds(child1->bounds()); EXPECT_GT(bounds.width(), 0); - // The |tabbed_pane| has no border. Therefore the children should be as wide - // as the |tabbed_pane|. + // The |tabbed_pane_| has no border. Therefore the children should be as wide + // as the |tabbed_pane_|. EXPECT_LE(bounds.width(), 100); EXPECT_GT(bounds.height(), 0); EXPECT_LT(bounds.height(), 200); // If we switch to the other tab, it should get assigned the same bounds. - tabbed_pane->SelectTabAt(1); + tabbed_pane_->SelectTabAt(1); EXPECT_EQ(bounds, child2->bounds()); } TEST_F(TabbedPaneTest, AddAndSelect) { - std::unique_ptr<TabbedPane> tabbed_pane(new TabbedPane()); - // Add several tabs; only the first should be a selected automatically. + // Add several tabs; only the first should be selected automatically. for (int i = 0; i < 3; ++i) { View* tab = new View(); - tabbed_pane->AddTab(ASCIIToUTF16("tab"), tab); - EXPECT_EQ(i + 1, tabbed_pane->GetTabCount()); - EXPECT_EQ(0, tabbed_pane->GetSelectedTabIndex()); + tabbed_pane_->AddTab(DefaultTabTitle(), tab); + EXPECT_EQ(i + 1, tabbed_pane_->GetTabCount()); + EXPECT_EQ(0, tabbed_pane_->GetSelectedTabIndex()); } // Select each tab. - for (int i = 0; i < tabbed_pane->GetTabCount(); ++i) { - tabbed_pane->SelectTabAt(i); - EXPECT_EQ(i, tabbed_pane->GetSelectedTabIndex()); + for (int i = 0; i < tabbed_pane_->GetTabCount(); ++i) { + tabbed_pane_->SelectTabAt(i); + EXPECT_EQ(i, tabbed_pane_->GetSelectedTabIndex()); } // Add a tab at index 0, it should not be selected automatically. View* tab0 = new View(); - tabbed_pane->AddTabAtIndex(0, ASCIIToUTF16("tab0"), tab0); - EXPECT_NE(tab0, tabbed_pane->GetSelectedTabContentView()); - EXPECT_NE(0, tabbed_pane->GetSelectedTabIndex()); -} - -ui::KeyEvent MakeKeyPressedEvent(ui::KeyboardCode keyboard_code, int flags) { - return ui::KeyEvent(ui::ET_KEY_PRESSED, keyboard_code, - ui::UsLayoutKeyboardCodeToDomCode(keyboard_code), flags); + tabbed_pane_->AddTabAtIndex(0, ASCIIToUTF16("tab0"), tab0); + EXPECT_NE(tab0, GetSelectedTabContentView()); + EXPECT_NE(0, tabbed_pane_->GetSelectedTabIndex()); } TEST_F(TabbedPaneTest, ArrowKeyBindings) { - std::unique_ptr<TabbedPane> tabbed_pane(new TabbedPane()); - // Add several tabs; only the first should be a selected automatically. + // Add several tabs; only the first should be selected automatically. for (int i = 0; i < 3; ++i) { View* tab = new View(); - tabbed_pane->AddTab(ASCIIToUTF16("tab"), tab); - EXPECT_EQ(i + 1, tabbed_pane->GetTabCount()); + tabbed_pane_->AddTab(DefaultTabTitle(), tab); + EXPECT_EQ(i + 1, tabbed_pane_->GetTabCount()); } - EXPECT_EQ(0, tabbed_pane->GetSelectedTabIndex()); + EXPECT_EQ(0, tabbed_pane_->GetSelectedTabIndex()); // Right arrow should select tab 1: - tabbed_pane->GetSelectedTab()->OnKeyPressed( - MakeKeyPressedEvent(ui::VKEY_RIGHT, 0)); - EXPECT_EQ(1, tabbed_pane->GetSelectedTabIndex()); + SendKeyPressToSelectedTab(ui::VKEY_RIGHT); + EXPECT_EQ(1, tabbed_pane_->GetSelectedTabIndex()); // Left arrow should select tab 0: - tabbed_pane->GetSelectedTab()->OnKeyPressed( - MakeKeyPressedEvent(ui::VKEY_LEFT, 0)); - EXPECT_EQ(0, tabbed_pane->GetSelectedTabIndex()); + SendKeyPressToSelectedTab(ui::VKEY_LEFT); + EXPECT_EQ(0, tabbed_pane_->GetSelectedTabIndex()); // Left arrow again should wrap to tab 2: - tabbed_pane->GetSelectedTab()->OnKeyPressed( - MakeKeyPressedEvent(ui::VKEY_LEFT, 0)); - EXPECT_EQ(2, tabbed_pane->GetSelectedTabIndex()); + SendKeyPressToSelectedTab(ui::VKEY_LEFT); + EXPECT_EQ(2, tabbed_pane_->GetSelectedTabIndex()); // Right arrow again should wrap to tab 0: - tabbed_pane->GetSelectedTab()->OnKeyPressed( - MakeKeyPressedEvent(ui::VKEY_RIGHT, 0)); - EXPECT_EQ(0, tabbed_pane->GetSelectedTabIndex()); + SendKeyPressToSelectedTab(ui::VKEY_RIGHT); + EXPECT_EQ(0, tabbed_pane_->GetSelectedTabIndex()); } +// Use TabbedPane::HandleAccessibleAction() to select tabs and make sure their +// a11y information is correct. +TEST_F(TabbedPaneTest, SelectTabWithAccessibleAction) { + // Testing accessibility information requires the View to have a Widget. + Widget* widget = new Widget; + Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_WINDOW); + widget->Init(params); + widget->GetContentsView()->AddChildView(tabbed_pane_.get()); + widget->Show(); + + constexpr int kNumTabs = 3; + for (int i = 0; i < kNumTabs; ++i) { + tabbed_pane_->AddTab(DefaultTabTitle(), new View()); + } + // Check the first tab is selected. + EXPECT_EQ(0, tabbed_pane_->GetSelectedTabIndex()); + + // Check the a11y information for each tab. + for (int i = 0; i < kNumTabs; ++i) { + NativeViewAccessibility* nva = NativeViewAccessibility::Create(GetTabAt(i)); + ui::AXNodeData data = nva->GetData(); + SCOPED_TRACE(testing::Message() << "Tab at index: " << i); + EXPECT_EQ(ui::AX_ROLE_TAB, data.role); + EXPECT_EQ(DefaultTabTitle(), data.GetString16Attribute(ui::AX_ATTR_NAME)); + EXPECT_TRUE(data.HasStateFlag(ui::AX_STATE_SELECTABLE)); + EXPECT_EQ(i == 0, data.HasStateFlag(ui::AX_STATE_SELECTED)); + nva->Destroy(); + } + + ui::AXActionData action; + action.action = ui::AX_ACTION_SET_SELECTION; + // Select the first tab. + NativeViewAccessibility* nva = NativeViewAccessibility::Create(GetTabAt(0)); + nva->AccessibilityPerformAction(action); + EXPECT_EQ(0, tabbed_pane_->GetSelectedTabIndex()); + nva->Destroy(); + + // Select the second tab. + nva = NativeViewAccessibility::Create(GetTabAt(1)); + nva->AccessibilityPerformAction(action); + EXPECT_EQ(1, tabbed_pane_->GetSelectedTabIndex()); + // Select the second tab again. + nva->AccessibilityPerformAction(action); + EXPECT_EQ(1, tabbed_pane_->GetSelectedTabIndex()); + nva->Destroy(); + + widget->CloseNow(); +} + +} // namespace test } // namespace views
diff --git a/ui/views/widget/widget.cc b/ui/views/widget/widget.cc index 08578add..8b062753 100644 --- a/ui/views/widget/widget.cc +++ b/ui/views/widget/widget.cc
@@ -1303,18 +1303,24 @@ } bool Widget::SetInitialFocus(ui::WindowShowState show_state) { + FocusManager* focus_manager = GetFocusManager(); View* v = widget_delegate_->GetInitiallyFocusedView(); if (!focus_on_creation_ || show_state == ui::SHOW_STATE_INACTIVE || show_state == ui::SHOW_STATE_MINIMIZED) { // If not focusing the window now, tell the focus manager which view to // focus when the window is restored. - if (v && focus_manager_.get()) - focus_manager_->SetStoredFocusView(v); + if (v && focus_manager) + focus_manager->SetStoredFocusView(v); return true; } - if (v) + if (v) { v->RequestFocus(); - return !!v; + // If the request for focus was unsuccessful, fall back to using the first + // focusable View instead. + if (focus_manager && focus_manager->GetFocusedView() == nullptr) + focus_manager->AdvanceFocus(false); + } + return !!focus_manager->GetFocusedView(); } ////////////////////////////////////////////////////////////////////////////////
diff --git a/ui/views/window/dialog_delegate_unittest.cc b/ui/views/window/dialog_delegate_unittest.cc index 10e45c0..bf2a32e 100644 --- a/ui/views/window/dialog_delegate_unittest.cc +++ b/ui/views/window/dialog_delegate_unittest.cc
@@ -18,6 +18,10 @@ #include "ui/views/window/dialog_client_view.h" #include "ui/views/window/dialog_delegate.h" +#if defined(OS_MACOSX) +#include "ui/base/test/scoped_fake_full_keyboard_access.h" +#endif + namespace views { namespace { @@ -273,4 +277,38 @@ EXPECT_EQ(dialog()->input(), dialog()->GetFocusManager()->GetFocusedView()); } +// If the initially focused View provided is unfocusable, check the next +// available focusable View is focused. +TEST_F(DialogTest, UnfocusableInitialFocus) { +#if defined(OS_MACOSX) + // On Mac, make all buttons unfocusable by turning off full keyboard access. + // This is the more common configuration, and if a dialog has a focusable + // textfield, tree or table, that should obtain focus instead. + ui::test::ScopedFakeFullKeyboardAccess::GetInstance() + ->set_full_keyboard_access_state(false); +#endif + + DialogDelegateView* dialog = new DialogDelegateView(); + Textfield* textfield = new Textfield(); + dialog->AddChildView(textfield); + Widget* dialog_widget = + DialogDelegate::CreateDialogWidget(dialog, GetContext(), nullptr); + +#if !defined(OS_MACOSX) + // For non-Mac, turn off focusability on all the dialog's buttons manually. + // This achieves the same effect as disabling full keyboard access. + DialogClientView* dcv = dialog->GetDialogClientView(); + dcv->ok_button()->SetFocusBehavior(View::FocusBehavior::NEVER); + dcv->cancel_button()->SetFocusBehavior(View::FocusBehavior::NEVER); +#endif + + // On showing the dialog, the initially focused View will be the OK button. + // Since it is no longer focusable, focus should advance to the next focusable + // View, which is |textfield|. + dialog_widget->Show(); + EXPECT_TRUE(textfield->HasFocus()); + EXPECT_EQ(textfield, dialog->GetFocusManager()->GetFocusedView()); + dialog_widget->Close(); +} + } // namespace views
