Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1680483002:
ARC: Implement the startScan API to start scanning of WiFI APs. (Closed)
|
Created:
4 years, 10 months ago by abhishekbh Modified:
4 years, 10 months ago CC:
Aaron Boodman, abarth-chromium, ben+mojo_chromium.org, cbentzel+watch_chromium.org, chromium-reviews, darin (slow to review), qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionARC: Implement the startScan API to start scanning of WiFI APs.
This change makes the host start scanning for WiFi APs when a startScan request
is sent from the instance.
BUG=584886
BUG=b/26495967
Committed: https://crrev.com/e62feb84e8b87eb3020a2c9b55324387b240412c
Cr-Commit-Position: refs/heads/master@{#376112}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Add MinVersion to mojom #
Total comments: 2
Patch Set 3 : StartScan does not have a callback. #Patch Set 4 : Removed callback in StartScan prototype. #
Total comments: 2
Messages
Total messages: 27 (9 generated)
abhishekbh@google.com changed reviewers: + cernekee@google.com, dcheng@chromium.org, elijahtaylor@google.com, lhchavez@google.com
CL for startScan WiFi API.
abhishekbh@google.com changed reviewers: + hidehiko@chromium.org
lhchavez@chromium.org changed reviewers: + elijahtaylor@chromium.org, lhchavez@chromium.org - elijahtaylor@google.com, lhchavez@google.com
Adding the correct lhchavez and elijahtaylor.
https://codereview.chromium.org/1680483002/diff/1/components/arc/common/net.m... File components/arc/common/net.mojom (right): https://codereview.chromium.org/1680483002/diff/1/components/arc/common/net.m... components/arc/common/net.mojom:38: StartScan@2() => (bool result); [MinVersion=1] https://codereview.chromium.org/1680483002/diff/1/components/arc/net/arc_net_... File components/arc/net/arc_net_host_impl.cc (right): https://codereview.chromium.org/1680483002/diff/1/components/arc/net/arc_net_... components/arc/net/arc_net_host_impl.cc:115: callback.Run(true); Why add this if it is always going to be true? If you want to have a callback when it is actually run, you can declare it like so: StartScan() => ();
Please run trybots.
On 2016/02/09 06:47:14, hidehiko wrote: > Please run trybots. Done.
https://codereview.chromium.org/1680483002/diff/20001/components/arc/common/n... File components/arc/common/net.mojom (right): https://codereview.chromium.org/1680483002/diff/20001/components/arc/common/n... components/arc/common/net.mojom:38: [MinVersion=1] StartScan@2() => (); Per our discussion yesterday, we'll implement this as: ARC->Chrome: StartScan(); // no callback Chrome->ARC: ScanCompleted(); // can arrive at any time, even unsolicited
Addressed concerns. Changed StartScan() => () to StartScan() https://codereview.chromium.org/1680483002/diff/1/components/arc/common/net.m... File components/arc/common/net.mojom (right): https://codereview.chromium.org/1680483002/diff/1/components/arc/common/net.m... components/arc/common/net.mojom:38: StartScan@2() => (bool result); On 2016/02/08 16:35:46, Luis Héctor Chávez wrote: > [MinVersion=1] Done. https://codereview.chromium.org/1680483002/diff/1/components/arc/net/arc_net_... File components/arc/net/arc_net_host_impl.cc (right): https://codereview.chromium.org/1680483002/diff/1/components/arc/net/arc_net_... components/arc/net/arc_net_host_impl.cc:115: callback.Run(true); On 2016/02/08 16:35:46, Luis Héctor Chávez wrote: > Why add this if it is always going to be true? > > If you want to have a callback when it is actually run, you can declare it like > so: > > StartScan() => (); Removed. https://codereview.chromium.org/1680483002/diff/20001/components/arc/common/n... File components/arc/common/net.mojom (right): https://codereview.chromium.org/1680483002/diff/20001/components/arc/common/n... components/arc/common/net.mojom:38: [MinVersion=1] StartScan@2() => (); On 2016/02/10 21:37:58, cernekee wrote: > Per our discussion yesterday, we'll implement this as: > > ARC->Chrome: > > StartScan(); // no callback > > Chrome->ARC: > > ScanCompleted(); // can arrive at any time, even unsolicited Done.
lgtm
abhishekbh@google.com changed reviewers: - cernekee@google.com, elijahtaylor@chromium.org, hidehiko@chromium.org, lhchavez@chromium.org
Can you approve this when you get time ?
abhishekbh@google.com changed reviewers: + cernekee@google.com, lhchavez@chromium.org
hidehiko@chromium.org changed reviewers: + hidehiko@chromium.org
LGTM. dcheng@, PTAL for net.mojom as an OWNER?
dcheng@chromium.org changed reviewers: + elijahtaylor@chromium.org
https://codereview.chromium.org/1680483002/diff/60001/components/arc/net/arc_... File components/arc/net/arc_net_host_impl.cc (right): https://codereview.chromium.org/1680483002/diff/60001/components/arc/net/arc_... components/arc/net/arc_net_host_impl.cc:114: GetStateHandler()->RequestScan(); How does ARC++ know when this is done?
PTAL (dcheng) https://codereview.chromium.org/1680483002/diff/60001/components/arc/net/arc_... File components/arc/net/arc_net_host_impl.cc (right): https://codereview.chromium.org/1680483002/diff/60001/components/arc/net/arc_... components/arc/net/arc_net_host_impl.cc:114: GetStateHandler()->RequestScan(); On 2016/02/13 02:32:01, dcheng wrote: > How does ARC++ know when this is done? This API mimics the Android startScan API which is async. Another CL will be sent out when the completion of this event and its results will be sent from Host to ARC.
lgtm
The CQ bit was checked by abhishekbh@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1680483002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1680483002/60001
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== ARC: Implement the startScan API to start scanning of WiFI APs. This change makes the host start scanning for WiFi APs when a startScan request is sent from the instance. BUG=584886 BUG=b/26495967 ========== to ========== ARC: Implement the startScan API to start scanning of WiFI APs. This change makes the host start scanning for WiFi APs when a startScan request is sent from the instance. BUG=584886 BUG=b/26495967 Committed: https://crrev.com/e62feb84e8b87eb3020a2c9b55324387b240412c Cr-Commit-Position: refs/heads/master@{#376112} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/e62feb84e8b87eb3020a2c9b55324387b240412c Cr-Commit-Position: refs/heads/master@{#376112} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
