-
Notifications
You must be signed in to change notification settings - Fork 891
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix master debug build failure from BraveActionAPIDependencyManager #3796
Conversation
if (base::CommandLine::ForCurrentProcess()->HasSwitch( | ||
kDumpBraveActionaAPIDependencyGraphFlag)) { | ||
base::FilePath dot_file = | ||
static_cast<const SimpleFactoryKey*>(context)->GetPath().AppendASCII( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Context is a Browser. Can it still be cast to SimpleFactoryKey?
Otherwise I doubt we would ever use this flag as this architecture stands - there are no service dependencies so if it's any more complicated than this, we can just stub it out since we're using KeyedService here as a utility to key BraveActionAPI on Browser
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, you're right. it's wrong casting. It will cause runtime crash.
Also, made it as a stub impl.
There was a missing abstract method that we missed only for debug build.
b632a84
to
baf343c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the build fix @simonhong ⚡️
Fix master debug build failure from BraveActionAPIDependencyManager
Fix master debug build failure from BraveActionAPIDependencyManager
There was a missing abstract method that used only for debug build.
Fix brave/brave-browser#6628
Submitter Checklist:
npm run lint
)git rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
Reviewer Checklist:
After-merge Checklist:
changes has landed on.