-
Notifications
You must be signed in to change notification settings - Fork 885
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
Show connection error view if network unavailable #13593
Conversation
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.
How about early return when network is unavailalbe in the Connect()
instead of going normal path?
@@ -12,6 +12,7 @@ | |||
#include "base/json/json_writer.h" | |||
#include "base/strings/utf_string_conversions.h" | |||
#include "brave/components/skus/browser/skus_utils.h" | |||
#include "net/base/network_change_notifier.h" |
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.
How about moving this include to below #if !BUILDFLAG(IS_ANDROID)
as it's used in non-android code?
and this include doesn't need to add //net
to deps list?
Hmm, already we've included net's headers below. Curious why gn_check doesn't complain about it.
Maybe it's allowed by public_deps
of other deps? :)
I think just adding //net
to deps explicitely is sufficient.
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.
done
done |
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.
++
Failed tests are not replated to this PR |
Resolves brave/brave-browser#22754
Brave.Browser.Development.-.New.Tab.-.3.June.2022.mp4
Checking network adapters status before setting internal connection status and correct it if no network access.
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
,npm run lint
,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan:
Try again
button