Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1547533002:
remoting: use VerifyHostPinHash() in place on IsPinValid() (Closed)
Descriptionremoting: use VerifyHostPinHash() in place on IsPinValid()
For over a year, we have been shipping a 64-bit version of Chrome for Mac.
So with this statement, this patch makes the switch to
VerifyHostPinHash() function from remoting/host/pin_hash.h, fixing the
lambroslambrou's TODO.
BUG=None
R=sergeyu@chromium.org
Committed: https://crrev.com/7acd392e4018d121977e2738b6e3c59f5cf418f1
Cr-Commit-Position: refs/heads/master@{#366762}
Committed: https://crrev.com/d433493df4b98ea006e6407590cff7a7912e16df
Cr-Commit-Position: refs/heads/master@{#366783}
Committed: https://crrev.com/38b2f73d2594ec921ea1b73bebfd2da113b6799e
Cr-Commit-Position: refs/heads/master@{#366936}
Patch Set 1 #Patch Set 2 : restore libmodp_b64_nacl.a #
Total comments: 2
Patch Set 3 : restore modp_b64_nacl as well #
Total comments: 12
Patch Set 4 : review #Patch Set 5 : revert the revert #Patch Set 6 : fix for the revert: remoting:: #Patch Set 7 : revert the revert again #Patch Set 8 : revert some of my changes #
Total comments: 3
Patch Set 9 : back with dependencies - remoe ARCHS #
Total comments: 3
Messages
Total messages: 41 (15 generated)
Sergey, is this correct?
https://codereview.chromium.org/1547533002/diff/20001/remoting/remoting_nacl.gyp File remoting/remoting_nacl.gyp (left): https://codereview.chromium.org/1547533002/diff/20001/remoting/remoting_nacl.... remoting/remoting_nacl.gyp:191: '../third_party/modp_b64/modp_b64_nacl.gyp:modp_b64_nacl', I think we still need this. It's not related to the Mac code you are changing. The PNaCl module compiled here still has indirect dependency on modp_b64
https://codereview.chromium.org/1547533002/diff/20001/remoting/remoting_nacl.gyp File remoting/remoting_nacl.gyp (left): https://codereview.chromium.org/1547533002/diff/20001/remoting/remoting_nacl.... remoting/remoting_nacl.gyp:191: '../third_party/modp_b64/modp_b64_nacl.gyp:modp_b64_nacl', On 2015/12/22 17:44:19, Sergey Ulanov wrote: > I think we still need this. It's not related to the Mac code you are changing. > The PNaCl module compiled here still has indirect dependency on modp_b64 Thanks! Restored.
https://codereview.chromium.org/1547533002/diff/40001/remoting/remoting_host_... File remoting/remoting_host_mac.gypi (right): https://codereview.chromium.org/1547533002/diff/40001/remoting/remoting_host_... remoting/remoting_host_mac.gypi:180: 'dependencies': [ I think you need to add other dependencies here. At very least remoting_base. It will fail to compile without this. https://codereview.chromium.org/1547533002/diff/40001/remoting/remoting_host_... remoting/remoting_host_mac.gypi:194: # These source files are included directly, instead of adding target remove this comment https://codereview.chromium.org/1547533002/diff/40001/remoting/remoting_host_... remoting/remoting_host_mac.gypi:198: # TODO(lambroslambrou): Fix this when Chrome supports building for remove TODO https://codereview.chromium.org/1547533002/diff/40001/remoting/remoting_host_... remoting/remoting_host_mac.gypi:201: '../third_party/jsoncpp/overrides/src/lib_json/json_value.cpp', add dependency on third_party/jsoncpp instead of compiling the files here. https://codereview.chromium.org/1547533002/diff/40001/remoting/remoting_host_... remoting/remoting_host_mac.gypi:206: 'host/host_config_constants.cc', add dependency on remoting_host instead of compiling these files again. https://codereview.chromium.org/1547533002/diff/40001/remoting/remoting_nacl.gyp File remoting/remoting_nacl.gyp (left): https://codereview.chromium.org/1547533002/diff/40001/remoting/remoting_nacl.... remoting/remoting_nacl.gyp:220: '-lmodp_b64_nacl', Still need this
https://codereview.chromium.org/1547533002/diff/40001/remoting/remoting_host_... File remoting/remoting_host_mac.gypi (right): https://codereview.chromium.org/1547533002/diff/40001/remoting/remoting_host_... remoting/remoting_host_mac.gypi:180: 'dependencies': [ On 2015/12/22 18:28:53, Sergey Ulanov wrote: > I think you need to add other dependencies here. At very least remoting_base. It > will fail to compile without this. Done. https://codereview.chromium.org/1547533002/diff/40001/remoting/remoting_host_... remoting/remoting_host_mac.gypi:194: # These source files are included directly, instead of adding target On 2015/12/22 18:28:53, Sergey Ulanov wrote: > remove this comment Done. https://codereview.chromium.org/1547533002/diff/40001/remoting/remoting_host_... remoting/remoting_host_mac.gypi:198: # TODO(lambroslambrou): Fix this when Chrome supports building for On 2015/12/22 18:28:53, Sergey Ulanov wrote: > remove TODO Done. https://codereview.chromium.org/1547533002/diff/40001/remoting/remoting_host_... remoting/remoting_host_mac.gypi:201: '../third_party/jsoncpp/overrides/src/lib_json/json_value.cpp', On 2015/12/22 18:28:53, Sergey Ulanov wrote: > add dependency on third_party/jsoncpp instead of compiling the files here. Done. https://codereview.chromium.org/1547533002/diff/40001/remoting/remoting_host_... remoting/remoting_host_mac.gypi:206: 'host/host_config_constants.cc', On 2015/12/22 18:28:53, Sergey Ulanov wrote: > add dependency on remoting_host instead of compiling these files again. Done. https://codereview.chromium.org/1547533002/diff/40001/remoting/remoting_nacl.gyp File remoting/remoting_nacl.gyp (left): https://codereview.chromium.org/1547533002/diff/40001/remoting/remoting_nacl.... remoting/remoting_nacl.gyp:220: '-lmodp_b64_nacl', On 2015/12/22 18:28:53, Sergey Ulanov wrote: > Still need this Done.
lgtm. Thanks!
The CQ bit was checked by tfarina@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1547533002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1547533002/60001
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== remoting: use VerifyHostPinHash() in place on IsPinValid() For over a year, we have been shipping a 64-bit version of Chrome for Mac. So with this statement, this patch makes the switch to VerifyHostPinHash() function from remoting/host/pin_hash.h, fixing the lambroslambrou's TODO. BUG=None R=sergeyu@chromium.org ========== to ========== remoting: use VerifyHostPinHash() in place on IsPinValid() For over a year, we have been shipping a 64-bit version of Chrome for Mac. So with this statement, this patch makes the switch to VerifyHostPinHash() function from remoting/host/pin_hash.h, fixing the lambroslambrou's TODO. BUG=None R=sergeyu@chromium.org Committed: https://crrev.com/7acd392e4018d121977e2738b6e3c59f5cf418f1 Cr-Commit-Position: refs/heads/master@{#366762} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/7acd392e4018d121977e2738b6e3c59f5cf418f1 Cr-Commit-Position: refs/heads/master@{#366762}
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:60001) has been created in https://codereview.chromium.org/1543303002/ by lukasza@chromium.org. The reason for reverting is: This seems to break the build on Mac: https://build.chromium.org/p/chromium/builders/Mac/builds/10276/steps/compile...: FAILED: /b/build/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/remoting/host/mac/remoting_host_prefpane.me2me_preference_pane.x86_64.o.d -DBINARY_CORE=1 -DBINARY_DESKTOP=2 -DBINARY_HOST_ME2ME=3 -DBINARY_NATIVE_MESSAGING_HOST=4 -DBINARY_REMOTE_ASSISTANCE_HOST=5 -DV8_DEPRECATION_WARNINGS -DCLD_VERSION=2 -D__ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORE=0 -DCHROMIUM_BUILD -DCR_CLANG_REVISION=255169-1 -DUSE_LIBJPEG_TURBO=1 -DENABLE_ONE_CLICK_SIGNIN -DENABLE_WEBRTC=1 -DENABLE_MEDIA_ROUTER=1 -DENABLE_PEPPER_CDMS -DENABLE_CONFIGURATION_POLICY -DENABLE_NOTIFICATIONS -DENABLE_HIDPI=1 -DDONT_EMBED_BUILD_METADATA -DFIELDTRIAL_TESTING_ENABLED -DENABLE_TASK_MANAGER=1 -DENABLE_EXTENSIONS=1 -DENABLE_PDF=1 -DENABLE_PLUGIN_INSTALLATION=1 -DENABLE_PLUGINS=1 -DENABLE_SESSION_SERVICE=1 -DENABLE_THEMES=1 -DENABLE_AUTOFILL_DIALOG=1 -DENABLE_BACKGROUND=1 -DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1 -DENABLE_PRINT_PREVIEW=1 -DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DENABLE_APP_LIST=1 -DENABLE_SETTINGS_APP=1 -DENABLE_SUPERVISED_USERS=1 -DENABLE_SERVICE_DISCOVERY=1 -DV8_USE_EXTERNAL_STARTUP_DATA -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL '-DHOST_BUNDLE_NAME="ChromotingHost.bundle"' '-DPREFPANE_BUNDLE_NAME="Chromoting.prefPane"' -DJSON_USE_EXCEPTION=0 -DPROTOBUF_USE_DLLS -DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DUSE_LIBPCI=1 -DUSE_OPENSSL=1 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -D_FORTIFY_SOURCE=2 -I../.. -Igen -I../../third_party/jsoncpp/overrides/include -I../../third_party/jsoncpp/source/include -I../../third_party/jsoncpp/source/src/lib_json -I../../third_party/khronos -I../../gpu -I../../skia/config -I../../third_party/protobuf -I../../third_party/protobuf/src -Igen/protoc_out -isysroot /Applications/Xcode5.1.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk -O2 -gdwarf-2 -fvisibility=hidden -Werror -Wnewline-eof -mmacosx-version-min=10.6 -arch x86_64 -Wall -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wno-selector-type-mismatch -Wpartial-availability -Wheader-hygiene -Wno-char-subscripts -Wno-unneeded-internal-declaration -Wno-covered-switch-default -Wstring-conversion -Wno-c++11-narrowing -Wno-deprecated-register -Wno-inconsistent-missing-override -Wno-shift-negative-value -Wno-bitfield-width -std=c++11 -stdlib=libc++ -fno-rtti -fno-exceptions -fvisibility-inlines-hidden -fno-threadsafe-statics -Xclang -load -Xclang /b/build/slave/Mac/build/src/third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.dylib -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang check-templates -fcolor-diagnostics -fno-strict-aliasing -fobjc-gc -Wobjc-missing-property-synthesis -fobjc-call-cxx-cdtors -c ../../remoting/host/mac/me2me_preference_pane.mm -o obj/remoting/host/mac/remoting_host_prefpane.me2me_preference_pane.x86_64.o ../../remoting/host/mac/me2me_preference_pane.mm:299:8: error: use of undeclared identifier 'VerifyHostPinHash'; did you mean 'remoting::VerifyHostPinHash'? if (!VerifyHostPinHash(pin_utf8, host_id, host_secret_hash)) { ^~~~~~~~~~~~~~~~~ remoting::VerifyHostPinHash .
Message was sent while issue was closed.
Thanks! Relanding!
Message was sent while issue was closed.
Description was changed from ========== remoting: use VerifyHostPinHash() in place on IsPinValid() For over a year, we have been shipping a 64-bit version of Chrome for Mac. So with this statement, this patch makes the switch to VerifyHostPinHash() function from remoting/host/pin_hash.h, fixing the lambroslambrou's TODO. BUG=None R=sergeyu@chromium.org Committed: https://crrev.com/7acd392e4018d121977e2738b6e3c59f5cf418f1 Cr-Commit-Position: refs/heads/master@{#366762} ========== to ========== remoting: use VerifyHostPinHash() in place on IsPinValid() For over a year, we have been shipping a 64-bit version of Chrome for Mac. So with this statement, this patch makes the switch to VerifyHostPinHash() function from remoting/host/pin_hash.h, fixing the lambroslambrou's TODO. BUG=None R=sergeyu@chromium.org Committed: https://crrev.com/7acd392e4018d121977e2738b6e3c59f5cf418f1 Cr-Commit-Position: refs/heads/master@{#366762} ==========
The CQ bit was checked by tfarina@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org Link to the patchset: https://codereview.chromium.org/1547533002/#ps100001 (title: "fix for the revert: remoting::")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1547533002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1547533002/100001
Message was sent while issue was closed.
Description was changed from ========== remoting: use VerifyHostPinHash() in place on IsPinValid() For over a year, we have been shipping a 64-bit version of Chrome for Mac. So with this statement, this patch makes the switch to VerifyHostPinHash() function from remoting/host/pin_hash.h, fixing the lambroslambrou's TODO. BUG=None R=sergeyu@chromium.org Committed: https://crrev.com/7acd392e4018d121977e2738b6e3c59f5cf418f1 Cr-Commit-Position: refs/heads/master@{#366762} ========== to ========== remoting: use VerifyHostPinHash() in place on IsPinValid() For over a year, we have been shipping a 64-bit version of Chrome for Mac. So with this statement, this patch makes the switch to VerifyHostPinHash() function from remoting/host/pin_hash.h, fixing the lambroslambrou's TODO. BUG=None R=sergeyu@chromium.org Committed: https://crrev.com/7acd392e4018d121977e2738b6e3c59f5cf418f1 Cr-Commit-Position: refs/heads/master@{#366762} ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== remoting: use VerifyHostPinHash() in place on IsPinValid() For over a year, we have been shipping a 64-bit version of Chrome for Mac. So with this statement, this patch makes the switch to VerifyHostPinHash() function from remoting/host/pin_hash.h, fixing the lambroslambrou's TODO. BUG=None R=sergeyu@chromium.org Committed: https://crrev.com/7acd392e4018d121977e2738b6e3c59f5cf418f1 Cr-Commit-Position: refs/heads/master@{#366762} ========== to ========== remoting: use VerifyHostPinHash() in place on IsPinValid() For over a year, we have been shipping a 64-bit version of Chrome for Mac. So with this statement, this patch makes the switch to VerifyHostPinHash() function from remoting/host/pin_hash.h, fixing the lambroslambrou's TODO. BUG=None R=sergeyu@chromium.org Committed: https://crrev.com/7acd392e4018d121977e2738b6e3c59f5cf418f1 Cr-Commit-Position: refs/heads/master@{#366762} Committed: https://crrev.com/d433493df4b98ea006e6407590cff7a7912e16df Cr-Commit-Position: refs/heads/master@{#366783} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/d433493df4b98ea006e6407590cff7a7912e16df Cr-Commit-Position: refs/heads/master@{#366783}
Message was sent while issue was closed.
A revert of this CL (patchset #6 id:100001) has been created in https://codereview.chromium.org/1541323004/ by lukasza@chromium.org. The reason for reverting is: Relanding still broke the build - https://build.chromium.org/p/chromium/builders/Mac/builds/10279/steps/compile...: Undefined symbols for architecture i386: "remoting::kHostConfigFileName", referenced from: -[Me2MePreferencePane readNewConfig] in remoting_host_prefpane.me2me_preference_pane.i386.o -[Me2MePreferencePane mismatchAlertDidEnd:returnCode:contextInfo:] in remoting_host_prefpane.me2me_preference_pane.i386.o.
Message was sent while issue was closed.
Description was changed from ========== remoting: use VerifyHostPinHash() in place on IsPinValid() For over a year, we have been shipping a 64-bit version of Chrome for Mac. So with this statement, this patch makes the switch to VerifyHostPinHash() function from remoting/host/pin_hash.h, fixing the lambroslambrou's TODO. BUG=None R=sergeyu@chromium.org Committed: https://crrev.com/7acd392e4018d121977e2738b6e3c59f5cf418f1 Cr-Commit-Position: refs/heads/master@{#366762} Committed: https://crrev.com/d433493df4b98ea006e6407590cff7a7912e16df Cr-Commit-Position: refs/heads/master@{#366783} ========== to ========== remoting: use VerifyHostPinHash() in place on IsPinValid() For over a year, we have been shipping a 64-bit version of Chrome for Mac. So with this statement, this patch makes the switch to VerifyHostPinHash() function from remoting/host/pin_hash.h, fixing the lambroslambrou's TODO. BUG=None R=sergeyu@chromium.org Committed: https://crrev.com/7acd392e4018d121977e2738b6e3c59f5cf418f1 Cr-Commit-Position: refs/heads/master@{#366762} Committed: https://crrev.com/d433493df4b98ea006e6407590cff7a7912e16df Cr-Commit-Position: refs/heads/master@{#366783} ==========
I'm going to let this for when Lambros return from vacation! He may know what I'm missing here! https://codereview.chromium.org/1547533002/diff/140001/remoting/remoting_host... File remoting/remoting_host_mac.gypi (right): https://codereview.chromium.org/1547533002/diff/140001/remoting/remoting_host... remoting/remoting_host_mac.gypi:193: 'sources': [ I don't understand what is going on with remoting_host_prefpane. Looks like it wants to build i386 (32-bit) even when the host is x86_64 (64-bit). Adding 'remoting_host' to dependencies list does not have any effect for: $ ninja -C out/Release remoting_host_prefpane ... Undefined symbols for architecture i386: "remoting::VerifyHostPinHash(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)", referenced from: -[Me2MePreferencePane applyConfiguration:pin:] in remoting_host_prefpane.me2me_preference_pane.i386.o
The problem is that remoting_host_prefpane is still compiled as fat binary, i.e. it's compiled for both 32-bit and 64-bit. The dependencies (i.e. remoting_base) are compiled for 64-bit only. We don't need it to be a fat binary anymore. It should compile fine if you compile it only for x86_64 https://codereview.chromium.org/1547533002/diff/140001/remoting/remoting_host... File remoting/remoting_host_mac.gypi (right): https://codereview.chromium.org/1547533002/diff/140001/remoting/remoting_host... remoting/remoting_host_mac.gypi:216: 'ARCHS': ['i386', 'x86_64'], Remove this line
Trying once more! https://codereview.chromium.org/1547533002/diff/140001/remoting/remoting_host... File remoting/remoting_host_mac.gypi (right): https://codereview.chromium.org/1547533002/diff/140001/remoting/remoting_host... remoting/remoting_host_mac.gypi:216: 'ARCHS': ['i386', 'x86_64'], On 2015/12/27 01:21:15, Sergey Ulanov wrote: > Remove this line Thanks Sergey! After removing this, remoting_host_prefpane built for me.
The CQ bit was checked by tfarina@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org Link to the patchset: https://codereview.chromium.org/1547533002/#ps160001 (title: "back with dependencies - remoe ARCHS")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1547533002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1547533002/160001
Message was sent while issue was closed.
Description was changed from ========== remoting: use VerifyHostPinHash() in place on IsPinValid() For over a year, we have been shipping a 64-bit version of Chrome for Mac. So with this statement, this patch makes the switch to VerifyHostPinHash() function from remoting/host/pin_hash.h, fixing the lambroslambrou's TODO. BUG=None R=sergeyu@chromium.org Committed: https://crrev.com/7acd392e4018d121977e2738b6e3c59f5cf418f1 Cr-Commit-Position: refs/heads/master@{#366762} Committed: https://crrev.com/d433493df4b98ea006e6407590cff7a7912e16df Cr-Commit-Position: refs/heads/master@{#366783} ========== to ========== remoting: use VerifyHostPinHash() in place on IsPinValid() For over a year, we have been shipping a 64-bit version of Chrome for Mac. So with this statement, this patch makes the switch to VerifyHostPinHash() function from remoting/host/pin_hash.h, fixing the lambroslambrou's TODO. BUG=None R=sergeyu@chromium.org Committed: https://crrev.com/7acd392e4018d121977e2738b6e3c59f5cf418f1 Cr-Commit-Position: refs/heads/master@{#366762} Committed: https://crrev.com/d433493df4b98ea006e6407590cff7a7912e16df Cr-Commit-Position: refs/heads/master@{#366783} ==========
Message was sent while issue was closed.
Committed patchset #9 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== remoting: use VerifyHostPinHash() in place on IsPinValid() For over a year, we have been shipping a 64-bit version of Chrome for Mac. So with this statement, this patch makes the switch to VerifyHostPinHash() function from remoting/host/pin_hash.h, fixing the lambroslambrou's TODO. BUG=None R=sergeyu@chromium.org Committed: https://crrev.com/7acd392e4018d121977e2738b6e3c59f5cf418f1 Cr-Commit-Position: refs/heads/master@{#366762} Committed: https://crrev.com/d433493df4b98ea006e6407590cff7a7912e16df Cr-Commit-Position: refs/heads/master@{#366783} ========== to ========== remoting: use VerifyHostPinHash() in place on IsPinValid() For over a year, we have been shipping a 64-bit version of Chrome for Mac. So with this statement, this patch makes the switch to VerifyHostPinHash() function from remoting/host/pin_hash.h, fixing the lambroslambrou's TODO. BUG=None R=sergeyu@chromium.org Committed: https://crrev.com/7acd392e4018d121977e2738b6e3c59f5cf418f1 Cr-Commit-Position: refs/heads/master@{#366762} Committed: https://crrev.com/d433493df4b98ea006e6407590cff7a7912e16df Cr-Commit-Position: refs/heads/master@{#366783} Committed: https://crrev.com/38b2f73d2594ec921ea1b73bebfd2da113b6799e Cr-Commit-Position: refs/heads/master@{#366936} ==========
Message was sent while issue was closed.
Patchset 9 (id:??) landed as https://crrev.com/38b2f73d2594ec921ea1b73bebfd2da113b6799e Cr-Commit-Position: refs/heads/master@{#366936}
Message was sent while issue was closed.
thakis@chromium.org changed reviewers: + thakis@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/1547533002/diff/160001/remoting/remoting_host... File remoting/remoting_host_mac.gypi (left): https://codereview.chromium.org/1547533002/diff/160001/remoting/remoting_host... remoting/remoting_host_mac.gypi:224: 'ARCHS': ['i386', 'x86_64'], Doesn't this break using the remoting prefpane on older versions of OS X where System Preferences is still a 32-bit app?
Message was sent while issue was closed.
https://codereview.chromium.org/1547533002/diff/160001/remoting/remoting_host... File remoting/remoting_host_mac.gypi (left): https://codereview.chromium.org/1547533002/diff/160001/remoting/remoting_host... remoting/remoting_host_mac.gypi:224: 'ARCHS': ['i386', 'x86_64'], On 2016/01/04 16:28:42, Nico wrote: > Doesn't this break using the remoting prefpane on older versions of OS X where > System Preferences is still a 32-bit app? The host is compiled for 64-bit only. I assumed that System Preferences is 64-bit app by default on systems that support 64-bit. Is that not true?
Message was sent while issue was closed.
mark@chromium.org changed reviewers: + mark@chromium.org
Message was sent while issue was closed.
It wasn’t always, but I do see an x86_64 slice in System Preferences on 10.6, which is the oldest that we currently care about.
Message was sent while issue was closed.
nicholss@chromium.org changed reviewers: + nicholss@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/1547533002/diff/160001/remoting/host/mac/me2m... File remoting/host/mac/me2me_preference_pane.mm (right): https://codereview.chromium.org/1547533002/diff/160001/remoting/host/mac/me2m... remoting/host/mac/me2me_preference_pane.mm:299: if (!remoting::VerifyHostPinHash(pin_utf8, host_id, host_secret_hash)) { The arguments are flipped here. I will have a CL to correct this in a few minutes. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
