Move FeatureListCreation on Cast from CastBrowserMainParts::PreCreateThreads() to CastMainDelegate::PostEarlyInitialization(). |
||||
Issue descriptionThe FeatureList should be created before starting the TaskSchedueler in ContentMainRunnerImpl. This is because TaskSchedueler needs to check a flag and requires the FeatureList is ready. Currently, the FeatureList creation on Cast is implemented in CastBrowserMainParts::PreCreateThreads(), so we should move it to CastMainDelegate::PostEarlyInitialization() which happens before TaskSchedueler starts.
,
Sep 20
+slan, eli know most about this codepath from cast team
,
Sep 20
@halliwell are you looking for someone to answer questions or to take ownership of this bug? I'm happy to take ownership, if that's what you want.
,
Sep 20
Eli - seems like some changes being made to core feature system initialisation (I added you to the code review as well), and upstream folks may need someone from our team to help (even if they write the CL, we should help to test and verify). I'll be out on vacation for a few days, so wanted to make sure you're in the loop.
,
Sep 20
Sounds good, thanks for the clarification.
,
Sep 20
Thanks all! I also file crbug.com/887638 and may need your help for code review:)
,
Oct 29
,
Nov 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/42ec7eb93a9a12dd8cf1bdfaa7ef8087eaa38e86 commit 42ec7eb93a9a12dd8cf1bdfaa7ef8087eaa38e86 Author: Henrique Nakashima <hnakashima@chromium.org> Date: Fri Nov 02 15:15:08 2018 Create FeatureList and PrefService early in Cast. TaskScheduler requires the FeatureList, so it needs to be moved from CastBrowserMainParts::PreCreateThreads() to an earlier point, CastMainDelegate::PostEarlyInitialization(). The creation of the "singleton" FieldTrialList instance also needs to be moved to CastMainDelegate::PostEarlyInitialization(), since the FeatureList initialization depends on the FieldTrialList. Its ownership was transferred from BrowserMainParts to CastMainDelegate. A new CastFeatureListCreator class holds ownership of the service while the browser process isn't started. Bug: 887459 Change-Id: I4bcd60720518807e2089ec99a3f7db3e647203d6 Reviewed-on: https://chromium-review.googlesource.com/c/1308096 Commit-Queue: Henrique Nakashima <hnakashima@chromium.org> Reviewed-by: Luke Halliwell <halliwell@chromium.org> Reviewed-by: Xi Han <hanxi@chromium.org> Cr-Commit-Position: refs/heads/master@{#604920} [modify] https://crrev.com/42ec7eb93a9a12dd8cf1bdfaa7ef8087eaa38e86/chromecast/app/cast_main_delegate.cc [modify] https://crrev.com/42ec7eb93a9a12dd8cf1bdfaa7ef8087eaa38e86/chromecast/app/cast_main_delegate.h [modify] https://crrev.com/42ec7eb93a9a12dd8cf1bdfaa7ef8087eaa38e86/chromecast/browser/BUILD.gn [modify] https://crrev.com/42ec7eb93a9a12dd8cf1bdfaa7ef8087eaa38e86/chromecast/browser/cast_browser_main_parts.cc [modify] https://crrev.com/42ec7eb93a9a12dd8cf1bdfaa7ef8087eaa38e86/chromecast/browser/cast_browser_main_parts.h [modify] https://crrev.com/42ec7eb93a9a12dd8cf1bdfaa7ef8087eaa38e86/chromecast/browser/cast_content_browser_client.cc [modify] https://crrev.com/42ec7eb93a9a12dd8cf1bdfaa7ef8087eaa38e86/chromecast/browser/cast_content_browser_client.h [modify] https://crrev.com/42ec7eb93a9a12dd8cf1bdfaa7ef8087eaa38e86/chromecast/browser/cast_content_browser_client_simple.cc [add] https://crrev.com/42ec7eb93a9a12dd8cf1bdfaa7ef8087eaa38e86/chromecast/browser/cast_feature_list_creator.cc [add] https://crrev.com/42ec7eb93a9a12dd8cf1bdfaa7ef8087eaa38e86/chromecast/browser/cast_feature_list_creator.h
,
Nov 2
|
||||
►
Sign in to add a comment |
||||
Comment 1 by hanxi@chromium.org
, Sep 20