Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2960883002:
Add installer_package_name and ABI to crash report (Closed)
DescriptionAdd installer_package_name and ABI to crash report
As the discussion of issue 722800, we should track the install source and
ABI in the bug report.
BUG=722800
Review-Url: https://codereview.chromium.org/2960883002
Cr-Commit-Position: refs/heads/master@{#483452}
Committed: https://chromium.googlesource.com/chromium/src/+/f2c67e43244683696e038cc51eb7d0dbd5c68e00
Patch Set 1 #Patch Set 2 : fix crash #Patch Set 3 : add abi field #
Total comments: 2
Patch Set 4 : fix style #
Messages
Total messages: 38 (26 generated)
The CQ bit was checked by ranj@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by ranj@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Add installer_package_name BUG=722800 ========== to ========== Add installer_package_name to crash report BUG=722800 ==========
Description was changed from ========== Add installer_package_name to crash report BUG=722800 ========== to ========== Add installer_package_name to crash report As the discussion of issue 722800, we should track the install source for chrome in the bug report. BUG=722800 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
ranj@chromium.org changed reviewers: + wnwen@chromium.org, yfriedman@chromium.org
Don't know how to test this patch, getInstallerPackageName always gives me null for local build. Not sure if it's going to change when download from play store. Any suggestion? Also should I add ABI field to bug report or we can get it from CPU Architecture?
On 2017/06/28 15:36:59, Ran wrote: > Don't know how to test this patch, getInstallerPackageName always gives me null > for local build. > Not sure if it's going to change when download from play store. > Any suggestion? Seems like that's the case, it's the name of the store that the app came from: https://developer.android.com/reference/android/content/pm/PackageManager.htm... > Also should I add ABI field to bug report or we can get it from CPU > Architecture? I think we want ABI since the CPU Architecture is for the device, whereas sometimes third party stores may serve the wrong one: https://bugs.chromium.org/p/chromium/issues/detail?id=722800#c45
On 2017/06/28 15:58:47, Peter Wen wrote: > On 2017/06/28 15:36:59, Ran wrote: > > Don't know how to test this patch, getInstallerPackageName always gives me > null > > for local build. > > Not sure if it's going to change when download from play store. > > Any suggestion? You can try testing it by doing: https://developer.android.com/reference/android/content/pm/PackageManager.htm... on say com.android.chrome to test stable. Of course only do that locally but you can verify the API use and what value is returned > > Seems like that's the case, it's the name of the store that the app came from: > https://developer.android.com/reference/android/content/pm/PackageManager.htm... > > > Also should I add ABI field to bug report or we can get it from CPU > > Architecture? > > I think we want ABI since the CPU Architecture is for the device, whereas > sometimes third party stores may serve the wrong one: > https://bugs.chromium.org/p/chromium/issues/detail?id=722800#c45
The CQ bit was checked by ranj@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Add ABI field.
On 2017/06/28 18:41:12, Ran wrote: > Add ABI field. lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm but looks like you still need a breakpad owner https://codereview.chromium.org/2960883002/diff/40001/base/android/java/src/o... File base/android/java/src/org/chromium/base/BuildInfo.java (right): https://codereview.chromium.org/2960883002/diff/40001/base/android/java/src/o... base/android/java/src/org/chromium/base/BuildInfo.java:52: String abi = Build.CPU_ABI; nit: no need for these locals, just inline.
Patchset #4 (id:60001) has been deleted
ranj@chromium.org changed reviewers: + jochen@chromium.org
+jochen for components/crash/content/app/breakpad_linux.cc https://codereview.chromium.org/2960883002/diff/40001/base/android/java/src/o... File base/android/java/src/org/chromium/base/BuildInfo.java (right): https://codereview.chromium.org/2960883002/diff/40001/base/android/java/src/o... base/android/java/src/org/chromium/base/BuildInfo.java:52: String abi = Build.CPU_ABI; On 2017/06/28 20:43:45, Yaron wrote: > nit: no need for these locals, just inline. Done.
The CQ bit was checked by ranj@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
oh, please also update the cl descriptoin
oh, please also update the cl descriptoin
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Add installer_package_name to crash report As the discussion of issue 722800, we should track the install source for chrome in the bug report. BUG=722800 ========== to ========== Add installer_package_name and ABI to crash report As the discussion of issue 722800, we should track the install source and ABI in the bug report. BUG=722800 ==========
lgtm
The CQ bit was checked by ranj@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from yfriedman@chromium.org, wnwen@chromium.org Link to the patchset: https://codereview.chromium.org/2960883002/#ps80001 (title: "fix style")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1498765155835110,
"parent_rev": "8456c95540475b61f4c894d5b9d1c5e79fcffd2f", "commit_rev":
"f2c67e43244683696e038cc51eb7d0dbd5c68e00"}
Message was sent while issue was closed.
Description was changed from ========== Add installer_package_name and ABI to crash report As the discussion of issue 722800, we should track the install source and ABI in the bug report. BUG=722800 ========== to ========== Add installer_package_name and ABI to crash report As the discussion of issue 722800, we should track the install source and ABI in the bug report. BUG=722800 Review-Url: https://codereview.chromium.org/2960883002 Cr-Commit-Position: refs/heads/master@{#483452} Committed: https://chromium.googlesource.com/chromium/src/+/f2c67e43244683696e038cc51eb7... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:80001) as https://chromium.googlesource.com/chromium/src/+/f2c67e43244683696e038cc51eb7... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
