Commit 94cbc0db authored by Moe Ahmadi's avatar Moe Ahmadi Committed by Commit Bot

[WebUI][NTP] Fixes crash in AutocompleteController::StopHelper

The crash in crbug/1086749 is likely due to the page method calls being
handled after destruction of |autocomplete_controller_|. mojo::Receiver
guarantees that page method calls will NEVER run beyond the lifetime of
the receiver object. Moving the Receiver (as well as the Remote) member
variable toward the end of the list of member variables should fix this
crash as the receiver object gets destroyed first.

Bug: 1086749
Change-Id: I42d1229416e5909c4f3b9bd187a4db2da88b7d2a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2217211
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772285}
parent c31836c2
...@@ -253,9 +253,7 @@ NewTabPageHandler::NewTabPageHandler( ...@@ -253,9 +253,7 @@ NewTabPageHandler::NewTabPageHandler(
logo_service_(LogoServiceFactory::GetForProfile(profile)), logo_service_(LogoServiceFactory::GetForProfile(profile)),
one_google_bar_service_( one_google_bar_service_(
OneGoogleBarServiceFactory::GetForProfile(profile)), OneGoogleBarServiceFactory::GetForProfile(profile)),
page_{std::move(pending_page)},
profile_(profile), profile_(profile),
receiver_{this, std::move(pending_page_handler)},
favicon_cache_(FaviconServiceFactory::GetForProfile( favicon_cache_(FaviconServiceFactory::GetForProfile(
profile, profile,
ServiceAccessType::EXPLICIT_ACCESS), ServiceAccessType::EXPLICIT_ACCESS),
...@@ -264,7 +262,9 @@ NewTabPageHandler::NewTabPageHandler( ...@@ -264,7 +262,9 @@ NewTabPageHandler::NewTabPageHandler(
ServiceAccessType::EXPLICIT_ACCESS)), ServiceAccessType::EXPLICIT_ACCESS)),
web_contents_(web_contents), web_contents_(web_contents),
ntp_navigation_start_time_(ntp_navigation_start_time), ntp_navigation_start_time_(ntp_navigation_start_time),
logger_(NTPUserDataLogger::GetOrCreateFromWebContents(web_contents)) { logger_(NTPUserDataLogger::GetOrCreateFromWebContents(web_contents)),
page_{std::move(pending_page)},
receiver_{this, std::move(pending_page_handler)} {
CHECK(instant_service_); CHECK(instant_service_);
CHECK(ntp_background_service_); CHECK(ntp_background_service_);
CHECK(logo_service_); CHECK(logo_service_);
......
...@@ -206,9 +206,7 @@ class NewTabPageHandler : public new_tab_page::mojom::PageHandler, ...@@ -206,9 +206,7 @@ class NewTabPageHandler : public new_tab_page::mojom::PageHandler,
ScopedObserver<OneGoogleBarService, OneGoogleBarServiceObserver> ScopedObserver<OneGoogleBarService, OneGoogleBarServiceObserver>
one_google_bar_service_observer_{this}; one_google_bar_service_observer_{this};
base::Optional<base::TimeTicks> one_google_bar_load_start_time_; base::Optional<base::TimeTicks> one_google_bar_load_start_time_;
mojo::Remote<new_tab_page::mojom::Page> page_;
Profile* profile_; Profile* profile_;
mojo::Receiver<new_tab_page::mojom::PageHandler> receiver_;
scoped_refptr<ui::SelectFileDialog> select_file_dialog_; scoped_refptr<ui::SelectFileDialog> select_file_dialog_;
std::unique_ptr<AutocompleteController> autocomplete_controller_; std::unique_ptr<AutocompleteController> autocomplete_controller_;
FaviconCache favicon_cache_; FaviconCache favicon_cache_;
...@@ -220,6 +218,11 @@ class NewTabPageHandler : public new_tab_page::mojom::PageHandler, ...@@ -220,6 +218,11 @@ class NewTabPageHandler : public new_tab_page::mojom::PageHandler,
std::unique_ptr<network::SimpleURLLoader>> std::unique_ptr<network::SimpleURLLoader>>
loader_map_; loader_map_;
// These are located at the end of the list of member variables to ensure the
// WebUI page is disconnected before other members are destroyed.
mojo::Remote<new_tab_page::mojom::Page> page_;
mojo::Receiver<new_tab_page::mojom::PageHandler> receiver_;
base::WeakPtrFactory<NewTabPageHandler> weak_ptr_factory_{this}; base::WeakPtrFactory<NewTabPageHandler> weak_ptr_factory_{this};
DISALLOW_COPY_AND_ASSIGN(NewTabPageHandler); DISALLOW_COPY_AND_ASSIGN(NewTabPageHandler);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment