Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2953103002:
Fix Facebook attribution stats when opening links from Facebook app. (Closed)
|
Created:
3 years, 6 months ago by troyhildebrandt Modified:
3 years, 5 months ago Reviewers:
Maria CC:
chromium-reviews, agrieve+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix Facebook attribution stats when opening links from Facebook app.
Instead of using the intent's URL to determine if the app launching Chrome
is Facebook, we use the referrer URL instead and ensure it's
android-app://m.facebook.com.
BUG=733086
Review-Url: https://codereview.chromium.org/2953103002
Cr-Commit-Position: refs/heads/master@{#485028}
Committed: https://chromium.googlesource.com/chromium/src/+/393150a2245442dce462a8f1c82c5ebdbae66859
Patch Set 1 #Patch Set 2 : . #Patch Set 3 : . #
Total comments: 4
Patch Set 4 : . #Patch Set 5 : . #Patch Set 6 : Fix NPE when getting headers. #Messages
Total messages: 20 (12 generated)
Description was changed from ========== Fix Facebook attribution stats when opening links from Facebook app. Instead of using the intent's URL to determine if the app launching Chrome is Facebook, we use the referrer URL instead and ensure it's android-app://m.facebook.com. BUG=733086 ========== to ========== Fix Facebook attribution stats when opening links from Facebook app. Instead of using the intent's URL to determine if the app launching Chrome is Facebook, we use the referrer URL instead and ensure it's android-app://m.facebook.com. BUG=733086 ==========
thildebr@chromium.org changed reviewers: + mariakhomenko@chromium.org
lgtm https://codereview.chromium.org/2953103002/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java (right): https://codereview.chromium.org/2953103002/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java:7: import android.app.Activity; what's this used for? https://codereview.chromium.org/2953103002/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java:295: } else if (FACEBOOK_REFERRER_URL.equals(referrer)) { There were some cases we tested where we didn't have a referrer? Can we document here which case this captures?
https://codereview.chromium.org/2953103002/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java (right): https://codereview.chromium.org/2953103002/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java:7: import android.app.Activity; On 2017/06/22 23:23:57, Maria wrote: > what's this used for? It's for one of the Javadoc comments further down. https://codereview.chromium.org/2953103002/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java:295: } else if (FACEBOOK_REFERRER_URL.equals(referrer)) { On 2017/06/22 23:23:57, Maria wrote: > There were some cases we tested where we didn't have a referrer? Can we document > here which case this captures? I've included both the cases now, opening links in the internal browser then "Open With..." and opening externally.
lgtm
The CQ bit was checked by thildebr@chromium.org
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
Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
The CQ bit was checked by thildebr@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: This issue passed the CQ dry run.
The CQ bit was checked by thildebr@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mariakhomenko@chromium.org Link to the patchset: https://codereview.chromium.org/2953103002/#ps100001 (title: "Fix NPE when getting headers.")
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": 100001, "attempt_start_ts": 1499458673237250,
"parent_rev": "1f461df4f62cdd1bf5bf8db3524b795dc9ef5bd5", "commit_rev":
"393150a2245442dce462a8f1c82c5ebdbae66859"}
Message was sent while issue was closed.
Description was changed from ========== Fix Facebook attribution stats when opening links from Facebook app. Instead of using the intent's URL to determine if the app launching Chrome is Facebook, we use the referrer URL instead and ensure it's android-app://m.facebook.com. BUG=733086 ========== to ========== Fix Facebook attribution stats when opening links from Facebook app. Instead of using the intent's URL to determine if the app launching Chrome is Facebook, we use the referrer URL instead and ensure it's android-app://m.facebook.com. BUG=733086 Review-Url: https://codereview.chromium.org/2953103002 Cr-Commit-Position: refs/heads/master@{#485028} Committed: https://chromium.googlesource.com/chromium/src/+/393150a2245442dce462a8f1c82c... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as https://chromium.googlesource.com/chromium/src/+/393150a2245442dce462a8f1c82c... |
