Remove the obsolete enum webrtc::PeerConnectionInterface::IceState.

Remove the obsolete enum webrtc::PeerConnectionInterface::IceState.

Was replaced by IceConnectionState + IceGatheringState.

BUG=webrtc:6299
Committed: https://crrev.com/31dea98e9c87e640e185fd86fe63d952b5402e05
Cr-Commit-Position: refs/heads/master@{#13963}

[johan, 2016-08-17 12:43:38]

johan@webrtc.org changed reviewers:
+ pthatcher@webrtc.org

[johan, 2016-08-17 13:37:10]

The CQ bit was checked by johan@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-17 13:37:20]

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

[commit-bot: I haz the power, 2016-08-17 15:37:48]

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

[commit-bot: I haz the power, 2016-08-17 15:37:49]

Dry run: Try jobs failed on following builders:
  android_arm64_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_baremetal on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[pthatcher1, 2016-08-22 23:27:06]

lgtm

I found one product that uses this still, and so would break their build.  I
have notified them and asked them to remove it.  If we wait a few days (maybe a
week), we should be free to remove this.   Please don't submit until then.

[johan, 2016-08-23 07:40:13]

On 2016/08/22 23:27:06, pthatcher1 wrote:
> lgtm
> 
> I found one product that uses this still, and so would break their build.  I
> have notified them and asked them to remove it.  If we wait a few days (maybe
a
> week), we should be free to remove this.   Please don't submit until then.

Great. Will wait until next week. Thank you!

[pthatcher1, 2016-08-29 19:03:25]

I believe that product has been updated now and this can be submitted.

[pthatcher1, 2016-08-29 19:03:32]

The CQ bit was checked by pthatcher@webrtc.org

[commit-bot: I haz the power, 2016-08-29 19:03:41]

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

[commit-bot: I haz the power, 2016-08-29 21:11:33]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-08-29 21:11:41]

Description was changed from

==========
Remove the obsolete enum webrtc::PeerConnectionInterface::IceState.

Was replaced by IceConnectionState + IceGatheringState.

BUG=
==========

to

==========
Remove the obsolete enum webrtc::PeerConnectionInterface::IceState.

Was replaced by IceConnectionState + IceGatheringState.

BUG=

Committed: https://crrev.com/31dea98e9c87e640e185fd86fe63d952b5402e05
Cr-Commit-Position: refs/heads/master@{#13963}
==========

[commit-bot: I haz the power, 2016-08-29 21:11:42]

Patchset 1 (id:??) landed as
https://crrev.com/31dea98e9c87e640e185fd86fe63d952b5402e05
Cr-Commit-Position: refs/heads/master@{#13963}

[perkj_webrtc, 2016-08-30 06:51:04]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.webrtc.org/2290963002/ by perkj@webrtc.org.

The reason for reverting is: This breaks Chromes build. 
You will need to update tests in Chrome first. 

[1874/1925] CXX obj/content/test/test_support/mock_peer_connection_impl.o
FAILED: obj/content/test/test_support/mock_peer_connection_impl.o 
/b/c/cipd/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++
-MMD -MF obj/content/test/test_support/mock_peer_connection_impl.o.d
-DV8_DEPRECATION_WARNINGS -DENABLE_NOTIFICATIONS -DENABLE_PEPPER_CDMS
-DENABLE_PLUGINS=1 -DENABLE_PDF=1 -DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1
-DENABLE_PRINT_PREVIEW=1 -DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1
-DDCHECK_ALWAYS_ON=1 -DNO_TCMALLOC -DUSE_EXTERNAL_POPUP_MENU=1 -DENABLE_WEBRTC=1
-DENABLE_EXTENSIONS=1 -DENABLE_TASK_MANAGER=1 -DENABLE_THEMES=1
-DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DENABLE_SESSION_SERVICE=1
-DENABLE_PLUGIN_INSTALLATION=1 -DENABLE_SUPERVISED_USERS=1
-DENABLE_SERVICE_DISCOVERY=1 -DUSE_PROPRIETARY_CODECS -DFULL_SAFE_BROWSING
-DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD
-DENABLE_MEDIA_ROUTER=1 -DFIELDTRIAL_TESTING_ENABLED
-DCR_CLANG_REVISION=278861-1 -DCR_XCODE_VERSION=0511 -D__STDC_CONSTANT_MACROS
-D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2
-D__ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORE=0 -DNDEBUG -DNVALGRIND
-DDYNAMIC_ANNOTATIONS_ENABLED=0 -DV8_USE_EXTERNAL_STARTUP_DATA
-DGTEST_HAS_POSIX_RE=0 -DGTEST_LANG_CXX11=1 -DENABLE_IPC_FUZZER
-DSK_IGNORE_DW_GRAY_FIX -DSK_IGNORE_LINEONLY_AA_CONVEX_PATH_OPTS
-DSK_SUPPORT_GPU=1 -DSK_BUILD_FOR_MAC -DU_USING_ICU_NAMESPACE=0
-DU_ENABLE_DYLOAD=0 -DU_NOEXCEPT= -DU_STATIC_IMPLEMENTATION
-DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DENABLE_WEBSOCKETS
-DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DHAVE_PTHREAD
-DUSE_LIBJPEG_TURBO=1 -DENABLE_LAYOUT_UNIT_IN_INLINE_BOXES=0 -DENABLE_OILPAN=1
-DWTF_USE_CONCATENATED_IMPULSE_RESPONSES=1 -DWTF_USE_ICCJPEG=1
-DWTF_USE_QCMSLIB=1 -DLOG_DISABLED=0 -DMESA_EGL_NO_X11_HEADERS -DUNIT_TEST
-DLEVELDB_PLATFORM_CHROMIUM=1 -DFEATURE_ENABLE_SSL -DFEATURE_ENABLE_VOICEMAIL
-DEXPAT_RELATIVE_PATH -DGTEST_RELATIVE_PATH -DNO_MAIN_THREAD_WRAPPING
-DNO_SOUND_SYSTEM -DWEBRTC_CHROMIUM_BUILD -DWEBRTC_POSIX -DWEBRTC_MAC
-DSSL_USE_OPENSSL -DHAVE_OPENSSL_SSL_H -DFEATURE_ENABLE_SSL -DLOGGING=1
-DNO_MAIN_THREAD_WRAPPING -I../.. -Igen -I../../third_party/khronos -I../../gpu
-I../../third_party/libwebp -I../../testing/gtest/include -I../../skia/config
-I../../skia/ext -I../../third_party/skia/include/c
-I../../third_party/skia/include/config -I../../third_party/skia/include/core
-I../../third_party/skia/include/effects -I../../third_party/skia/include/images
-I../../third_party/skia/include/lazy -I../../third_party/skia/include/pathops
-I../../third_party/skia/include/pdf -I../../third_party/skia/include/pipe
-I../../third_party/skia/include/ports -I../../third_party/skia/include/utils
-I../../third_party/skia/include/gpu -I../../third_party/skia/src/gpu
-I../../third_party/icu/source/common -I../../third_party/icu/source/i18n
-I../../third_party/WebKit -Igen/third_party/WebKit -I../../v8/include -Igen
-I../../third_party/ced/src -I../../third_party/protobuf/src -Igen/protoc_out
-I../../third_party/protobuf/src -I../../third_party/boringssl/src/include
-I../../third_party/libjpeg_turbo -I../../third_party/WebKit/Source
-I../../third_party/WebKit -Igen/blink -Igen/third_party/WebKit
-I../../third_party/iccjpeg -I../../third_party/libpng -I../../third_party/zlib
-I../../third_party/ots/include -I../../third_party/qcms/src -I../../v8/include
-I../../third_party/mesa/src/include -I../../testing/gmock_custom
-I../../testing/gmock/include -I../../third_party/leveldatabase
-I../../third_party/leveldatabase/src
-I../../third_party/leveldatabase/src/include -I../../third_party/libwebm/source
-I../../third_party/opus/src/include -Igen/ui/resources -Igen/ui/resources
-I../../third_party/webrtc_overrides -I../../testing/gtest/include
-I../../third_party -I../../third_party/webrtc_overrides -I../../third_party
-I../../third_party/jsoncpp/overrides/include
-I../../third_party/jsoncpp/source/include -I../../third_party/libyuv
-I../../third_party/libyuv/include -I../../third_party/libvpx/source/libvpx
-fno-strict-aliasing -fstack-protector -fcolor-diagnostics -arch x86_64 -Wall
-Werror -Wextra -Wpartial-availability -Wno-missing-field-initializers
-Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default
-Wno-deprecated-register -Wno-unneeded-internal-declaration
-Wno-inconsistent-missing-override -Wno-shift-negative-value
-Wno-undefined-var-template -Wno-nonportable-include-path
-Wno-address-of-packed-member -O2 -g1 -isysroot
/Applications/Xcode5.1.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk
-mmacosx-version-min=10.7 -fvisibility=hidden -Xclang -load -Xclang
../../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 -Xclang
-plugin-arg-find-bad-constructs -Xclang follow-macro-expansion -Xclang
-plugin-arg-find-bad-constructs -Xclang enforce-in-pdf -Wheader-hygiene
-Wstring-conversion -Wno-unused-function -Xclang -load -Xclang
../../third_party/llvm-build/Release+Asserts/lib/libBlinkGCPlugin.dylib -Xclang
-add-plugin -Xclang blink-gc-plugin -fno-threadsafe-statics
-fvisibility-inlines-hidden -std=c++11 -stdlib=libc++ -fno-rtti -fno-exceptions
-c ../../content/renderer/media/mock_peer_connection_impl.cc -o
obj/content/test/test_support/mock_peer_connection_impl.o
In file included from
../../content/renderer/media/mock_peer_connection_impl.cc:5:
../../content/renderer/media/mock_peer_connection_impl.h:52:3: error: unknown
type name 'IceState'
  IceState ice_state() override {
  ^
../../content/renderer/media/mock_peer_connection_impl.h:54:37: error: no member
named 'kIceNew' in 'webrtc::PeerConnectionInterface'
    return PeerConnectionInterface::kIceNew;




See for example
https://build.chromium.org/p/chromium.webrtc.fyi/builders/Mac%20Builder/build...
           ~~~~~~~~~~~~~~~~~~~~~~~~~^.

[johan, 2016-09-02 10:32:16]

How should we proceed here?  
IceState PeerConnectionInterface::ice_state() is pure virtual.
That makes it difficult to remove the mock method implementation from Chromium.

I suggest following approach:

1) Provide a default implementation in PeerConnectionInterface for a limited
time frame. Like 

IceState PeerConnectionInterface::ice_state() {
  RTC_NOTREACHED;
  return kIceNew;
}


2) Remove the ice_state() implementation from Chromium's
mock_peer_connection_impl.h [1].

chromium-linux/src((ac9c785...))$ git diff
diff --git a/content/renderer/media/mock_peer_connection_impl.h
b/content/renderer/media/mock_peer_connection_impl.h
index c6a1d19..e597f6e 100644
--- a/content/renderer/media/mock_peer_connection_impl.h
+++ b/content/renderer/media/mock_peer_connection_impl.h
@@ -49,10 +49,6 @@ class MockPeerConnectionImpl : public
webrtc::PeerConnectionInterface {
     NOTIMPLEMENTED();
     return PeerConnectionInterface::kStable;
   }
-  IceState ice_state() override {
-    NOTIMPLEMENTED();
-    return PeerConnectionInterface::kIceNew;
-  }
   IceConnectionState ice_connection_state() override {
     NOTIMPLEMENTED();
     return PeerConnectionInterface::kIceConnectionNew;

3) Wait a few days. Exact time frame is t.b.d.

4) Reland this patch.



[1]
https://cs.chromium.org/chromium/src/content/renderer/media/mock_peer_connect...

[sprang_webrtc, 2016-09-02 11:55:18]

On 2016/09/02 10:32:16, johan wrote:
> How should we proceed here?  
> IceState PeerConnectionInterface::ice_state() is pure virtual.
> That makes it difficult to remove the mock method implementation from
Chromium.
> 
> I suggest following approach:
> 
> 1) Provide a default implementation in PeerConnectionInterface for a limited
> time frame. Like 
> 
> IceState PeerConnectionInterface::ice_state() {
>   RTC_NOTREACHED;
>   return kIceNew;
> }
> 
> 
> 2) Remove the ice_state() implementation from Chromium's
> mock_peer_connection_impl.h [1].
> 
> chromium-linux/src((ac9c785...))$ git diff
> diff --git a/content/renderer/media/mock_peer_connection_impl.h
> b/content/renderer/media/mock_peer_connection_impl.h
> index c6a1d19..e597f6e 100644
> --- a/content/renderer/media/mock_peer_connection_impl.h
> +++ b/content/renderer/media/mock_peer_connection_impl.h
> @@ -49,10 +49,6 @@ class MockPeerConnectionImpl : public
> webrtc::PeerConnectionInterface {
>      NOTIMPLEMENTED();
>      return PeerConnectionInterface::kStable;
>    }
> -  IceState ice_state() override {
> -    NOTIMPLEMENTED();
> -    return PeerConnectionInterface::kIceNew;
> -  }
>    IceConnectionState ice_connection_state() override {
>      NOTIMPLEMENTED();
>      return PeerConnectionInterface::kIceConnectionNew;
> 
> 3) Wait a few days. Exact time frame is t.b.d.
> 
> 4) Reland this patch.
> 
> 
> 
> [1]
>
https://cs.chromium.org/chromium/src/content/renderer/media/mock_peer_connect...

Yes, that is essentially standard practice.

[johan, 2016-09-02 12:55:33]

Description was changed from

==========
Remove the obsolete enum webrtc::PeerConnectionInterface::IceState.

Was replaced by IceConnectionState + IceGatheringState.

BUG=

Committed: https://crrev.com/31dea98e9c87e640e185fd86fe63d952b5402e05
Cr-Commit-Position: refs/heads/master@{#13963}
==========

to

==========
Remove the obsolete enum webrtc::PeerConnectionInterface::IceState.

Was replaced by IceConnectionState + IceGatheringState.

BUG=webrtc:6299

Committed: https://crrev.com/31dea98e9c87e640e185fd86fe63d952b5402e05
Cr-Commit-Position: refs/heads/master@{#13963}
==========