Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2451093004:
Handle navigations with PAGE_TRANSITION_CLIENT_REDIRECT qualifier (Closed)
|
Created:
4 years, 1 month ago by Yusuke Sato Modified:
4 years, 1 month ago CC:
chromium-reviews, elijahtaylor+arcwatch_chromium.org, yusukes+watch_chromium.org, hidehiko+watch_chromium.org, lhchavez+watch_chromium.org, oshima+watch_chromium.org, davemoore+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionHandle navigations with PAGE_TRANSITION_CLIENT_REDIRECT qualifier
to allow the AppAuth web page to work properly with ARC.
BUG=659817
TEST=try, run the apk attached to the bug on ARC
Committed: https://crrev.com/250459af2d654e5bf411440f4a034c7e7799b7d6
Cr-Commit-Position: refs/heads/master@{#427866}
Patch Set 1 #
Total comments: 1
Messages
Total messages: 18 (10 generated)
The CQ bit was checked by yusukes@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...
yusukes@chromium.org changed reviewers: + djacobo@google.com, lhchavez@chromium.org, lhchavez@google.com
PTAL. To make this merge-friendly, I didn't write unittests for this. Will add some right after this is submitted. Also, to lower the risk, I modified only c/b/c/arc/intent_helper/arc_external_protocol_dialog.cc (rather than components/arc/intent_helper/page_transition_util.cc) and left a TODO.
lhchavez@chromium.org changed reviewers: - lhchavez@google.com
(removing the wrong lhchavez) lgtm but please use a crbug bug if you intend to merge this.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Handle navigations with PAGE_TRANSITION_CLIENT_REDIRECT qualifier to allow the AppAuth web page to work properly with ARC. BUG=b:32442730 TEST=try, run the apk attached to the bug on ARC ========== to ========== Handle navigations with PAGE_TRANSITION_CLIENT_REDIRECT qualifier to allow the AppAuth web page to work properly with ARC. BUG=659817 TEST=try, run the apk attached to the bug on ARC ==========
Updated the BUG. (Sorry, I added both because the UI didn't auto-complete lhchavez@chromium.org for some reason and thought you might have stopped using @chromium account :) The UI is working fine now and suggests the correct account.)
lgtm https://codereview.chromium.org/2451093004/diff/1/chrome/browser/chromeos/arc... File chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.cc (right): https://codereview.chromium.org/2451093004/diff/1/chrome/browser/chromeos/arc... chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.cc:355: // and move it to the function if it is. (b/32442730#comment3) I will, thanks!
The CQ bit was checked by yusukes@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Handle navigations with PAGE_TRANSITION_CLIENT_REDIRECT qualifier to allow the AppAuth web page to work properly with ARC. BUG=659817 TEST=try, run the apk attached to the bug on ARC ========== to ========== Handle navigations with PAGE_TRANSITION_CLIENT_REDIRECT qualifier to allow the AppAuth web page to work properly with ARC. BUG=659817 TEST=try, run the apk attached to the bug on ARC ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Handle navigations with PAGE_TRANSITION_CLIENT_REDIRECT qualifier to allow the AppAuth web page to work properly with ARC. BUG=659817 TEST=try, run the apk attached to the bug on ARC ========== to ========== Handle navigations with PAGE_TRANSITION_CLIENT_REDIRECT qualifier to allow the AppAuth web page to work properly with ARC. BUG=659817 TEST=try, run the apk attached to the bug on ARC Committed: https://crrev.com/250459af2d654e5bf411440f4a034c7e7799b7d6 Cr-Commit-Position: refs/heads/master@{#427866} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/250459af2d654e5bf411440f4a034c7e7799b7d6 Cr-Commit-Position: refs/heads/master@{#427866}
Message was sent while issue was closed.
On 2016/10/26 21:51:50, Yusuke Sato wrote: > PTAL. > > To make this merge-friendly, I didn't write unittests for this. Will add some > right after this is submitted. Done: https://codereview.chromium.org/2449213006/ > > Also, to lower the risk, I modified only > c/b/c/arc/intent_helper/arc_external_protocol_dialog.cc (rather than > components/arc/intent_helper/page_transition_util.cc) and left a TODO. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
