Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2704373002:
[Local NTP] Add a test for embeddedSearch API availability (Closed)
|
Created:
3 years, 10 months ago by Marc Treib Modified:
3 years, 10 months ago Reviewers:
sfiera CC:
chromium-reviews, skanuj+watch_chromium.org, melevin+watch_chromium.org, donnd+watch_chromium.org, jfweitz+watch_chromium.org, David Black, samarth+watch_chromium.org, kmadhusu+watch_chromium.org, Jered Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Local NTP] Add a test for embeddedSearch API availability
BUG=692002
Review-Url: https://codereview.chromium.org/2704373002
Cr-Commit-Position: refs/heads/master@{#451750}
Committed: https://chromium.googlesource.com/chromium/src/+/769bc0d70e165bfea010611d8eb3065e6fa6f1d0
Patch Set 1 #
Total comments: 2
Patch Set 2 : test back/fwd #Messages
Total messages: 18 (12 generated)
The CQ bit was checked by treib@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.
treib@chromium.org changed reviewers: + sfiera@chromium.org
PTAL! This might be the last one for now.
LGTM https://codereview.chromium.org/2704373002/diff/1/chrome/browser/ui/search/lo... File chrome/browser/ui/search/local_ntp_browsertest.cc (right): https://codereview.chromium.org/2704373002/diff/1/chrome/browser/ui/search/lo... chrome/browser/ui/search/local_ntp_browsertest.cc:97: // Navigate back to an NTP. You're not actually navigating "back", right? (though that could be interesting to check too)
The CQ bit was checked by treib@chromium.org to run a CQ dry run
https://codereview.chromium.org/2704373002/diff/1/chrome/browser/ui/search/lo... File chrome/browser/ui/search/local_ntp_browsertest.cc (right): https://codereview.chromium.org/2704373002/diff/1/chrome/browser/ui/search/lo... chrome/browser/ui/search/local_ntp_browsertest.cc:97: // Navigate back to an NTP. On 2017/02/21 11:49:46, sfiera wrote: > You're not actually navigating "back", right? Right, the formulation was misleading. Changed. > (though that could be interesting to check too) Good point! I added back/fwd navigations to the test.
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.
Still LGTM
The CQ bit was checked by treib@chromium.org
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": 20001, "attempt_start_ts": 1487682734282440,
"parent_rev": "f53167270c44da971a2f085299404319e42756f4", "commit_rev":
"769bc0d70e165bfea010611d8eb3065e6fa6f1d0"}
Message was sent while issue was closed.
Description was changed from ========== [Local NTP] Add a test for embeddedSearch API availability BUG=692002 ========== to ========== [Local NTP] Add a test for embeddedSearch API availability BUG=692002 Review-Url: https://codereview.chromium.org/2704373002 Cr-Commit-Position: refs/heads/master@{#451750} Committed: https://chromium.googlesource.com/chromium/src/+/769bc0d70e165bfea010611d8eb3... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/769bc0d70e165bfea010611d8eb3... |
