Removing TODO for It2Me Confirmation Dialog

Removing TODO for It2Me Confirmation Dialog

Now that the It2Me confirmation dialog has been implemented on all platforms
we currently support, we can remove the TODO/workaround which would auto-
approve connections for platforms w/o a dialog.  Note that Android has an
It2Me host implementation but is not currently used so I did not implement
a dialog there.  I have added a NOTIMPLEMENTED() macro to indicate this if we
decide to proceed with it.

BUG=645540
Committed: https://crrev.com/a4829361cdafaa4fe5e5b70b4a786289bb66f97c
Committed: https://crrev.com/1015b508f2699cd15bf917a12e2077cdfcb52996
Cr-Original-Commit-Position: refs/heads/master@{#419776}
Cr-Commit-Position: refs/heads/master@{#419814}

[joedow, 2016-09-16 16:12:02]

The CQ bit was checked by joedow@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 16:12:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 17:11:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-16 17:11:07]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[joedow, 2016-09-16 17:15:38]

The CQ bit was checked by joedow@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 17:16:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 18:08:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-16 18:08:40]

Dry run: This issue passed the CQ dry run.

[joedow, 2016-09-16 18:11:10]

joedow@chromium.org changed reviewers:
+ sergeyu@chromium.org

[joedow, 2016-09-16 18:11:11]

Hopefully this is the last CL for the It2Me confirmation dialog changes.  PTAL!

[Jamie, 2016-09-16 19:50:41]

jamiewalch@chromium.org changed reviewers:
+ jamiewalch@chromium.org

[Jamie, 2016-09-16 19:50:42]

lgtm

[Sergey Ulanov, 2016-09-19 20:56:36]

lgtm

https://codereview.chromium.org/2342393003/diff/20001/remoting/host/it2me/it2...
File remoting/host/it2me/it2me_confirmation_dialog_android.cc (right):

https://codereview.chromium.org/2342393003/diff/20001/remoting/host/it2me/it2...
remoting/host/it2me/it2me_confirmation_dialog_android.cc:15: NOTIMPLEMENTED();
Should this be NOTREACHED()? The calling code doesn't expect nullptr result.
OTOH we shouldn't be compiling host on android, so I'm not sure we even need
this file.

[Lambros, 2016-09-19 21:30:02]

On 2016/09/19 20:56:36, Sergey Ulanov wrote:
> lgtm
> 
>
https://codereview.chromium.org/2342393003/diff/20001/remoting/host/it2me/it2...
> File remoting/host/it2me/it2me_confirmation_dialog_android.cc (right):
> 
>
https://codereview.chromium.org/2342393003/diff/20001/remoting/host/it2me/it2...
> remoting/host/it2me/it2me_confirmation_dialog_android.cc:15: NOTIMPLEMENTED();
> Should this be NOTREACHED()? The calling code doesn't expect nullptr result.
> OTOH we shouldn't be compiling host on android, so I'm not sure we even need
> this file.

There is a remoting_host_apk target that builds the host stuff, but nothing
depends on it so it's probably not being built.
We should consider removing it if we're not planning to implement host for
Android.

[joedow, 2016-09-19 23:40:27]

The CQ bit was checked by joedow@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-19 23:40:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-20 01:17:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-20 01:17:46]

Dry run: This issue passed the CQ dry run.

[joedow, 2016-09-20 03:23:41]

Thanks!

https://codereview.chromium.org/2342393003/diff/20001/remoting/host/it2me/it2...
File remoting/host/it2me/it2me_confirmation_dialog_android.cc (right):

https://codereview.chromium.org/2342393003/diff/20001/remoting/host/it2me/it2...
remoting/host/it2me/it2me_confirmation_dialog_android.cc:15: NOTIMPLEMENTED();
On 2016/09/19 20:56:35, Sergey Ulanov wrote:
> Should this be NOTREACHED()? The calling code doesn't expect nullptr result.
> OTOH we shouldn't be compiling host on android, so I'm not sure we even need
> this file.

I added it to make things simpler if we decided to proceed with the Android host
in the future, since you and Lambros are both suggesting I remove it then I am
happy to do so.

[joedow, 2016-09-20 15:42:18]

The CQ bit was checked by joedow@chromium.org

[joedow, 2016-09-20 15:42:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
sergeyu@chromium.org, jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/2342393003/#ps40001
(title: "Removing Android cc file")

[commit-bot: I haz the power, 2016-09-20 15:42:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-20 15:48:07]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-20 15:49:15]

Description was changed from

==========
Removing TODO for It2Me Confirmation Dialog

Now that the It2Me confirmation dialog has been implemented on all platforms
we currently support, we can remove the TODO/workaround which would auto-
approve connections for platforms w/o a dialog.  Note that Android has an
It2Me host implementation but is not currently used so I did not implement
a dialog there.  I have added a NOTIMPLEMENTED() macro to indicate this if we
decide to proceed with it.

BUG=645540
==========

to

==========
Removing TODO for It2Me Confirmation Dialog

Now that the It2Me confirmation dialog has been implemented on all platforms
we currently support, we can remove the TODO/workaround which would auto-
approve connections for platforms w/o a dialog.  Note that Android has an
It2Me host implementation but is not currently used so I did not implement
a dialog there.  I have added a NOTIMPLEMENTED() macro to indicate this if we
decide to proceed with it.

BUG=645540

Committed: https://crrev.com/a4829361cdafaa4fe5e5b70b4a786289bb66f97c
Cr-Commit-Position: refs/heads/master@{#419776}
==========

[commit-bot: I haz the power, 2016-09-20 15:49:15]

Patchset 3 (id:??) landed as
https://crrev.com/a4829361cdafaa4fe5e5b70b4a786289bb66f97c
Cr-Commit-Position: refs/heads/master@{#419776}

[dewittj, 2016-09-20 16:54:03]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2352123002/ by dewittj@chromium.org.

The reason for reverting is: Compile failure

see https://build.chromium.org/p/chromium/builders/Android/builds/62438/

FAILED: libremoting_host_jni.so libremoting_host_jni.so.TOC
lib.unstripped/libremoting_host_jni.so 
python "/b/c/b/Android/src/build/toolchain/gcc_solink_wrapper.py"
--readelf="../../third_party/android_tools/ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-readelf"
--nm="../../third_party/android_tools/ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-nm"
--strip=../../third_party/android_tools/ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-strip
--sofile="./lib.unstripped/libremoting_host_jni.so"
--tocfile="./libremoting_host_jni.so.TOC" --output="./libremoting_host_jni.so" 
--
../../third_party/android_tools/ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-g++
-shared -Wl,--fatal-warnings -fPIC -Wl,-z,noexecstack -Wl,-z,now -Wl,-z,relro
-Wl,-z,defs -Wl,--as-needed -fuse-ld=gold -Wl,--icf=all -Wl,--build-id=sha1
-Wl,--no-undefined -Wl,--exclude-libs=libgcc.a
-Wl,--exclude-libs=libc++_static.a -Wl,--exclude-libs=libvpx_assembly_arm.a
-Wl,--warn-shared-textrel -Wl,-O1 -Wl,--gc-sections -nostdlib
-Wl,--warn-shared-textrel
--sysroot=../../third_party/android_tools/ndk/platforms/android-16/arch-arm 
-Wl,--version-script=/b/c/b/Android/src/build/android/android_no_jni_exports.lst
-Wl,-wrap,calloc -Wl,-wrap,free -Wl,-wrap,malloc -Wl,-wrap,memalign
-Wl,-wrap,posix_memalign -Wl,-wrap,pvalloc -Wl,-wrap,realloc -Wl,-wrap,valloc
-L../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libs/armeabi-v7a
-o "./lib.unstripped/libremoting_host_jni.so"
-Wl,-soname="libremoting_host_jni.so" @"./libremoting_host_jni.so.rsp"
obj/remoting/host/it2me/common/it2me_confirmation_dialog.o:it2me_confirmation_dialog.cc:vtable
for remoting::It2MeConfirmationDialogFactory: error: undefined reference to
'remoting::It2MeConfirmationDialogFactory::Create()'
collect2: error: ld returned 1 exit status.

[joedow, 2016-09-20 16:59:14]

Patchset #3 (id:40001) has been deleted

[findit-for-me, 2016-09-20 17:19:14]

FYI: Findit identified this CL at revision 419776 as the culprit for
failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb...

[joedow, 2016-09-20 17:34:40]

Description was changed from

==========
Removing TODO for It2Me Confirmation Dialog

Now that the It2Me confirmation dialog has been implemented on all platforms
we currently support, we can remove the TODO/workaround which would auto-
approve connections for platforms w/o a dialog.  Note that Android has an
It2Me host implementation but is not currently used so I did not implement
a dialog there.  I have added a NOTIMPLEMENTED() macro to indicate this if we
decide to proceed with it.

BUG=645540

Committed: https://crrev.com/a4829361cdafaa4fe5e5b70b4a786289bb66f97c
Cr-Commit-Position: refs/heads/master@{#419776}
==========

to

==========
Removing TODO for It2Me Confirmation Dialog

Now that the It2Me confirmation dialog has been implemented on all platforms
we currently support, we can remove the TODO/workaround which would auto-
approve connections for platforms w/o a dialog.  Note that Android has an
It2Me host implementation but is not currently used so I did not implement
a dialog there.  I have added a NOTIMPLEMENTED() macro to indicate this if we
decide to proceed with it.

BUG=645540

Committed: https://crrev.com/a4829361cdafaa4fe5e5b70b4a786289bb66f97c
Cr-Commit-Position: refs/heads/master@{#419776}
==========

[joedow, 2016-09-20 17:35:13]

The CQ bit was checked by joedow@chromium.org

[joedow, 2016-09-20 17:35:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
sergeyu@chromium.org, jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/2342393003/#ps60001
(title: "Switching Logging statement")

[commit-bot: I haz the power, 2016-09-20 17:35:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[cblume, 2016-09-20 17:49:38]

On 2016/09/20 17:35:39, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

I think this might be ....whoa -- findit-for-me!!
I was just about to say I think this is causing a crash.

[commit-bot: I haz the power, 2016-09-20 18:17:04]

Description was changed from

==========
Removing TODO for It2Me Confirmation Dialog

Now that the It2Me confirmation dialog has been implemented on all platforms
we currently support, we can remove the TODO/workaround which would auto-
approve connections for platforms w/o a dialog.  Note that Android has an
It2Me host implementation but is not currently used so I did not implement
a dialog there.  I have added a NOTIMPLEMENTED() macro to indicate this if we
decide to proceed with it.

BUG=645540

Committed: https://crrev.com/a4829361cdafaa4fe5e5b70b4a786289bb66f97c
Cr-Commit-Position: refs/heads/master@{#419776}
==========

to

==========
Removing TODO for It2Me Confirmation Dialog

Now that the It2Me confirmation dialog has been implemented on all platforms
we currently support, we can remove the TODO/workaround which would auto-
approve connections for platforms w/o a dialog.  Note that Android has an
It2Me host implementation but is not currently used so I did not implement
a dialog there.  I have added a NOTIMPLEMENTED() macro to indicate this if we
decide to proceed with it.

BUG=645540

Committed: https://crrev.com/a4829361cdafaa4fe5e5b70b4a786289bb66f97c
Cr-Commit-Position: refs/heads/master@{#419776}
==========

[commit-bot: I haz the power, 2016-09-20 18:17:06]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-09-20 18:18:52]

Description was changed from

==========
Removing TODO for It2Me Confirmation Dialog

Now that the It2Me confirmation dialog has been implemented on all platforms
we currently support, we can remove the TODO/workaround which would auto-
approve connections for platforms w/o a dialog.  Note that Android has an
It2Me host implementation but is not currently used so I did not implement
a dialog there.  I have added a NOTIMPLEMENTED() macro to indicate this if we
decide to proceed with it.

BUG=645540

Committed: https://crrev.com/a4829361cdafaa4fe5e5b70b4a786289bb66f97c
Cr-Commit-Position: refs/heads/master@{#419776}
==========

to

==========
Removing TODO for It2Me Confirmation Dialog

Now that the It2Me confirmation dialog has been implemented on all platforms
we currently support, we can remove the TODO/workaround which would auto-
approve connections for platforms w/o a dialog.  Note that Android has an
It2Me host implementation but is not currently used so I did not implement
a dialog there.  I have added a NOTIMPLEMENTED() macro to indicate this if we
decide to proceed with it.

BUG=645540

Committed: https://crrev.com/a4829361cdafaa4fe5e5b70b4a786289bb66f97c
Committed: https://crrev.com/1015b508f2699cd15bf917a12e2077cdfcb52996
Cr-Original-Commit-Position: refs/heads/master@{#419776}
Cr-Commit-Position: refs/heads/master@{#419814}
==========

[commit-bot: I haz the power, 2016-09-20 18:18:53]

Patchset 3 (id:??) landed as
https://crrev.com/1015b508f2699cd15bf917a12e2077cdfcb52996
Cr-Commit-Position: refs/heads/master@{#419814}