New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 887459 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 729596



Sign in to add a comment

Move FeatureListCreation on Cast from CastBrowserMainParts::PreCreateThreads() to CastMainDelegate::PostEarlyInitialization().

Project Member Reported by hanxi@chromium.org, Sep 20

Issue description

The 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.
 
Blocking: 729596
Cc: s...@chromium.org eliribble@chromium.org
+slan, eli know most about this codepath from cast team
@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.
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.
Sounds good, thanks for the clarification.
Thanks all! I also file  crbug.com/887638  and may need your help for code review:)
Labels: -Pri-3 Pri-2
Owner: hnakashima@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 8 by bugdroid1@chromium.org, 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

Status: Fixed (was: Assigned)

Sign in to add a comment