Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(956)

Issue 1786813003: Fix bug in specialized handler logic. (Closed)

Created:
4 years, 9 months ago by Yaron
Modified:
4 years, 9 months ago
Reviewers:
Ted C
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix bug in specialized handler logic. It was too demanding requiring both authority and path rather than just one. More info in attached bug. Also begins adding unit tests for this code. BUG=591280 Committed: https://crrev.com/2bf85a70160c1408738b63c850647dd6e594290a Cr-Commit-Position: refs/heads/master@{#380970}

Patch Set 1 #

Patch Set 2 : #

Total comments: 1

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+94 lines, -2 lines) Patch
M chrome/android/java/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImpl.java View 2 chunks +2 lines, -2 lines 0 comments Download
A chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImplTest.java View 1 2 1 chunk +92 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (6 generated)
Yaron
I suspect current tests could be robolectric but this test suite if done fully probably ...
4 years, 9 months ago (2016-03-11 22:06:59 UTC) #2
Yaron
https://codereview.chromium.org/1786813003/diff/20001/chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImplTest.java File chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImplTest.java (right): https://codereview.chromium.org/1786813003/diff/20001/chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImplTest.java#newcode1 chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImplTest.java:1: // Copyright 2015 The Chromium Authors. All rights reserved. ...
4 years, 9 months ago (2016-03-11 22:07:31 UTC) #3
Ted C
lgtm
4 years, 9 months ago (2016-03-11 23:07:41 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1786813003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1786813003/20001
4 years, 9 months ago (2016-03-14 13:31:25 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1786813003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1786813003/40001
4 years, 9 months ago (2016-03-14 13:36:57 UTC) #10
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 9 months ago (2016-03-14 14:31:52 UTC) #11
commit-bot: I haz the power
4 years, 9 months ago (2016-03-14 14:32:52 UTC) #13
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/2bf85a70160c1408738b63c850647dd6e594290a
Cr-Commit-Position: refs/heads/master@{#380970}

Powered by Google App Engine
This is Rietveld 408576698