Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1914983002:
Add CHECK in case PathProviderAndroid is failing. (Closed)
DescriptionAdd CHECK in case PathProviderAndroid is failing.
In bug 600226, RenderProcessHost::Init() is returning
false. The only possible cause I can see is if the path
service returned false, so see if PathProviderAndroid
is returning false.
BUG=600226
Committed: https://crrev.com/c1a595d1736123171d216b67535303916d95874d
Cr-Commit-Position: refs/heads/master@{#389468}
Patch Set 1 #
Total comments: 2
Patch Set 2 : PCHECK #Messages
Total messages: 14 (5 generated)
Description was changed from ========== Add CHECK in case PathProviderAndroid is failing. BUG=600226 ========== to ========== Add CHECK in case PathProviderAndroid is failing. In bug 600226, RenderProcessHost::Init() is returning false. The only possible cause I can see is if the path service returned false, so see if PathProviderAndroid is returning false. BUG=600226 ==========
falken@chromium.org changed reviewers: + torne@chromium.org
torne, could you review this as android OWNER?
https://codereview.chromium.org/1914983002/diff/1/base/base_paths_android.cc File base/base_paths_android.cc (right): https://codereview.chromium.org/1914983002/diff/1/base/base_paths_android.cc#... base/base_paths_android.cc:31: << ", errno=" << errno; Use PCHECK here instead of including errno this way - it interprets errno and gives you a meaningful string, making the log easier to understand (also works on windows by using GetLastError, just for reference). I know this is just a temporary debugging thing, but it's a good general habit ;)
Thanks, revised. https://codereview.chromium.org/1914983002/diff/1/base/base_paths_android.cc File base/base_paths_android.cc (right): https://codereview.chromium.org/1914983002/diff/1/base/base_paths_android.cc#... base/base_paths_android.cc:31: << ", errno=" << errno; On 2016/04/25 09:41:54, Torne wrote: > Use PCHECK here instead of including errno this way - it interprets errno and > gives you a meaningful string, making the log easier to understand (also works > on windows by using GetLastError, just for reference). > > I know this is just a temporary debugging thing, but it's a good general habit > ;) Ah, didn't know that existed. Done. I also removed the unreachable return statement.
On 2016/04/25 12:52:58, falken wrote: > Thanks, revised. > > https://codereview.chromium.org/1914983002/diff/1/base/base_paths_android.cc > File base/base_paths_android.cc (right): > > https://codereview.chromium.org/1914983002/diff/1/base/base_paths_android.cc#... > base/base_paths_android.cc:31: << ", errno=" << errno; > On 2016/04/25 09:41:54, Torne wrote: > > Use PCHECK here instead of including errno this way - it interprets errno and > > gives you a meaningful string, making the log easier to understand (also works > > on windows by using GetLastError, just for reference). > > > > I know this is just a temporary debugging thing, but it's a good general habit > > ;) > > Ah, didn't know that existed. Done. I also removed the unreachable return > statement. Yeah, PCHECK/PLOG/DPCHECK/DPLOG are useful - the P is for "platform error". LGTM.
The CQ bit was checked by falken@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1914983002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1914983002/20001
Message was sent while issue was closed.
Description was changed from ========== Add CHECK in case PathProviderAndroid is failing. In bug 600226, RenderProcessHost::Init() is returning false. The only possible cause I can see is if the path service returned false, so see if PathProviderAndroid is returning false. BUG=600226 ========== to ========== Add CHECK in case PathProviderAndroid is failing. In bug 600226, RenderProcessHost::Init() is returning false. The only possible cause I can see is if the path service returned false, so see if PathProviderAndroid is returning false. BUG=600226 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Add CHECK in case PathProviderAndroid is failing. In bug 600226, RenderProcessHost::Init() is returning false. The only possible cause I can see is if the path service returned false, so see if PathProviderAndroid is returning false. BUG=600226 ========== to ========== Add CHECK in case PathProviderAndroid is failing. In bug 600226, RenderProcessHost::Init() is returning false. The only possible cause I can see is if the path service returned false, so see if PathProviderAndroid is returning false. BUG=600226 Committed: https://crrev.com/c1a595d1736123171d216b67535303916d95874d Cr-Commit-Position: refs/heads/master@{#389468} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/c1a595d1736123171d216b67535303916d95874d Cr-Commit-Position: refs/heads/master@{#389468}
Message was sent while issue was closed.
BTW this PCHECK is indeed getting hit: F/chromium( 6511): [FATAL:base_paths_android.cc(28)] Check failed: bin_dir_size > 0 && bin_dir_size <= PATH_MAX. Unable to resolve /proc/self/exe. bin_dir_size=-1: Permission denied ... but I'm not sure why that'd happen. If you have ideas, please comment on the bug :)
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/1959153003/ by falken@chromium.org. The reason for reverting is: We got enough crash reports, reverting now.. |
