Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
809803005:
wifi_sync: add ability to convert WifiCredential to onc properties (Closed)
|
Created:
5 years, 11 months ago by mukesh agrawal Modified:
5 years, 11 months ago CC:
chromium-reviews, tim+watch_chromium.org, pvalenzuela+watch_chromium.org, maxbogue+watch_chromium.org, zea+watch_chromium.org, maniscalco+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@submit-4.0-wifi-security-class Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptionwifi_sync: add ability to convert WifiCredential to onc properties
There will be a couple of places where we need to convert from
a WifiCredential, to a DictionaryValue of ONC properties. So let's
put the conversion code someplace where it can be re-used.
The two places we'll need to convert a WifiCredential to ONC
properties will be WifiConfigDelegate and sync integration tests.
Both will need ONC properties to create a network in the
platform-specific network configuration for ChromeOS.
BUG=chromium:431435
TEST=components_unittests --gtest_filter="Wifi*"
Committed: https://crrev.com/80674e39108f7c132716a5c8c9a9056ec4716d1e
Cr-Commit-Position: refs/heads/master@{#310930}
Patch Set 1 #
Total comments: 29
Patch Set 2 : add validation, return onc_properties via scoped_ptr, fix nits #
Total comments: 3
Patch Set 3 : fix typo #
Total comments: 14
Patch Set 4 : fix nits #Patch Set 5 : rebase + resolve conflict #
Messages
Total messages: 19 (5 generated)
quiche@chromium.org changed reviewers: + erikwright@chromium.org, pneubeck@chromium.org, pstew@chromium.org, stevenjb@chromium.org
quiche@chromium.org changed required reviewers: + erikwright@chromium.org, stevenjb@chromium.org
This CL replaces https://codereview.chromium.org/831693005/
https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... File components/wifi_sync/wifi_credential.h (right): https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.h:63: static SsidBytes MakeSsidBytes(const std::string& ssid); If this is only used for tests, append ForTest to the end of the function name. https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... File components/wifi_sync/wifi_credential_unittest.cc (right): https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential_unittest.cc:18: } nit: helper functions should be in a local namespace https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential_unittest.cc:49: const auto kCredential(MakeCredential("fake-ssid", SECURITY_CLASS_NONE, "")); nit: use a const string for repeated strings here and below
https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... File components/wifi_sync/wifi_credential.cc (right): https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.cc:8: #include <string> not required (included in header) https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.cc:16: #include "components/wifi_sync/wifi_security_class.h" not required (included in header) https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.cc:41: if (!WifiSecurityClassToOncSecurityString(security_class(), &onc_security)) { Assuming that security_class() is not ..._INVALID this method should definitely succeed. I assume there is no reason that WifiCredential should ever be constructed with ..._INVALID. My preferred approach if there is no legitimate reason to have a WifiCredential that is invalid would be: (1) make the constructor private (not the copy constructor/assignment operators) (2) disallow default constructor (3) define public method "static scoped_ptr<WifiCredential> Create(ssid, security_class, passphrase);" which returns a WifiCredential instance iff the parameters are valid. https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... File components/wifi_sync/wifi_credential.h (right): https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.h:24: class WifiCredential final { // final because the class is copyable Is it your intention to permit use of the default constructor? https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.h:32: // and |passphrase|. No assumptions are made about the input You now make an assumption that it is UTF-8. https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.h:47: bool ToOncProperties(base::DictionaryValue* onc_properties) const; Does it make sense to add properties to an existing non-empty dictionary? If not, consider: scoped_ptr<DictionaryValue> ToOncProperties() const; https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.h:47: bool ToOncProperties(base::DictionaryValue* onc_properties) const; It seems a bit strange to me that you can instantiate a valid WifiCredential object that cannot be converted to OncProperties. Certainly, the fact that this method performs additional validation is not evident from the description. https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.h:63: static SsidBytes MakeSsidBytes(const std::string& ssid); On 2015/01/08 17:27:51, stevenjb wrote: > If this is only used for tests, append ForTest to the end of the function name. How will SsidBytes normally be constructed? What is the benefit of the custom type vs. a plain-old std::string? https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... File components/wifi_sync/wifi_credential_unittest.cc (right): https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential_unittest.cc:18: } On 2015/01/08 17:27:51, stevenjb wrote: > nit: helper functions should be in a local namespace by which he means anonymous. https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential_unittest.cc:49: const auto kCredential(MakeCredential("fake-ssid", SECURITY_CLASS_NONE, "")); not a valid use of auto. "Use auto to avoid type names that are just clutter. Continue to use manifest type declarations when it helps readability"
https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... File components/wifi_sync/wifi_credential.cc (right): https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.cc:8: #include <string> On 2015/01/08 19:27:07, erikwright wrote: > not required (included in header) Done. Also removed limits, since it's also unnecessary. (I probably should have removed it when I removed the numeric_limits check in ToString.) https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.cc:16: #include "components/wifi_sync/wifi_security_class.h" On 2015/01/08 19:27:07, erikwright wrote: > not required (included in header) Done. https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.cc:41: if (!WifiSecurityClassToOncSecurityString(security_class(), &onc_security)) { On 2015/01/08 19:27:07, erikwright wrote: > Assuming that security_class() is not ..._INVALID this method should definitely > succeed. I assume there is no reason that WifiCredential should ever be > constructed with ..._INVALID. > > My preferred approach if there is no legitimate reason to have a WifiCredential > that is invalid would be: > > (1) make the constructor private (not the copy constructor/assignment operators) > (2) disallow default constructor > (3) define public method "static scoped_ptr<WifiCredential> Create(ssid, > security_class, passphrase);" which returns a WifiCredential instance iff the > parameters are valid. Done. I've left in the check that the SecurityClass conversion succeeded, but changed it from LOG(ERROR) to NOTREACHED(). Let me know if it would make more sense to remove the check. https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... File components/wifi_sync/wifi_credential.h (right): https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.h:24: class WifiCredential final { // final because the class is copyable On 2015/01/08 19:27:08, erikwright wrote: > Is it your intention to permit use of the default constructor? No, I don't expect there will be a need for default construction. https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.h:32: // and |passphrase|. No assumptions are made about the input On 2015/01/08 19:27:07, erikwright wrote: > You now make an assumption that it is UTF-8. See below (comment on ToOncProperties). https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.h:47: bool ToOncProperties(base::DictionaryValue* onc_properties) const; On 2015/01/08 19:27:08, erikwright wrote: > Does it make sense to add properties to an existing non-empty dictionary? If > not, consider: > > scoped_ptr<DictionaryValue> ToOncProperties() const; Done. Thanks for the idea. I wanted to do something like this, but switched to an out parameter when the compiler pointed out that DictionaryValue is not copyable. scoped_ptr is better, though. :) https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.h:47: bool ToOncProperties(base::DictionaryValue* onc_properties) const; On 2015/01/08 19:27:08, erikwright wrote: > It seems a bit strange to me that you can instantiate a valid WifiCredential > object that cannot be converted to OncProperties. Certainly, the fact that this > method performs additional validation is not evident from the description. It's definitely strange. We have a bug open for this (crbug.com/432546), which pneubeck@ is working on. Until that's fixed, I've updated the method's documentation, to note this oddity. https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.h:63: static SsidBytes MakeSsidBytes(const std::string& ssid); On 2015/01/08 17:27:51, stevenjb wrote: > If this is only used for tests, append ForTest to the end of the function name. Done. https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.h:63: static SsidBytes MakeSsidBytes(const std::string& ssid); On 2015/01/08 19:27:07, erikwright wrote: > On 2015/01/08 17:27:51, stevenjb wrote: > > If this is only used for tests, append ForTest to the end of the function > name. > > How will SsidBytes normally be constructed? What is the benefit of the custom > type vs. a plain-old std::string? In the normal case, SsidBytes will be (copy-)constructed from a std::vector<uint8_t>. SsidBytes is, itself, just an alias for std::vector<uint8_t>. The reason for introducing the custom type is to discourage people from assuming that SSID is "text", or ASCII or UTF-8. I think people would be prone to such assumptions if the type were std::string. FWIW, so far, I've drafted three places where this happens: 1) when WifiCredentialSyncableService deserializes a WifiCredentialSpecifics protobuf 2) when network_state_helper_chromeos reads raw_ssid() from a NetworkState 3) when WifiCredentialManager reads raw_ssid() from a NetworkState (WifiCredentialManager is a new class that I haven't posted for review yet) https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... File components/wifi_sync/wifi_credential_unittest.cc (right): https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential_unittest.cc:18: } On 2015/01/08 17:27:51, stevenjb wrote: > nit: helper functions should be in a local namespace Done. https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential_unittest.cc:18: } On 2015/01/08 19:27:08, erikwright wrote: > On 2015/01/08 17:27:51, stevenjb wrote: > > nit: helper functions should be in a local namespace > > by which he means anonymous. Done. https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential_unittest.cc:49: const auto kCredential(MakeCredential("fake-ssid", SECURITY_CLASS_NONE, "")); On 2015/01/08 17:27:51, stevenjb wrote: > nit: use a const string for repeated strings here and below Done. https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential_unittest.cc:49: const auto kCredential(MakeCredential("fake-ssid", SECURITY_CLASS_NONE, "")); On 2015/01/08 19:27:08, erikwright wrote: > not a valid use of auto. > > "Use auto to avoid type names that are just clutter. Continue to use manifest > type declarations when it helps readability" Done. https://codereview.chromium.org/809803005/diff/20001/components/wifi_sync/wif... File components/wifi_sync/wifi_credential.cc (right): https://codereview.chromium.org/809803005/diff/20001/components/wifi_sync/wif... components/wifi_sync/wifi_credential.cc:35: auto onc_properties = make_scoped_ptr(new base::DictionaryValue()); I think auto is appropriate here, since the rest of the line is almost exactly stating the type (scoped_ptr<base::DictionaryValue>). Let me know if that's mistaken.
Thanks, all. PTAL.
LGTM with nits. https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... File components/wifi_sync/wifi_credential.cc (right): https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.cc:41: if (!WifiSecurityClassToOncSecurityString(security_class(), &onc_security)) { On 2015/01/09 02:00:18, mukesh agrawal wrote: > On 2015/01/08 19:27:07, erikwright wrote: > > Assuming that security_class() is not ..._INVALID this method should > definitely > > succeed. I assume there is no reason that WifiCredential should ever be > > constructed with ..._INVALID. > > > > My preferred approach if there is no legitimate reason to have a > WifiCredential > > that is invalid would be: > > > > (1) make the constructor private (not the copy constructor/assignment > operators) > > (2) disallow default constructor > > (3) define public method "static scoped_ptr<WifiCredential> Create(ssid, > > security_class, passphrase);" which returns a WifiCredential instance iff the > > parameters are valid. > > Done. > > I've left in the check that the SecurityClass conversion succeeded, but changed > it from LOG(ERROR) to NOTREACHED(). Let me know if it would make more sense to > remove the check. The NOTREACHED is good, but I would remove the NULL return. Once you deal with the ONC/UTF-8 thing it would seem appropriate for this method to always return a valid object. https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... File components/wifi_sync/wifi_credential.h (right): https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.h:63: static SsidBytes MakeSsidBytes(const std::string& ssid); On 2015/01/09 02:00:18, mukesh agrawal wrote: > On 2015/01/08 19:27:07, erikwright wrote: > > On 2015/01/08 17:27:51, stevenjb wrote: > > > If this is only used for tests, append ForTest to the end of the function > > name. > > > > How will SsidBytes normally be constructed? What is the benefit of the custom > > type vs. a plain-old std::string? > > In the normal case, SsidBytes will be (copy-)constructed from a > std::vector<uint8_t>. SsidBytes is, itself, just an alias for > std::vector<uint8_t>. > > The reason for introducing the custom type is to discourage people from assuming > that SSID is "text", or ASCII or UTF-8. I think people would be prone to such > assumptions if the type were std::string. > > FWIW, so far, I've drafted three places where this happens: > 1) when WifiCredentialSyncableService deserializes a WifiCredentialSpecifics > protobuf > 2) when network_state_helper_chromeos reads raw_ssid() from a NetworkState > 3) when WifiCredentialManager reads raw_ssid() from a NetworkState > (WifiCredentialManager is a new class that I haven't posted for review yet) Can you, in that case, please add a link or comment explaining what a valid SSID is? Maybe right above the definition of SsidBytes? If there is only a single test that uses this, and it's the test for this class, I would suggest just inlining this method there. Maybe there will be others? https://codereview.chromium.org/809803005/diff/20001/components/wifi_sync/wif... File components/wifi_sync/wifi_credential.cc (right): https://codereview.chromium.org/809803005/diff/20001/components/wifi_sync/wif... components/wifi_sync/wifi_credential.cc:35: auto onc_properties = make_scoped_ptr(new base::DictionaryValue()); On 2015/01/09 02:00:18, mukesh agrawal wrote: > I think auto is appropriate here, since the rest of the line is almost exactly > stating the type (scoped_ptr<base::DictionaryValue>). Let me know if that's > mistaken. This would be more typically done without the make_scoped_ptr: scoped_ptr<base::DictionaryValue()> onc_properties(new base::DictionaryValue()); I don't think that's longer by more than a few characters and reads a lot better to habituated Chromium developers. https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/net... File components/wifi_sync/network_state_helper_chromeos.cc (right): https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/net... components/wifi_sync/network_state_helper_chromeos.cc:44: if (!credential) { The braces are not required here, and should be ommitted for consistency since you have already opted to do so above. https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... File components/wifi_sync/wifi_credential.cc (right): https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... components/wifi_sync/wifi_credential.cc:66: return nullptr; I didn't know this could be coerced to a scoped_ptr. Nice! https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... File components/wifi_sync/wifi_credential.h (right): https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... components/wifi_sync/wifi_credential.h:55: // parameters are invalid, returns a scoped_ptr with value nullptr. You can simplify the last two sentences to: Returns NULL if the parameters are invalid. https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... components/wifi_sync/wifi_credential.h:56: static scoped_ptr<WifiCredential> Create( Make this the first method declared (right after the destructor). https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... File components/wifi_sync/wifi_credential_unittest.cc (right): https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... components/wifi_sync/wifi_credential_unittest.cc:84: const WifiCredential kCredential( The 'k' is reserved for static constants, not merely immutable locals. https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... components/wifi_sync/wifi_credential_unittest.cc:95: const WifiCredential kCredential( ditto https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... components/wifi_sync/wifi_credential_unittest.cc:137: EXPECT_EQ(nullptr, onc_properties); I think EXPECT_FALSE will also work here.
lgtm
Thanks, all. Holding off on the commit bit until the previous CL in this series lands. https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... File components/wifi_sync/wifi_credential.h (right): https://codereview.chromium.org/809803005/diff/1/components/wifi_sync/wifi_cr... components/wifi_sync/wifi_credential.h:63: static SsidBytes MakeSsidBytes(const std::string& ssid); On 2015/01/09 14:58:20, erikwright wrote: > Can you, in that case, please add a link or comment explaining what a valid SSID > is? Maybe right above the definition of SsidBytes? Done. (Added comment about valid SSIDs.) > If there is only a single test that uses this, and it's the test for this class, > I would suggest just inlining this method there. Maybe there will be others? There will be others. E.g. in sync integration tests. https://codereview.chromium.org/809803005/diff/20001/components/wifi_sync/wif... File components/wifi_sync/wifi_credential.cc (right): https://codereview.chromium.org/809803005/diff/20001/components/wifi_sync/wif... components/wifi_sync/wifi_credential.cc:35: auto onc_properties = make_scoped_ptr(new base::DictionaryValue()); On 2015/01/09 14:58:20, erikwright wrote: > On 2015/01/09 02:00:18, mukesh agrawal wrote: > > I think auto is appropriate here, since the rest of the line is almost exactly > > stating the type (scoped_ptr<base::DictionaryValue>). Let me know if that's > > mistaken. > > This would be more typically done without the make_scoped_ptr: > > scoped_ptr<base::DictionaryValue()> onc_properties(new base::DictionaryValue()); > > I don't think that's longer by more than a few characters and reads a lot better > to habituated Chromium developers. Done. https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/net... File components/wifi_sync/network_state_helper_chromeos.cc (right): https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/net... components/wifi_sync/network_state_helper_chromeos.cc:44: if (!credential) { On 2015/01/09 14:58:21, erikwright wrote: > The braces are not required here, and should be ommitted for consistency since > you have already opted to do so above. Done. https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... File components/wifi_sync/wifi_credential.cc (right): https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... components/wifi_sync/wifi_credential.cc:66: return nullptr; On 2015/01/09 14:58:21, erikwright wrote: > I didn't know this could be coerced to a scoped_ptr. Nice! Acknowledged. https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... File components/wifi_sync/wifi_credential.h (right): https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... components/wifi_sync/wifi_credential.h:55: // parameters are invalid, returns a scoped_ptr with value nullptr. On 2015/01/09 14:58:21, erikwright wrote: > You can simplify the last two sentences to: > > Returns NULL if the parameters are invalid. Done. https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... components/wifi_sync/wifi_credential.h:56: static scoped_ptr<WifiCredential> Create( On 2015/01/09 14:58:21, erikwright wrote: > Make this the first method declared (right after the destructor). Done. https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... File components/wifi_sync/wifi_credential_unittest.cc (right): https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... components/wifi_sync/wifi_credential_unittest.cc:84: const WifiCredential kCredential( On 2015/01/09 14:58:21, erikwright wrote: > The 'k' is reserved for static constants, not merely immutable locals. Done. https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... components/wifi_sync/wifi_credential_unittest.cc:95: const WifiCredential kCredential( On 2015/01/09 14:58:21, erikwright wrote: > ditto Done. https://codereview.chromium.org/809803005/diff/40001/components/wifi_sync/wif... components/wifi_sync/wifi_credential_unittest.cc:137: EXPECT_EQ(nullptr, onc_properties); On 2015/01/09 14:58:21, erikwright wrote: > I think EXPECT_FALSE will also work here. Done.
The CQ bit was checked by quiche@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/809803005/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...) android_chromium_gn_compile_dbg on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...) android_chromium_gn_compile_rel on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...) android_dbg_tests_recipe on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...) chromium_presubmit on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...) linux_chromium_asan_rel on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_gn_dbg on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_gn_rel on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_ng on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) ios_dbg_simulator on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...) ios_rel_device on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...) ios_rel_device_ninja on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...) mac_chromium_compile_dbg_ng on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...) win8_chromium_rel on tryserver.chromium.win (http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
small conflict in network_state_helper_chromeos.cc
The CQ bit was checked by quiche@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/809803005/80001
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/80674e39108f7c132716a5c8c9a9056ec4716d1e Cr-Commit-Position: refs/heads/master@{#310930} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
