Issue 1417743005: Update media notification when page title changes

Zhiqiang Zhang (Slow)

zqzhang@chromium.org changed reviewers: + avayvod@chromium.org, mlamouri@chromium.org

5 years, 1 month ago (2015-11-05 14:23:00 UTC) #1

Zhiqiang Zhang (Slow)

On 2015/11/05 14:23:01, zqzhang wrote: Sorry, I guess there are some naming issues. I got ...

5 years, 1 month ago (2015-11-05 14:29:23 UTC) #3

whywhat

https://codereview.chromium.org/1417743005/diff/1/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java File chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java (right): https://codereview.chromium.org/1417743005/diff/1/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java#newcode127 chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java:127: MediaNotificationManager.updateTitle(tab, R.id.media_playback_notification); Note that when updating the tab title ...

5 years, 1 month ago (2015-11-05 14:31:46 UTC) #4

mlamouri (slow - plz ping)

https://codereview.chromium.org/1417743005/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java File chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java (right): https://codereview.chromium.org/1417743005/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java#newcode136 chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java:136: MediaNotificationManager.update( I'm afraid things will not work well if ...

5 years, 1 month ago (2015-11-05 17:22:28 UTC) #6

mlamouri (slow - plz ping)

Could you add an instrumentation test in chrome/android/javatests/src/org/chromium/chrome/browser/media/ (needs to be created) in order to ...

5 years, 1 month ago (2015-11-06 14:23:48 UTC) #8

Zhiqiang Zhang (Slow)

The CQ bit was unchecked by zqzhang@chromium.org

5 years, 1 month ago (2015-11-10 14:52:13 UTC) #9

Zhiqiang Zhang (Slow)

The CQ bit was checked by zqzhang@chromium.org

5 years, 1 month ago (2015-11-10 14:52:15 UTC) #10

Zhiqiang Zhang (Slow)

The CQ bit was unchecked by zqzhang@chromium.org

5 years, 1 month ago (2015-11-10 14:52:24 UTC) #11

whywhat

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

5 years, 1 month ago (2015-11-10 18:52:11 UTC) #13

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1417743005/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1417743005/60001

5 years, 1 month ago (2015-11-10 18:52:55 UTC) #14

commit-bot: I haz the power

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

5 years, 1 month ago (2015-11-10 18:56:28 UTC) #15

commit-bot: I haz the power

Dry run: Try jobs failed on following builders: ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator_ninja/builds/92445) mac_chromium_compile_dbg_ng on ...

5 years, 1 month ago (2015-11-10 18:56:29 UTC) #16

Zhiqiang Zhang (Slow)

https://codereview.chromium.org/1417743005/diff/80001/chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java File chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java (right): https://codereview.chromium.org/1417743005/diff/80001/chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java#newcode93 chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:93: manager.onStop(MediaNotificationListener.ACTION_SOURCE_MEDIA_NOTIFICATION); The test fails here. test_runner.py only shows an ...

5 years, 1 month ago (2015-11-10 21:43:12 UTC) #18

https://codereview.chromium.org/1417743005/diff/80001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java
(right):

https://codereview.chromium.org/1417743005/diff/80001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:93:
manager.onStop(MediaNotificationListener.ACTION_SOURCE_MEDIA_NOTIFICATION);
The test fails here. test_runner.py only shows an "UNKNOWN" fail. Here's the adb
lolcat logs. Maybe you should start from the "RuntimeException" part:


11-10 21:15:49.522 14051 14068 I TestRunner: started:
testTitleUpdated_ControlledStopping(org.chromium.chrom.browser.media.ui.NotificationTitleUpdatedTest)
11-10 21:15:49.539   900   911 I ActivityManager: START u0
{act=android.intent.action.VIEW cat=[android.intent.category.LAUNCHER]
dat=about:blank flg=0x10000000
cmp=org.chromium.chrome/.browser.document.ChromeLauncherActivity} from uid 10157
on display 0
11-10 21:15:49.550 18688 21510 W SearchService: Abort, client detached.
11-10 21:15:49.550 18688 21510 W SearchService: Abort, client detached.
11-10 21:15:49.550 18688 21510 W SearchService: Abort, client detached.
11-10 21:15:49.554 18688 18688 I HotwordDetector: Closing mic
11-10 21:15:49.554 18688 19092 I MicrophoneInputStream: mic_close
com.google.android.apps.gsa.speech.audio.y@4417880
11-10 21:15:49.554 18688 13942 E AudioRecord-JNI: Error -4 during AudioRecord
native read
11-10 21:15:49.578 14051 14077 E cr_StorageDelegate: DocumentTabModel file not
found.
11-10 21:15:49.582   900   911 I ActivityManager: START u0
{act=android.intent.action.VIEW cat=[android.intent.category.LAUNCHER]
dat=document://723?about:blank flg=0x10082000
cmp=org.chromium.chrome/.browser.document.DocumentActivity (has extras)} from
uid 10157 on display 0
11-10 21:15:49.612   493 13944 D audio_hw_primary: disable_audio_route: reset
and update mixer path: audio-record
11-10 21:15:49.613   493 13944 D audio_hw_primary: disable_snd_device:
snd_device(61: voice-rec-mic)
11-10 21:15:49.617   493 13944 I soundtrigger:
audio_extn_sound_trigger_update_device_status: device 0x3d of type 1 for Event 0
11-10 21:15:49.617   493 13944 D sound_trigger_platform:
platform_stdev_check_and_update_concurrency: concurrency active 0, tx 0, rx 0,
concurrency session_allowed 1
11-10 21:15:49.618   493  2914 I SoundTriggerHwService::Module: void
android::SoundTriggerHwService::Module::onCallbackEvent(const
android::sp<android::SoundTriggerHwService::CallbackEvent>&) mClient == 0
11-10 21:15:49.623 18688 13941 I MicroRecognitionRnrImpl: Detection finished
11-10 21:15:49.624 18688 19095 I MicroRecognitionRnrImpl: Stopping hotword
detection.
11-10 21:15:49.634 14051 14051 W ResourceType: Failure getting entry for
0x7f04005a (t=3 e=90) (error -75)
11-10 21:15:49.672 14051 14079 I cr_LibraryLoader: Using linker:
org.chromium.base.library_loader.ModernLinker
11-10 21:15:49.675 14051 14080 I cr_base : Extracting resource
/data/user/0/org.chromium.chrome/app_chrome/paks/en-GB.pak
11-10 21:15:49.680 14051 14080 I cr_base : Extracting resource
/data/user/0/org.chromium.chrome/app_chrome/paks/en-US.pak
11-10 21:15:49.690 14051 14081 D OpenGLRenderer: Use
EGL_SWAP_BEHAVIOR_PRESERVED: true
11-10 21:15:49.695 14051 14079 I cr_LibraryLoader: Loading chrome_public
11-10 21:15:49.744 14051 14081 I Adreno  : QUALCOMM build                   :
63c06b2, I8366cd0437
11-10 21:15:49.744 14051 14081 I Adreno  : Build Date                       :
10/21/15
11-10 21:15:49.744 14051 14081 I Adreno  : OpenGL ES Shader Compiler Version:
XE031.05.13.02
11-10 21:15:49.744 14051 14081 I Adreno  : Local Branch                     : 
11-10 21:15:49.744 14051 14081 I Adreno  : Remote Branch                    :
quic/LA.BF64.1.2.9_v2
11-10 21:15:49.744 14051 14081 I Adreno  : Remote Branch                    :
NONE
11-10 21:15:49.744 14051 14081 I Adreno  : Reconstruct Branch               :
NOTHING
11-10 21:15:49.753 14051 14081 I OpenGLRenderer: Initialized EGL, version 1.4
11-10 21:15:49.754 14051 14079 I cr_LibraryLoader: Time to load native
libraries: 82 ms (timestamps 6131-6213)
11-10 21:15:49.754 14051 14079 I cr_LibraryLoader: Expected native library
version number "48.0.2561.0", actual native library version number "48.0.2561.0"
11-10 21:15:49.754 14051 14079 I chromium: [INFO:library_loader_hooks.cc(130)]
Chromium logging enabled: level = 0, default verbosity = 0
11-10 21:15:49.771   900  3718 I ActivityManager: Start proc
14083:org.chromium.chrome:sandboxed_process0/u0i218 for service
org.chromium.chrome/org.chromium.content.app.SandboxedProcessService0
11-10 21:15:49.813 14083 14099 W FileUtils: Failed to
chmod(/data/user/0/org.chromium.chrome/app_chrome):
android.system.ErrnoException: chmod failed: EACCES (Permission denied)
11-10 21:15:49.814 14083 14099 W ContextImpl: Unable to create files subdir
/data/user/0/org.chromium.chrome/cache
11-10 21:15:49.806 14099 14099 W org.chromium.chrome:sandboxed_process0:
type=1400 audit(0.0:50291): avc: denied { search } for
comm=4173796E635461736B202331 name="org.chromium.chrome" dev="dm-2" ino=409009
scontext=u:r:isolated_app:s0:c512,c768
tcontext=u:object_r:app_data_file:s0:c512,c768 tclass=dir permissive=0
11-10 21:15:49.806 14099 14099 W org.chromium.chrome:sandboxed_process0:
type=1400 audit(0.0:50292): avc: denied { search } for
comm=4173796E635461736B202331 name="org.chromium.chrome" dev="dm-2" ino=409009
scontext=u:r:isolated_app:s0:c512,c768
tcontext=u:object_r:app_data_file:s0:c512,c768 tclass=dir permissive=0
11-10 21:15:49.806 14099 14099 W org.chromium.chrome:sandboxed_process0:
type=1400 audit(0.0:50293): avc: denied { search } for
comm=4173796E635461736B202331 name="org.chromium.chrome" dev="dm-2" ino=409009
scontext=u:r:isolated_app:s0:c512,c768
tcontext=u:object_r:app_data_file:s0:c512,c768 tclass=dir permissive=0
11-10 21:15:49.806 14099 14099 W org.chromium.chrome:sandboxed_process0:
type=1400 audit(0.0:50294): avc: denied { search } for
comm=4173796E635461736B202331 name="org.chromium.chrome" dev="dm-2" ino=409009
scontext=u:r:isolated_app:s0:c512,c768
tcontext=u:object_r:app_data_file:s0:c512,c768 tclass=dir permissive=0
11-10 21:15:49.806 14099 14099 W org.chromium.chrome:sandboxed_process0:
type=1400 audit(0.0:50295): avc: denied { search } for
comm=4173796E635461736B202331 name="org.chromium.chrome" dev="dm-2" ino=409009
scontext=u:r:isolated_app:s0:c512,c768
tcontext=u:object_r:app_data_file:s0:c512,c768 tclass=dir permissive=0
11-10 21:15:49.806 14099 14099 W org.chromium.chrome:sandboxed_process0:
type=1400 audit(0.0:50296): avc: denied { search } for
comm=4173796E635461736B202331 name="org.chromium.chrome" dev="dm-2" ino=409009
scontext=u:r:isolated_app:s0:c512,c768
tcontext=u:object_r:app_data_file:s0:c512,c768 tclass=dir permissive=0
11-10 21:15:49.815 14083 14083 I cr_ChildProcessService: Creating new
ChildProcessService pid=14083
11-10 21:15:49.878   900   919 I ActivityManager: Displayed
org.chromium.chrome/.browser.document.DocumentActivity: +287ms (total +327ms)
11-10 21:15:49.884  3566  3566 I Keyboard.Facilitator: onFinishInput()
11-10 21:15:49.887 14051 14051 I cr_BrowserStartup: Initializing chromium
process, singleProcess=false
11-10 21:15:49.956   900   918 D BluetoothManagerService: Added callback:
android.bluetooth.IBluetoothManagerCallback$Stub$Proxy@cc58729:true
11-10 21:15:49.962 14051 14051 E chromium:
[ERROR:shell_integration_android.cc(24)] Not implemented reached in static
ShellIntegration::DefaultWebClientSetPermission
ShellIntegration::CanSetAsDefaultBrowser()
11-10 21:15:49.971   900 10253 I ActivityManager: Start proc
14117:org.chromium.chrome:privileged_process0/u0a157 for service
org.chromium.chrome/org.chromium.content.app.PrivilegedProcessService0
11-10 21:15:49.990   900  3718 D ConnectivityService: listenForNetwork for
Listen from uid/pid:10157/14051 for NetworkRequest [ id=390, legacyType=-1, [
Capabilities: INTERNET&NOT_RESTRICTED&TRUSTED&NOT_VPN] ]
11-10 21:15:50.018 14117 14117 I cr_ChildProcessService: Creating new
ChildProcessService pid=14117
11-10 21:15:50.021 14117 14136 I cr_LibraryLoader: Using linker:
org.chromium.base.library_loader.ModernLinker
11-10 21:15:50.037 14051 14051 W InstanceID/Rpc: Found 10013
11-10 21:15:50.042 14117 14136 I cr_LibraryLoader: Loading chrome_public
11-10 21:15:50.061   900 18584 D ConnectivityService: listenForNetwork for
Listen from uid/pid:10157/14051 for NetworkRequest [ id=391, legacyType=-1, [
Capabilities: INTERNET&NOT_RESTRICTED&TRUSTED&NOT_VPN] ]
11-10 21:15:50.066 14112 14112 W Thread-3010: type=1400 audit(0.0:50297): avc:
denied { ioctl } for path="socket:[2242264]" dev="sockfs" ino=2242264
ioctlcmd=8b1b scontext=u:r:untrusted_app:s0:c512,c768
tcontext=u:r:untrusted_app:s0:c512,c768 tclass=udp_socket permissive=0
11-10 21:15:50.066 14112 14112 W Thread-3010: type=1400 audit(0.0:50298): avc:
denied { ioctl } for path="socket:[2242265]" dev="sockfs" ino=2242265
ioctlcmd=8b1b scontext=u:r:untrusted_app:s0:c512,c768
tcontext=u:r:untrusted_app:s0:c512,c768 tclass=udp_socket permissive=0
11-10 21:15:50.066 14112 14112 W Thread-3010: type=1400 audit(0.0:50299): avc:
denied { ioctl } for path="socket:[2242266]" dev="sockfs" ino=2242266
ioctlcmd=8b1b scontext=u:r:untrusted_app:s0:c512,c768
tcontext=u:r:untrusted_app:s0:c512,c768 tclass=udp_socket permissive=0
11-10 21:15:50.066 14112 14112 W Thread-3010: type=1400 audit(0.0:50300): avc:
denied { ioctl } for path="socket:[2242267]" dev="sockfs" ino=2242267
ioctlcmd=8b1b scontext=u:r:untrusted_app:s0:c512,c768
tcontext=u:r:untrusted_app:s0:c512,c768 tclass=udp_socket permissive=0
11-10 21:15:50.066 14112 14112 W Thread-3010: type=1400 audit(0.0:50301): avc:
denied { ioctl } for path="socket:[2242268]" dev="sockfs" ino=2242268
ioctlcmd=8b1b scontext=u:r:untrusted_app:s0:c512,c768
tcontext=u:r:untrusted_app:s0:c512,c768 tclass=udp_socket permissive=0
11-10 21:15:50.066 14112 14112 W Thread-3010: type=1400 audit(0.0:50302): avc:
denied { ioctl } for path="socket:[2242269]" dev="sockfs" ino=2242269
ioctlcmd=8b1b scontext=u:r:untrusted_app:s0:c512,c768
tcontext=u:r:untrusted_app:s0:c512,c768 tclass=udp_socket permissive=0
11-10 21:15:50.085 14117 14136 I cr_LibraryLoader: Time to load native
libraries: 64 ms (timestamps 6481-6545)
11-10 21:15:50.085 14117 14136 I cr_LibraryLoader: Expected native library
version number "48.0.2561.0", actual native library version number "48.0.2561.0"
11-10 21:15:50.086 14117 14136 I chromium: [INFO:library_loader_hooks.cc(130)]
Chromium logging enabled: level = 0, default verbosity = 0
11-10 21:15:50.101 14117 14136 E libEGL  : validate_display:255 error 3008
(EGL_BAD_DISPLAY)
11-10 21:15:50.102 14117 14136 I Adreno  : QUALCOMM build                   :
63c06b2, I8366cd0437
11-10 21:15:50.102 14117 14136 I Adreno  : Build Date                       :
10/21/15
11-10 21:15:50.102 14117 14136 I Adreno  : OpenGL ES Shader Compiler Version:
XE031.05.13.02
11-10 21:15:50.102 14117 14136 I Adreno  : Local Branch                     : 
11-10 21:15:50.102 14117 14136 I Adreno  : Remote Branch                    :
quic/LA.BF64.1.2.9_v2
11-10 21:15:50.102 14117 14136 I Adreno  : Remote Branch                    :
NONE
11-10 21:15:50.102 14117 14136 I Adreno  : Reconstruct Branch               :
NOTHING
11-10 21:15:50.132 14117 14141 I OMXClient: Using client-side OMX mux.
11-10 21:15:50.146 14094 14094 W Binder_1: type=1400 audit(0.0:50303): avc:
denied { ioctl } for path="socket:[2246822]" dev="sockfs" ino=2246822
ioctlcmd=7704 scontext=u:r:isolated_app:s0:c512,c768
tcontext=u:r:untrusted_app:s0:c512,c768 tclass=unix_stream_socket permissive=0
11-10 21:15:50.146 14094 14094 W Binder_1: type=1400 audit(0.0:50304): avc:
denied { ioctl } for
path="/data/data/org.chromium.chrome/app_chrome/paks/en-US.pak" dev="dm-2"
ino=409041 ioctlcmd=7704 scontext=u:r:isolated_app:s0:c512,c768
tcontext=u:object_r:app_data_file:s0:c512,c768 tclass=file permissive=0
11-10 21:15:50.162 14083 14094 I cr_LibraryLoader: Using linker:
org.chromium.base.library_loader.ModernLinker
11-10 21:15:50.146 14094 14094 W Binder_1: type=1400 audit(0.0:50305): avc:
denied { ioctl } for
path=2F646174612F646174612F6F72672E6368726F6D69756D2E6368726F6D652F6170705F6368726F6D652F52454C524F3A6C69626368726F6D655F7075626C69632E736F202864656C6574656429
dev="dm-2" ino=409043 ioctlcmd=7704 scontext=u:r:isolated_app:s0:c512,c768
tcontext=u:object_r:app_data_file:s0:c512,c768 tclass=file permissive=0
11-10 21:15:50.156 14094 14094 W Binder_1: type=1400 audit(0.0:50306): avc:
denied { ioctl } for
path=2F646174612F646174612F6F72672E6368726F6D69756D2E6368726F6D652F6170705F6368726F6D652F52454C524F3A6C69626368726F6D655F7075626C69632E736F202864656C6574656429
dev="dm-2" ino=409043 ioctlcmd=7704 scontext=u:r:isolated_app:s0:c512,c768
tcontext=u:object_r:app_data_file:s0:c512,c768 tclass=file permissive=0
11-10 21:15:50.156 14094 14094 W Binder_1: type=1400 audit(0.0:50307): avc:
denied { ioctl } for
path=2F646174612F646174612F6F72672E6368726F6D69756D2E6368726F6D652F6170705F6368726F6D652F52454C524F3A6C69626368726F6D655F7075626C69632E736F202864656C6574656429
dev="dm-2" ino=409043 ioctlcmd=7704 scontext=u:r:isolated_app:s0:c512,c768
tcontext=u:object_r:app_data_file:s0:c512,c768 tclass=file permissive=0
11-10 21:15:50.156 14094 14094 W Binder_1: type=1400 audit(0.0:50308): avc:
denied { ioctl } for
path="/data/data/org.chromium.chrome/app_chrome/paks/en-US.pak" dev="dm-2"
ino=409041 ioctlcmd=7704 scontext=u:r:isolated_app:s0:c512,c768
tcontext=u:object_r:app_data_file:s0:c512,c768 tclass=file permissive=0
11-10 21:15:50.156 14094 14094 W Binder_1: type=1400 audit(0.0:50309): avc:
denied { ioctl } for path="socket:[2246822]" dev="sockfs" ino=2246822
ioctlcmd=7704 scontext=u:r:isolated_app:s0:c512,c768
tcontext=u:r:untrusted_app:s0:c512,c768 tclass=unix_stream_socket permissive=0
11-10 21:15:50.156 14094 14094 W Binder_1: type=1400 audit(0.0:50310): avc:
denied { ioctl } for
path=2F646174612F646174612F6F72672E6368726F6D69756D2E6368726F6D652F6170705F6368726F6D652F52454C524F3A6C69626368726F6D655F7075626C69632E736F202864656C6574656429
dev="dm-2" ino=409043 ioctlcmd=7704 scontext=u:r:isolated_app:s0:c512,c768
tcontext=u:object_r:app_data_file:s0:c512,c768 tclass=file permissive=0
11-10 21:15:50.180  3566  3566 I Keyboard.Facilitator: onFinishInput()
11-10 21:15:50.183 14083 14100 I cr_LibraryLoader: Loading chrome_public
11-10 21:15:50.214 14083 14100 I cr_LibraryLoader: Time to load native
libraries: 51 ms (timestamps 6623-6674)
11-10 21:15:50.215 14083 14100 I cr_LibraryLoader: Expected native library
version number "48.0.2561.0", actual native library version number "48.0.2561.0"
11-10 21:15:50.215 14083 14100 I chromium: [INFO:library_loader_hooks.cc(130)]
Chromium logging enabled: level = 0, default verbosity = 0
11-10 21:15:50.231 14051 14068 D ChromeActivityTestCaseBase:
startActivityCompletely <<
11-10 21:15:50.236   493  2915 I OMX-VDEC-1080P: component_init:
OMX.qcom.video.decoder.vp8 : fd=44
11-10 21:15:50.236   493  2915 I OMX-VDEC-1080P: Capabilities: driver_name =
msm_vidc_driver, card = msm_vdec_8974, bus_info = , version = 1, capabilities =
4003000
11-10 21:15:50.242   493  2915 I OMX-VDEC-1080P: omx_vdec::component_init()
success : fd=44
11-10 21:15:50.243   493 24252 I OMX-VDEC-1080P: omx_vdec::component_deinit()
complete
11-10 21:15:50.245   493 24252 I OMX-VDEC-1080P: Exit OMX vdec Destructor: fd=44
11-10 21:15:50.286 14051 14051 E chromium: [ERROR:layer_tree_host_impl.cc(2181)]
Forcing zero-copy tile initialization as worker context is missing
11-10 21:15:50.382 18688 21510 W SearchService: Abort, client detached.
11-10 21:15:50.382 18688 21510 W SearchService: Abort, client detached.
11-10 21:15:51.300 14051 14077 W cr_CrashFileManager:
/data/user/0/org.chromium.chrome/cache/Crash Reports does not exist!
11-10 21:15:51.301 14051 14077 W cr_CrashFileManager:
/data/user/0/org.chromium.chrome/cache/Crash Reports does not exist!
11-10 21:15:51.317 14051 14160 E chromium: [ERROR:socket_posix.cc(121)] bind()
returned an error, errno=98: Address already in use
11-10 21:15:51.317 14051 14160 E chromium:
[ERROR:unix_domain_server_socket_posix.cc(90)] Could not bind unix domain socket
to chrome_devtools_remote (with abstract namespace): Address already in use
11-10 21:15:51.321 14051 14051 I cr_BindingManager: Moderate binding enabled:
maxSize=20 lowReduceRatio=0.250000 highReduceRatio=0.500000
11-10 21:15:51.334 14051 14161 W cr_CrashFileManager:
/data/user/0/org.chromium.chrome/cache/Crash Reports does not exist!
11-10 21:15:51.334 14051 14161 W cr_CrashFileManager:
/data/user/0/org.chromium.chrome/cache/Crash Reports does not exist!
11-10 21:15:51.334 14051 14161 I cr_MinidmpUploadService: Attempting to upload
accumulated crash dumps.
11-10 21:15:51.355 14051 14051 D AndroidRuntime: Shutting down VM
11-10 21:15:51.355   900  3799 I MediaFocusControl:  AudioFocus 
abandonAudioFocus() from
android.media.AudioManager@75f1758org.chromium.content.browser.MediaSession@2...
11-10 21:15:51.357 14051 14051 E AndroidRuntime: FATAL EXCEPTION: main
11-10 21:15:51.357 14051 14051 E AndroidRuntime: Process: org.chromium.chrome,
PID: 14051
11-10 21:15:51.357 14051 14051 E AndroidRuntime: java.lang.RuntimeException:
Unable to start service
org.chromium.chrome.browser.media.ui.MediaNotificationManager$PlaybackListenerService@3979996
with Intent {
cmp=org.chromium.chrome/.browser.media.ui.MediaNotificationManager$PlaybackListenerService
(has extras) }: java.lang.NullPointerException: Attempt to read from field 'int
org.chromium.chrome.browser.media.ui.MediaNotificationInfo.icon' on a null
object reference
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	at
android.app.ActivityThread.handleServiceArgs(ActivityThread.java:3027)
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	at
android.app.ActivityThread.-wrap17(ActivityThread.java)
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	at
android.app.ActivityThread$H.handleMessage(ActivityThread.java:1442)
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	at
android.os.Handler.dispatchMessage(Handler.java:102)
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	at
android.os.Looper.loop(Looper.java:148)
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	at
android.app.ActivityThread.main(ActivityThread.java:5417)
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	at
java.lang.reflect.Method.invoke(Native Method)
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	at
com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:726)
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	at
com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616)
11-10 21:15:51.357 14051 14051 E AndroidRuntime: Caused by:
java.lang.NullPointerException: Attempt to read from field 'int
org.chromium.chrome.browser.media.ui.MediaNotificationInfo.icon' on a null
object reference
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	at
org.chromium.chrome.browser.media.ui.MediaNotificationManager.updateNotification(MediaNotificationManager.java:546)
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	at
org.chromium.chrome.browser.media.ui.MediaNotificationManager.onServiceStarted(MediaNotificationManager.java:378)
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	at
org.chromium.chrome.browser.media.ui.MediaNotificationManager.access$200(MediaNotificationManager.java:40)
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	at
org.chromium.chrome.browser.media.ui.MediaNotificationManager$ListenerService.processIntent(MediaNotificationManager.java:119)
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	at
org.chromium.chrome.browser.media.ui.MediaNotificationManager$ListenerService.onStartCommand(MediaNotificationManager.java:91)
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	at
org.chromium.chrome.browser.media.ui.MediaNotificationManager$PlaybackListenerService.onStartCommand(MediaNotificationManager.java:173)
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	at
android.app.ActivityThread.handleServiceArgs(ActivityThread.java:3010)
11-10 21:15:51.357 14051 14051 E AndroidRuntime: 	... 8 more
11-10 21:15:51.359   900  3295 W ActivityManager: Error in app
org.chromium.chrome running instrumentation
ComponentInfo{org.chromium.chrome.tests/org.chromium.chrome.test.ChromeInstrumentationTestRunner}:
11-10 21:15:51.359   900  3295 W ActivityManager:  
java.lang.NullPointerException
11-10 21:15:51.359   900  3295 W ActivityManager:  
java.lang.NullPointerException: Attempt to read from field 'int
org.chromium.chrome.browser.media.ui.MediaNotificationInfo.icon' on a null
object reference
11-10 21:15:51.363 14043 14043 D AndroidRuntime: Shutting down VM
11-10 21:15:51.606   900  3295 I ActivityManager: Force stopping
org.chromium.chrome appid=10157 user=0: finished inst
11-10 21:15:51.606   900   914 W ActivityManager: Error shutting down
UiAutomationConnection
11-10 21:15:51.606   900  3295 I ActivityManager: Killing
14051:org.chromium.chrome/u0a157 (adj 0): stop org.chromium.chrome
11-10 21:15:51.645   900 10251 D ConnectivityService: ConnectivityService
NetworkRequestInfo binderDied(NetworkRequest [ id=390, legacyType=-1, [
Capabilities: INTERNET&NOT_RESTRICTED&TRUSTED&NOT_VPN] ],
android.os.BinderProxy@541924b)
11-10 21:15:51.645   900  3762 D GraphicsStats: Buffer count: 6
11-10 21:15:51.645   900 10251 I WindowState: WIN DEATH: Window{8805428 u0
SurfaceView}
11-10 21:15:51.646   900   912 D ConnectivityService: ConnectivityService
NetworkRequestInfo binderDied(NetworkRequest [ id=391, legacyType=-1, [
Capabilities: INTERNET&NOT_RESTRICTED&TRUSTED&NOT_VPN] ],
android.os.BinderProxy@7383541)
11-10 21:15:51.646   900  3053 D ConnectivityService: releasing NetworkRequest
NetworkRequest [ id=390, legacyType=-1, [ Capabilities:
INTERNET&NOT_RESTRICTED&TRUSTED&NOT_VPN] ]
11-10 21:15:51.647   900  3053 E ConnectivityService: RemoteException caught
trying to send a callback msg for NetworkRequest [ id=390, legacyType=-1, [
Capabilities: INTERNET&NOT_RESTRICTED&TRUSTED&NOT_VPN] ]
11-10 21:15:51.647   900  3053 D ConnectivityService: releasing NetworkRequest
NetworkRequest [ id=391, legacyType=-1, [ Capabilities:
INTERNET&NOT_RESTRICTED&TRUSTED&NOT_VPN] ]
11-10 21:15:51.649   900  3053 E ConnectivityService: RemoteException caught
trying to send a callback msg for NetworkRequest [ id=391, legacyType=-1, [
Capabilities: INTERNET&NOT_RESTRICTED&TRUSTED&NOT_VPN] ]
11-10 21:15:51.650   900  3011 W InputDispatcher: channel 'f391c67
org.chromium.chrome/org.chromium.chrome.browser.document.DocumentActivity
(server)' ~ Consumer closed input channel or an error occurred.  events=0x9
11-10 21:15:51.650   900  3011 E InputDispatcher: channel 'f391c67
org.chromium.chrome/org.chromium.chrome.browser.document.DocumentActivity
(server)' ~ Channel is unrecoverably broken and will be disposed!
11-10 21:15:51.657   900  3295 W ActivityManager: Force removing
ActivityRecord{7029011 u0 org.chromium.chrome/.browser.document.DocumentActivity
t723}: app died, no saved state
11-10 21:15:51.658   900  3762 I WindowState: WIN DEATH: Window{f391c67 u0
org.chromium.chrome/org.chromium.chrome.browser.document.DocumentActivity}
11-10 21:15:51.658   900  3762 W InputDispatcher: Attempted to unregister
already unregistered input channel 'f391c67
org.chromium.chrome/org.chromium.chrome.browser.document.DocumentActivity
(server)'
11-10 21:15:51.666   900  3295 I ActivityManager: Killing
14083:org.chromium.chrome:sandboxed_process0/u0a157i218 (adj 0): isolated not
needed
11-10 21:15:51.666   900  3295 W libprocessgroup: failed to open
/acct/uid_10157/pid_14083/cgroup.procs: No such file or directory
11-10 21:15:51.671   900  3295 I ActivityManager: Killing
14117:org.chromium.chrome:privileged_process0/u0a157 (adj 0): stop
org.chromium.chrome
11-10 21:15:51.710   900  3774 W art     : Long monitor contention event with
owner method=void
com.android.server.am.ActivityManagerService.crashApplication(com.android.server.am.ProcessRecord,
android.app.ApplicationErrorReport$CrashInfo) from
ActivityManagerService.java:12564 waiters=0 for 348ms
11-10 21:15:51.711   900 10253 W ActivityManager: Spurious death for
ProcessRecord{a2aa14 0:org.chromium.chrome/u0a157}, curProc for 14051: null
11-10 21:15:51.712   900  3213 W ActivityManager: Spurious death for
ProcessRecord{f150827 0:org.chromium.chrome:privileged_process0/u0a157}, curProc
for 14117: null
11-10 21:15:51.754   900  3718 W InputMethodManagerService: Got RemoteException
sending setActive(false) notification to pid 14051 uid 10157
11-10 21:15:51.754  3566  3566 I Keyboard.Facilitator: onFinishInput()
11-10 21:15:51.812 18688 14173 I MicroRecognitionRnrImpl: Starting detection.
11-10 21:15:51.813 18688 14174 I MicrophoneInputStream: mic_starting
com.google.android.apps.gsa.speech.audio.y@fb1849d
11-10 21:15:51.818   493 14176 I AudioFlinger: AudioFlinger's thread 0xeee40000
ready to run
11-10 21:15:51.821   493  2914 I SoundTriggerHwService::Module: void
android::SoundTriggerHwService::Module::onCallbackEvent(const
android::sp<android::SoundTriggerHwService::CallbackEvent>&) mClient == 0
11-10 21:15:51.833 18688 14174 I MicrophoneInputStream: mic_started
com.google.android.apps.gsa.speech.audio.y@fb1849d
11-10 21:15:51.842   493 14176 D audio_hw_primary: select_devices:
out_snd_device(0: none) in_snd_device(61: voice-rec-mic)
11-10 21:15:51.842   493 14176 D msm8974_platform:
platform_send_audio_calibration: sending audio calibration for snd_device(61)
acdb_id(62)
11-10 21:15:51.842   493 14176 I ACDB-LOADER: ACDB AFE returned = -19
11-10 21:15:51.842   493 14176 I soundtrigger:
audio_extn_sound_trigger_update_device_status: device 0x3d of type 1 for Event 1
11-10 21:15:51.842   493 14176 D sound_trigger_platform:
platform_stdev_check_and_update_concurrency: concurrency active 0, tx 1, rx 0,
concurrency session_allowed 0
11-10 21:15:51.842   493 14176 D audio_hw_primary: enable_snd_device:
snd_device(61: voice-rec-mic)
11-10 21:15:51.848   493 14176 D audio_hw_primary: enable_audio_route: apply and
update mixer path: audio-record
11-10 21:15:51.913 18688 18688 I HotwordWorkerImpl: onReady
11-10 21:15:52.267 18701 19069 W OpenGLRenderer: Incorrectly called buildLayer
on View: ShortcutAndWidgetContainer, destroying layer...
11-10 21:15:52.766 14195 14195 D AndroidRuntime: >>>>>> START
com.android.internal.os.RuntimeInit uid 2000 <<<<<<
11-10 21:15:52.771 14195 14195 D AndroidRuntime: CheckJNI is OFF
11-10 21:15:52.801 14195 14195 D ICU     : No timezone override file found:
/data/misc/zoneinfo/current/icu/icu_tzdata.dat
11-10 21:15:52.829 14195 14195 I Radio-JNI: register_android_hardware_Radio DONE
11-10 21:15:52.846 14195 14195 D AndroidRuntime: Calling main entry
com.android.commands.pm.Pm
11-10 21:15:52.851   900  3213 I ActivityManager: Force stopping
org.chromium.chrome appid=10157 user=0: clear data
11-10 21:15:52.863 11843 11893 D Documents: Update found 8 roots in 3ms
11-10 21:15:52.871 10356 14203 D PackageBroadcastService: Received broadcast
action=android.intent.action.PACKAGE_DATA_CLEARED and uri=org.chromium.chrome
11-10 21:15:52.871 10356 14203 D AccountUtils: Clearing selected account for
org.chromium.chrome
11-10 21:15:52.875 10356 14203 I LocationSettingsChecker: Removing dialog
suppression flag for package org.chromium.chrome
11-10 21:15:52.881 10356 10489 I Icing   : doRemovePackageData
org.chromium.chrome
11-10 21:15:52.889 14195 14195 I art     : System.exit called, status: 0
11-10 21:15:52.889 14195 14195 I AndroidRuntime: VM exiting with result code 0.
11-10 21:15:53.253 13516 13517 I chromium:
[1110/211553:INFO:device_controller.cc(123)] Unmapping port 8000
11-10 21:15:53.253 13516 13519 I chromium:
[1110/211553:INFO:device_listener.cc(87)] Received exit notification, stopped
accepting clients.
11-10 21:15:56.411   900  4306 D NetlinkSocketObserver:
NeighborEvent{elapsedMs=531235504, 192.168.159.254, [00005E000164],
RTM_NEWNEIGH, NUD_STALE}
11-10 21:16:00.006   900   900 I ActivityManager: Killing
11436:com.google.android.calendar/u0a40 (adj 15): empty for 1802s
11-10 21:16:00.101 18688 14220 W RequestManagerImpl: No request trace given for
request (type=SCHEDULED_REQUEST_FULL)
11-10 21:16:00.104   900  3762 W ActivityManager: Permission Denial: Accessing
service
ComponentInfo{com.google.android.music/com.google.android.music.dial.DialMediaRouteProviderService}
from pid=18688, uid=10031 that is not exported from uid 10062
11-10 21:16:00.108 10356 10356 D ChimeraCfgMgr: Loading module
com.google.android.gms.cast from APK com.google.android.gms
11-10 21:16:00.120 10356 10356 D ChimeraCfgMgr: Loading module
com.google.android.gms.cast from APK com.google.android.gms
11-10 21:16:00.124 18688 14223 W tvdetect: Error creating socket on interface
11-10 21:16:00.165 10356 14224 D MdnsClient: #acquireLock. Multicast lock not
held. Acquiring
11-10 21:16:00.173 10356 10356 D MdnsClient: Multicast lock held. Releasing
11-10 21:16:00.217 10356 10356 D MdnsClient: #acquireLock. Multicast lock not
held. Acquiring
11-10 21:16:00.296 10322 10322 V GLSActivity: AuthDelegateWrapperCreated with
selected intent: Intent {
cmp=com.google.android.gms/.auth.DefaultAuthDelegateService }
11-10 21:16:00.298 10322 10322 V GLSActivity: AuthDelegateWrapperCreated with
selected intent: Intent {
cmp=com.google.android.gms/.auth.DefaultAuthDelegateService }
11-10 21:16:00.832   900   900 D ZenLog  : intercepted:
0|com.google.android.googlequicksearchbox|65539|null|10031,alarmsOnly
11-10 21:16:00.832   900   900 V NotificationService:
pkg=com.google.android.googlequicksearchbox canInterrupt=false intercept=true
11-10 21:16:00.842 18688 14220 I TrainingQuestionManager:
updateFromServerResponse: no new training mode data and no pending answered
questions to clear
11-10 21:16:00.879 10356 10356 D ChimeraCfgMgr: Loading module
com.google.android.gms.cast from APK com.google.android.gms
11-10 21:16:00.883 10308 10733 V ctxmgr  : [FenceDeDuplicator]adding new fence
key to removals =
com.google.android.googlequicksearchbox:zqzhangandroidtest@gmail.com:com.google.android.apps.gsa.extradex.kato.GmsContextManagerClientHelper:KATO_REQ_SCHED_-1414948162
11-10 21:16:00.883 10308 10733 V ctxmgr  : [FenceDeDuplicator]adding new fence
key to removals =
com.google.android.googlequicksearchbox:zqzhangandroidtest@gmail.com:com.google.android.apps.gsa.extradex.kato.GmsContextManagerClientHelper:KATO_REQ_SCHED_-1795571398
11-10 21:16:00.883 10308 10733 V ctxmgr  : [FenceDeDuplicator]adding new fence
key to removals =
com.google.android.googlequicksearchbox:zqzhangandroidtest@gmail.com:com.google.android.apps.gsa.extradex.kato.GmsContextManagerClientHelper:KATO_REQ_SCHED_1123122228
11-10 21:16:00.883 10308 10733 V ctxmgr  : [FenceDeDuplicator]adding new fence
key to removals =
com.google.android.googlequicksearchbox:zqzhangandroidtest@gmail.com:com.google.android.apps.gsa.extradex.kato.GmsContextManagerClientHelper:KATO_REQ_SCHED_892457041
11-10 21:16:00.883 10308 10733 V ctxmgr  : [FenceDeDuplicator]adding new fence
key to removals =
com.google.android.googlequicksearchbox:zqzhangandroidtest@gmail.com:com.google.android.apps.gsa.extradex.kato.GmsContextManagerClientHelper:dummyFence
11-10 21:16:00.886 10356 14259 D MdnsClient: Multicast lock held. Releasing
11-10 21:16:00.919 10308 10733 W ctxmgr  : [AclContextNameCategories]No app ops
for context = 8
11-10 21:16:00.919 10308 10733 W ctxmgr  : [AclContextNameCategories]No app ops
for context = 8
11-10 21:16:00.921 10308 10733 W ctxmgr  : [AclContextNameCategories]No app ops
for context = 14
11-10 21:16:00.921 10308 10733 W ctxmgr  : [AclContextNameCategories]No app ops
for context = 14
11-10 21:16:00.980   900   910 I art     : Background partial concurrent mark
sweep GC freed 33043(2MB) AllocSpace objects, 5(116KB) LOS objects, 33% free,
31MB/47MB, paused 1.090ms total 110.898ms
11-10 21:16:05.341   900  4306 D NetlinkSocketObserver:
NeighborEvent{elapsedMs=531244434, 192.168.159.254, [00005E000164],
RTM_NEWNEIGH, NUD_PROBE}
11-10 21:16:05.382   900  4306 D NetlinkSocketObserver:
NeighborEvent{elapsedMs=531244475, fe80::200:5eff:fe00:20e, [00005E00020E],
RTM_NEWNEIGH, NUD_REACHABLE}
11-10 21:16:08.242   900  4306 D NetlinkSocketObserver:
NeighborEvent{elapsedMs=531247335, fe80::200:5eff:fe00:20e, [00005E00020E],
RTM_NEWNEIGH, NUD_STALE}
11-10 21:16:09.380 10308 10733 W ctxmgr  : [PowerConnectionState]Got same value
as before for power connection (Plug state: 2 BatteryLevel: 1.0)

Zhiqiang Zhang (Slow)

Hi! I've uploaded a new patch set. Now notification control actions are simulated by sending ...

5 years, 1 month ago (2015-11-11 17:46:42 UTC) #20

Hi! I've uploaded a new patch set. Now notification control actions are
simulated by sending intents. Now the tests seems to be working, and the
previous failing tests have passed.

However a new test case is failing which I suspect to be a synchronization
issue.

https://codereview.chromium.org/1417743005/diff/140001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java
(right):

https://codereview.chromium.org/1417743005/diff/140001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:56:
@DisabledTest
testSessionStateUncontrollable is disabled because it sometimes fails.
Might be a synchronization issue of two subsequent call of
onMediaSessionStateChanged().
The backtrace for diagnosis is as follows:

11-11 16:35:21.323  8393  8393 D AndroidRuntime: Shutting down VM
11-11 16:35:21.325  8393  8393 E AndroidRuntime: FATAL EXCEPTION: main
11-11 16:35:21.325  8393  8393 E AndroidRuntime: Process: org.chromium.chrome,
PID: 8393
11-11 16:35:21.325  8393  8393 E AndroidRuntime: java.lang.RuntimeException:
Unable to start service
org.chromium.chrome.browser.media.ui.MediaNotificationManager$PlaybackListenerService@e01a43b
with Intent {
cmp=org.chromium.chrome/.browser.media.ui.MediaNotificationManager$PlaybackListenerService
(has extras) }: java.lang.NullPointerException: Attempt to invoke virtual method
'boolean
org.chromium.chrome.browser.media.ui.MediaNotificationInfo.supportsSwipeAway()'
on a null object reference
11-11 16:35:21.325  8393  8393 E AndroidRuntime: 	at
android.app.ActivityThread.handleServiceArgs(ActivityThread.java:3027)
11-11 16:35:21.325  8393  8393 E AndroidRuntime: 	at
android.app.ActivityThread.-wrap17(ActivityThread.java)
11-11 16:35:21.325  8393  8393 E AndroidRuntime: 	at
android.app.ActivityThread$H.handleMessage(ActivityThread.java:1442)
11-11 16:35:21.325  8393  8393 E AndroidRuntime: 	at
android.os.Handler.dispatchMessage(Handler.java:102)
11-11 16:35:21.325  8393  8393 E AndroidRuntime: 	at
android.os.Looper.loop(Looper.java:148)
11-11 16:35:21.325  8393  8393 E AndroidRuntime: 	at
android.app.ActivityThread.main(ActivityThread.java:5417)
11-11 16:35:21.325  8393  8393 E AndroidRuntime: 	at
java.lang.reflect.Method.invoke(Native Method)
11-11 16:35:21.325  8393  8393 E AndroidRuntime: 	at
com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:726)
11-11 16:35:21.325  8393  8393 E AndroidRuntime: 	at
com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616)
11-11 16:35:21.325  8393  8393 E AndroidRuntime: Caused by:
java.lang.NullPointerException: Attempt to invoke virtual method 'boolean
org.chromium.chrome.browser.media.ui.MediaNotificationInfo.supportsSwipeAway()'
on a null object reference
11-11 16:35:21.325  8393  8393 E AndroidRuntime: 	at
org.chromium.chrome.browser.media.ui.MediaNotificationManager.updateNotification(MediaNotificationManager.java:555)
11-11 16:35:21.325  8393  8393 E AndroidRuntime: 	at
org.chromium.chrome.browser.media.ui.MediaNotificationManager.onServiceStarted(MediaNotificationManager.java:383)
11-11 16:35:21.325  8393  8393 E AndroidRuntime: 	at
org.chromium.chrome.browser.media.ui.MediaNotificationManager.access$200(MediaNotificationManager.java:40)
11-11 16:35:21.325  8393  8393 E AndroidRuntime: 	at
org.chromium.chrome.browser.media.ui.MediaNotificationManager$ListenerService.processIntent(MediaNotificationManager.java:124)
11-11 16:35:21.325  8393  8393 E AndroidRuntime: 	at
org.chromium.chrome.browser.media.ui.MediaNotificationManager$ListenerService.onStartCommand(MediaNotificationManager.java:95)
11-11 16:35:21.325  8393  8393 E AndroidRuntime: 	at
android.app.ActivityThread.handleServiceArgs(ActivityThread.java:3010)
11-11 16:35:21.325  8393  8393 E AndroidRuntime: 	... 8 more
11-11 16:35:21.327   900 10251 W ActivityManager: Error in app
org.chromium.chrome running instrumentation
ComponentInfo{org.chromium.chrome.tests/org.chromium.chrome.test.ChromeInstrumentationTestRunner}:
11-11 16:35:21.327   900 10251 W ActivityManager:  
java.lang.NullPointerException
11-11 16:35:21.327   900 10251 W ActivityManager:  
java.lang.NullPointerException: Attempt to invoke virtual method 'boolean
org.chromium.chrome.browser.media.ui.MediaNotificationInfo.supportsSwipeAway()'
on a null object reference

whywhat

https://codereview.chromium.org/1417743005/diff/140001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java File chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java (right): https://codereview.chromium.org/1417743005/diff/140001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java#newcode135 chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java:135: if (mNotificationInfoBuilder == null) return; This member is never ...

5 years, 1 month ago (2015-11-19 05:57:53 UTC) #21

https://codereview.chromium.org/1417743005/diff/140001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java
(right):

https://codereview.chromium.org/1417743005/diff/140001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java:135:
if (mNotificationInfoBuilder == null) return;
This member is never set to null, right?
I think the test fails somewhat legitimately - the
mediaSessionStateChanged(false, false) hides the notification which sets the
mMediaNotificationInfo of the manager to null. Then you call onTitleUpdated()
via simulateUpdateTitle() and it tries to update the notification that's hidden.
So it actually starts the service again which is when the crash happens. Since
there're null checks twice on the code path leading to the stack trace, I
suppose that the test destroys the activity that hides the notification and sets
the info to null in another thread or something.

You should run most of these tests with the annotation @UiThreadTest (see
http://developer.android.com/reference/android/test/UiThreadTest.html) or where
you need to use instrumentation, use
ThreadUtils.runOnUiThreadBlockingNoException() to run the non-instrumentation
test code on the UI thread.

But back to the original problem with the change - it seems that as it is now,
it will show the notification again if the page title changes when there's no
playback (but the notification has been shown previously).

https://codereview.chromium.org/1417743005/diff/140001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java:138:
// TODO(zqzhang): the cached builder and the manager builder are actually hte
same
nit: s/hte/the

https://codereview.chromium.org/1417743005/diff/140001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java:139:
// objects. The implementation here modifies the builder and assigns it to
iteslf
nit: s/iteslf/itself

https://codereview.chromium.org/1417743005/diff/140001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java
(right):

https://codereview.chromium.org/1417743005/diff/140001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:49:
assertTrue(mManager.mMediaNotificationInfo.title == "title1");
Shouldn't you use assertEqual() or at least compare strings with equals() rather
than == ?

https://codereview.chromium.org/1417743005/diff/140001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:50:
mHelper.mWebContentsObserver.mediaSessionStateChanged(true, false);
shouldn't it be (true, true) ?

Zhiqiang Zhang (Slow)

https://codereview.chromium.org/1417743005/diff/180001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java File chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java (right): https://codereview.chromium.org/1417743005/diff/180001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java#newcode305 chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java:305: public static void updateNotificationInfoBuilderAndRefresh( use a different name to ...

5 years, 1 month ago (2015-11-19 18:17:57 UTC) #23

Zhiqiang Zhang (Slow)

https://codereview.chromium.org/1417743005/diff/140001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java File chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java (right): https://codereview.chromium.org/1417743005/diff/140001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java#newcode135 chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java:135: if (mNotificationInfoBuilder == null) return; On 2015/11/19 05:57:53, whywhat ...

5 years, 1 month ago (2015-11-19 19:12:42 UTC) #24

https://codereview.chromium.org/1417743005/diff/140001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java
(right):

https://codereview.chromium.org/1417743005/diff/140001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java:135:
if (mNotificationInfoBuilder == null) return;
On 2015/11/19 05:57:53, whywhat wrote:
> This member is never set to null, right?
> I think the test fails somewhat legitimately - the
> mediaSessionStateChanged(false, false) hides the notification which sets the
> mMediaNotificationInfo of the manager to null. Then you call onTitleUpdated()
> via simulateUpdateTitle() and it tries to update the notification that's
hidden.
> So it actually starts the service again which is when the crash happens. Since
> there're null checks twice on the code path leading to the stack trace, I
> suppose that the test destroys the activity that hides the notification and
sets
> the info to null in another thread or something.
> 
> You should run most of these tests with the annotation @UiThreadTest (see
> http://developer.android.com/reference/android/test/UiThreadTest.html) or
where
> you need to use instrumentation, use
> ThreadUtils.runOnUiThreadBlockingNoException() to run the non-instrumentation
> test code on the UI thread.
> 
> But back to the original problem with the change - it seems that as it is now,
> it will show the notification again if the page title changes when there's no
> playback (but the notification has been shown previously).

Yes. I have added a check in the new patch. When the notification is dismissed,
we only update the cached builder, and do not update/show the notification.

https://codereview.chromium.org/1417743005/diff/140001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java
(right):

https://codereview.chromium.org/1417743005/diff/140001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:49:
assertTrue(mManager.mMediaNotificationInfo.title == "title1");
On 2015/11/19 05:57:53, whywhat wrote:
> Shouldn't you use assertEqual() or at least compare strings with equals()
rather
> than == ?

corrected

https://codereview.chromium.org/1417743005/diff/140001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:50:
mHelper.mWebContentsObserver.mediaSessionStateChanged(true, false);
On 2015/11/19 05:57:53, whywhat wrote:
> shouldn't it be (true, true) ?

Yep, corrected.

whywhat

https://codereview.chromium.org/1417743005/diff/180001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java File chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java (right): https://codereview.chromium.org/1417743005/diff/180001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java#newcode311 chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java:311: if (manager.mMediaNotificationInfo == null) { On 2015/11/19 at 18:17:57, ...

5 years, 1 month ago (2015-11-19 19:50:51 UTC) #25

https://codereview.chromium.org/1417743005/diff/180001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java
(right):

https://codereview.chromium.org/1417743005/diff/180001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java:311:
if (manager.mMediaNotificationInfo == null) {
On 2015/11/19 at 18:17:57, Zhiqiang Zhang wrote:
> Now if the notification is dismissed, we will do nothing

I believe this check will not work if another tab is showing the notification
with the same id, no?
Something like you played soundcloud in tab1, then played vimeo in tab2.
soundcloud would stop playback and probably change the title of tab1 - so the
notification manager will update the notification from tab2 with soundcloud's
title?

I think it's cleaner if you do as I suggested and null the builder in
MediaSessionTabHelper; or you could just check the tabId and if it doesn't
match, ignore the call (leave a TODO to refactor later).

https://codereview.chromium.org/1417743005/diff/180001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java
(right):

https://codereview.chromium.org/1417743005/diff/180001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:74:
getInstrumentation().callActivityOnNewIntent(getActivity(), intent);
I believe this can't be called on the UI thread? Does this test actually work?

https://codereview.chromium.org/1417743005/diff/180001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:84:
getInstrumentation().callActivityOnNewIntent(getActivity(), intent);
Ditto

https://codereview.chromium.org/1417743005/diff/180001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:94:
getInstrumentation().callActivityOnNewIntent(getActivity(), intent);
Ditto

Zhiqiang Zhang (Slow)

PTAL https://codereview.chromium.org/1417743005/diff/180001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java File chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java (right): https://codereview.chromium.org/1417743005/diff/180001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java#newcode311 chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java:311: if (manager.mMediaNotificationInfo == null) { On 2015/11/19 19:50:51, ...

5 years, 1 month ago (2015-11-23 20:34:31 UTC) #26

whywhat

lgtm with one comment though (note you'll need owners for Tab.java to submit). Miguel (miguelg@chromium.org) ...

5 years ago (2015-11-24 16:15:45 UTC) #27

Zhiqiang Zhang (Slow)

https://codereview.chromium.org/1417743005/diff/280001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java File chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java (right): https://codereview.chromium.org/1417743005/diff/280001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java#newcode302 chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java:302: * Updates the notification using a new NotificationInfoBuilder, and ...

5 years ago (2015-11-24 20:51:12 UTC) #28

https://codereview.chromium.org/1417743005/diff/280001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java
(right):

https://codereview.chromium.org/1417743005/diff/280001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java:302:
* Updates the notification using a new NotificationInfoBuilder, and refresh the
notification.
On 2015/11/24 16:15:45, whywhat wrote:
> nit: s/refresh/refreshes

Acknowledged.

https://codereview.chromium.org/1417743005/diff/280001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java:303:
* If the notification is hidden, no operations will be done.
On 2015/11/24 16:15:45, whywhat wrote:
> nit: replace with "If the notification for the same tab is hidden" since you
> check the tab id too?

Acknowledged.

https://codereview.chromium.org/1417743005/diff/280001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java:316:
// The notification is for another tab. Don't update
On 2015/11/24 16:15:45, whywhat wrote:
> Note: I still think this logic would be better in MediaSessionTabHelper to
avoid
> these checks but I don't feel strongly about it at this point.

I see no method available to get the tabId from the manager. Shall I add a
method for this?

https://codereview.chromium.org/1417743005/diff/280001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java:317:
if (manager.mMediaNotificationInfo.tabId != tabId) {
On 2015/11/24 16:15:45, whywhat wrote:
> nit: a one liner
> 
> (means you could and should simply write:
> 
> if (manager.mMediaNotificationInfo.tabId != tabId) return;
> 
> )

Acknowledged.

https://codereview.chromium.org/1417743005/diff/280001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java:320:
manager.mNotificationInfoBuilder = notificationInfoBuilder;
On 2015/11/24 16:15:45, whywhat wrote:
> nit: use some blank lines, e.g. after the if with the early return

Acknowledged.

https://codereview.chromium.org/1417743005/diff/280001/chrome/android/java/sr...
File chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java (right):

https://codereview.chromium.org/1417743005/diff/280001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:357: protected
MediaSessionTabHelper mMediaSessionTabHelper;
On 2015/11/24 16:15:45, whywhat wrote:
> Having this just for the test seems wrong to me.

Yes, I know. Let me see if calling
tab.getWebContents().onMediaSessionStateChanged() works.

https://codereview.chromium.org/1417743005/diff/280001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/tab/TabTestUtils.java
(right):

https://codereview.chromium.org/1417743005/diff/280001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/tab/TabTestUtils.java:72:
public static MediaSessionTabHelper getMediaSessionTabHelper(Tab tab) {
On 2015/11/24 16:15:45, whywhat wrote:
> Maybe we should rather have onMediaSessionStatusChanged on
> WebContents/WebContentsImpl and call
> tab.getWebContents().onMediaSessionStatusChanged() in the tests?

Yeah. Actually I tried that. But it seems that the
TabObserver.onMediaSessionStateChanged() was not called when I call the
tab.getWebContents() version.

Let me do a double check.

mlamouri (slow - plz ping)

BTW, Zhiqiang, it's expected that you send review comment replies when you have uploaded a ...

5 years ago (2015-11-26 14:17:02 UTC) #29

mlamouri (slow - plz ping)

https://codereview.chromium.org/1417743005/diff/340001/content/public/android/java/src/org/chromium/content_public/browser/WebContents.java File content/public/android/java/src/org/chromium/content_public/browser/WebContents.java (right): https://codereview.chromium.org/1417743005/diff/340001/content/public/android/java/src/org/chromium/content_public/browser/WebContents.java#newcode315 content/public/android/java/src/org/chromium/content_public/browser/WebContents.java:315: void mediaSessionStateChanged(boolean isControllable, boolean isSuspend); Instead of doing that, ...

5 years ago (2015-11-27 13:32:54 UTC) #32

Zhiqiang Zhang (Slow)

PTAL, with the following replies. https://codereview.chromium.org/1417743005/diff/340001/content/public/android/java/src/org/chromium/content_public/browser/WebContents.java File content/public/android/java/src/org/chromium/content_public/browser/WebContents.java (right): https://codereview.chromium.org/1417743005/diff/340001/content/public/android/java/src/org/chromium/content_public/browser/WebContents.java#newcode315 content/public/android/java/src/org/chromium/content_public/browser/WebContents.java:315: void mediaSessionStateChanged(boolean isControllable, boolean ...

5 years ago (2015-11-28 22:06:02 UTC) #34

mlamouri (slow - plz ping)

mlamouri@chromium.org changed reviewers: + tedchoc@chromium.org

5 years ago (2015-11-30 11:33:17 UTC) #35

mlamouri (slow - plz ping)

+tedchoc@ who OWNS the file that Anton and I do not own. So I think ...

5 years ago (2015-11-30 11:33:18 UTC) #36

Zhiqiang Zhang (Slow)

On 2015/11/30 11:33:18, Mounir Lamouri wrote: > +tedchoc@ who OWNS the file that Anton and ...

5 years ago (2015-11-30 13:50:58 UTC) #37

Ted C

https://codereview.chromium.org/1417743005/diff/380001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java File chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java (right): https://codereview.chromium.org/1417743005/diff/380001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java#newcode77 chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java:77: public void mediaSessionStateChanged(boolean isControllable, boolean isPaused) { It would ...

5 years ago (2015-11-30 19:17:04 UTC) #38

https://codereview.chromium.org/1417743005/diff/380001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java
(right):

https://codereview.chromium.org/1417743005/diff/380001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java:77:
public void mediaSessionStateChanged(boolean isControllable, boolean isPaused) {
It would be nice if you didn't have to worry about observers at all.  Ideally, I
would just pull the contents of this function out into a protected method that
would be called by this observer and by tests.

The problem is then that the test would need to get access to this tab helper,
which we don't really have a good story for.

Tab could store a reference to this and have a getter like we do for
AppBannerManager (i.e. getAppBannerManagerForTesting).

But I think we are getting definitely to the point where we need a concept of a
true tab helper.  Maybe tab_android should extend SupportsUserData and we could
make these hang off of that.  Then it would be very easy to have a createForTab
and getForTab on each helper to make testing easier.  Granted, that is
definitely outside of the scope of this particular change.

https://codereview.chromium.org/1417743005/diff/380001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/tab/TabTestUtils.java
(right):

https://codereview.chromium.org/1417743005/diff/380001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/tab/TabTestUtils.java:68:
tab.updateTitle(title);
Hmm...I'd honestly be tempted to just update the title of the page.

Like:

JavaScriptUtils.executeJavaScriptAndWaitForResult(
        tab.getWebContents(),
        "document.title = '" + title "';");

Or really just generalize this:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/android/jav...

And make it possible to do from other places

mlamouri (slow - plz ping)

https://codereview.chromium.org/1417743005/diff/380001/chrome/android/javatests/src/org/chromium/chrome/browser/tab/TabTestUtils.java File chrome/android/javatests/src/org/chromium/chrome/browser/tab/TabTestUtils.java (right): https://codereview.chromium.org/1417743005/diff/380001/chrome/android/javatests/src/org/chromium/chrome/browser/tab/TabTestUtils.java#newcode68 chrome/android/javatests/src/org/chromium/chrome/browser/tab/TabTestUtils.java:68: tab.updateTitle(title); On 2015/11/30 at 19:17:04, Ted C wrote: > ...

5 years ago (2015-12-01 18:32:12 UTC) #39

mlamouri (slow - plz ping)

https://codereview.chromium.org/1417743005/diff/380001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java File chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java (right): https://codereview.chromium.org/1417743005/diff/380001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java#newcode77 chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java:77: public void mediaSessionStateChanged(boolean isControllable, boolean isPaused) { On 2015/11/30 ...

5 years ago (2015-12-01 18:34:08 UTC) #40

Ted C

https://codereview.chromium.org/1417743005/diff/380001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java File chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java (right): https://codereview.chromium.org/1417743005/diff/380001/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java#newcode77 chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java:77: public void mediaSessionStateChanged(boolean isControllable, boolean isPaused) { On 2015/12/01 ...

5 years ago (2015-12-01 19:40:24 UTC) #41

https://codereview.chromium.org/1417743005/diff/380001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java
(right):

https://codereview.chromium.org/1417743005/diff/380001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java:77:
public void mediaSessionStateChanged(boolean isControllable, boolean isPaused) {
On 2015/12/01 18:34:08, Mounir Lamouri wrote:
> On 2015/11/30 at 19:17:04, Ted C wrote:
> > It would be nice if you didn't have to worry about observers at all. 
Ideally,
> I would just pull the contents of this function out into a protected method
that
> would be called by this observer and by tests.
> > 
> > The problem is then that the test would need to get access to this tab
helper,
> which we don't really have a good story for.
> > 
> > Tab could store a reference to this and have a getter like we do for
> AppBannerManager (i.e. getAppBannerManagerForTesting).
> > 
> > But I think we are getting definitely to the point where we need a concept
of
> a true tab helper.  Maybe tab_android should extend SupportsUserData and we
> could make these hang off of that.  Then it would be very easy to have a
> createForTab and getForTab on each helper to make testing easier.  Granted,
that
> is definitely outside of the scope of this particular change.
> 
> So, that should have moved to TabContentsObserver.

I don't know what that is.

> The idea was to follow the
> pattern used by splashscreen tests. It's a pattern we discussed with newt@ and
> dfalcantara@: simulate things from TabTestUtils (which was created recently
for
> that purpose). I think it might be good to keep this pattern for now at least
> for consistency?

The TabTestUtils simply sends observer updates.  You are deviating from that
path
in this change by exposing updateTitle and mucking around with the internal tab
state.

So while keeping to a pattern is nice, this isn't doing that, so I'm not
convinced.

And if you are trying to unit test this entity, then I argue my approach is
better for both.

Have two protected methods on this class.

protected void mediaSessionStateChanged(bool, bool)
protected void onTitleUpdated(String)

Then create a new one of these in your tests class and poke at it
directly ignoring the fact that you've already created one for the
tab by doing the activity creation.

Then just have the observer registered call these two methods and you
avoid anything else.

Otherwise, we should figure out a way to make a true testable Tab that
has no native and can manipulated in tests more easily.

Zhiqiang Zhang (Slow)

PTAL. I finally decided to use JavaScriptUtils to simulate title updates. Please see my replies ...

5 years ago (2015-12-08 19:58:44 UTC) #42

Ted C

https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java File chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java (right): https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java#newcode27 chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:27: Tab mTab; private for both of these? https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java#newcode46 chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:46: ...

5 years ago (2015-12-10 21:05:02 UTC) #44

https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java
(right):

https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:27:
Tab mTab;
private for both of these?

https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:46:
MediaNotificationManager.getNotificationInfoForTesting(NOTIFICATION_ID).title);
I think each time you fetch the title, you probably should be doing it on the UI
thread right?

https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:102:
private static class SimulateSessionStateChangedTask implements Runnable {
This definition seems a bit verbose as well.  I would just inline it in the
simulate method below:

private void simulateMedia<...>(final Tab tab, final boolean, final boolean) {
    ThreadUtils.runOnUiThreadBlocking(new Runnable() {
        @Override
        public void run() {
           
tab.getWebContents().simulateMediaSessionStateChanged(mIsControllable,
mIsSuspended);
        }
    });
}

https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:115:
WebContents mWebContents;
in java, we put these at the top of the definition.

https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:144:
getInstrumentation().runOnMainSync(
We more commonly use ThreadUtils.runOnUiThreadBlocking than the instrumentation
approach.

https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:162:
CreateTabTask task = new CreateTabTask(url);
any reason you don't just use:

ChromeActivityTestCaseBase#loadUrlInNewTab?  We could change that to return the
Tab to make the API easier.

The problem with creating a tab yourself is that the renderer priority will be
lower and you will more likely get killed.

https://codereview.chromium.org/1417743005/diff/490001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java
(right):

https://codereview.chromium.org/1417743005/diff/490001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java:379:
public void simulateMediaSessionStateChanged(boolean isControllable, boolean
isSuspended) {
I'd be tempted to just add getObserversForTesting() to avoid polluting this file
with a bunch of these simulate methods.

You'd have to expose them from the proxy as well, but that doesn't seem too bad
either.

Zhiqiang Zhang (Slow)

PTAL, with the following replies. https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java File chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java (right): https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java#newcode27 chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:27: Tab mTab; On 2015/12/10 ...

5 years ago (2015-12-11 22:19:20 UTC) #45

PTAL, with the following replies.

https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java
(right):

https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:27:
Tab mTab;
On 2015/12/10 21:05:02, Ted C wrote:
> private for both of these?

Done.

https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:46:
MediaNotificationManager.getNotificationInfoForTesting(NOTIFICATION_ID).title);
On 2015/12/10 21:05:02, Ted C wrote:
> I think each time you fetch the title, you probably should be doing it on the
UI
> thread right?

Done.

https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:102:
private static class SimulateSessionStateChangedTask implements Runnable {
On 2015/12/10 21:05:02, Ted C wrote:
> This definition seems a bit verbose as well.  I would just inline it in the
> simulate method below:
> 
> private void simulateMedia<...>(final Tab tab, final boolean, final boolean) {
>     ThreadUtils.runOnUiThreadBlocking(new Runnable() {
>         @Override
>         public void run() {
>            
> tab.getWebContents().simulateMediaSessionStateChanged(mIsControllable,
> mIsSuspended);
>         }
>     });
> }

Done.

https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:115:
WebContents mWebContents;
On 2015/12/10 21:05:02, Ted C wrote:
> in java, we put these at the top of the definition.

No need to change. I'm using inner class now.

https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:144:
getInstrumentation().runOnMainSync(
On 2015/12/10 21:05:02, Ted C wrote:
> We more commonly use ThreadUtils.runOnUiThreadBlocking than the
instrumentation
> approach.

Done.

https://codereview.chromium.org/1417743005/diff/490001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:162:
CreateTabTask task = new CreateTabTask(url);
On 2015/12/10 21:05:02, Ted C wrote:
> any reason you don't just use:
> 
> ChromeActivityTestCaseBase#loadUrlInNewTab?  We could change that to return
the
> Tab to make the API easier.
> 
> The problem with creating a tab yourself is that the renderer priority will be
> lower and you will more likely get killed.

Done.

https://codereview.chromium.org/1417743005/diff/490001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java
(right):

https://codereview.chromium.org/1417743005/diff/490001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java:379:
public void simulateMediaSessionStateChanged(boolean isControllable, boolean
isSuspended) {
On 2015/12/10 21:05:02, Ted C wrote:
> I'd be tempted to just add getObserversForTesting() to avoid polluting this
file
> with a bunch of these simulate methods.
> 
> You'd have to expose them from the proxy as well, but that doesn't seem too
bad
> either.

Done.

Ted C

lgtm w/ a couple little comments https://codereview.chromium.org/1417743005/diff/530001/chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java File chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java (right): https://codereview.chromium.org/1417743005/diff/530001/chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java#newcode72 chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java:72: public void testMultipleTabs() ...

5 years ago (2015-12-12 00:57:13 UTC) #46

Zhiqiang Zhang (Slow)

The patchset sent to the CQ was uploaded after l-g-t-m from avayvod@chromium.org, tedchoc@chromium.org Link to ...

5 years ago (2015-12-12 19:49:25 UTC) #48

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1417743005/570001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1417743005/570001

5 years ago (2015-12-12 19:49:35 UTC) #49

commit-bot: I haz the power

Description was changed from ========== Update media notification when page title changes This CL makes ...

5 years ago (2015-12-12 19:54:51 UTC) #51

commit-bot: I haz the power

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

5 years ago (2015-12-12 19:54:55 UTC) #52

tommi (sloooow) - chröme

A revert of this CL (patchset #25 id:570001) has been created in https://codereview.chromium.org/1526433002/ by tommi@chromium.org. ...

5 years ago (2015-12-12 21:30:50 UTC) #53

Message was sent while issue was closed.

A revert of this CL (patchset #25 id:570001) has been created in
https://codereview.chromium.org/1526433002/ by tommi@chromium.org.

The reason for reverting is: Broke compile step on Android:

https://build.chromium.org/p/chromium.linux/builders/Android%20Builder/builds...

Note: org.chromium.content.browser.test.NestedSystemMessageHandler accesses a
declared method 'next()' dynamically
      Maybe this is library method 'android.database.CursorJoiner {
android.database.CursorJoiner$Result next(); }'
      Maybe this is library method 'android.database.CursorJoiner {
java.lang.Object next(); }'
      Maybe this is library method
'android.support.v4.media.session.IMediaSession { void next(); }'
      Maybe this is library method
'android.support.v4.media.session.IMediaSession$Stub$Proxy { void next(); }'
      Maybe this is library method
'android.support.v4.media.session.MediaSessionCompat$MediaSessionImplBase$MediaSessionStub
{ void next(); }'
      Maybe this is library method
'android.support.v4.util.MapCollections$ArrayIterator { java.lang.Object next();
}'
      Maybe this is library method
'android.support.v4.util.MapCollections$MapIterator { java.util.Map$Entry
next(); }'
      Maybe this is library method
'android.support.v4.util.MapCollections$MapIterator { java.lang.Object next();
}'
      Maybe this is library method
'android.support.v7.util.MessageThreadUtil$MessageQueue {
android.support.v7.util.MessageThreadUtil$SyncQueueItem next(); }'
      Maybe this is library method 'android.text.TextUtils$SimpleStringSplitter
{ java.lang.String next(); }'
      Maybe this is library method 'android.text.TextUtils$SimpleStringSplitter
{ java.lang.Object next(); }'
      Maybe this is library method
'com.google.android.gms.measurement.internal.a { java.lang.Object next(); }'
      Maybe this is library method 'java.sql.ResultSet { boolean next(); }'
      Maybe this is library method 'java.text.BreakIterator { int next(); }'
      Maybe this is library method 'java.text.CharacterIterator { char next();
}'
      Maybe this is library method 'java.text.CollationElementIterator { int
next(); }'
      Maybe this is library method 'java.text.StringCharacterIterator { char
next(); }'
      Maybe this is library method 'java.util.Iterator { java.lang.Object
next(); }'
      Maybe this is library method 'java.util.ListIterator { java.lang.Object
next(); }'
      Maybe this is library method 'java.util.Scanner { java.lang.String next();
}'
      Maybe this is library method 'java.util.Scanner { java.lang.Object next();
}'
      Maybe this is library method
'org.apache.http.message.BasicHeaderElementIterator { java.lang.Object next();
}'
      Maybe this is library method 'org.apache.http.message.BasicHeaderIterator
{ java.lang.Object next(); }'
      Maybe this is library method
'org.apache.http.message.BasicListHeaderIterator { java.lang.Object next(); }'
      Maybe this is library method 'org.apache.http.message.BasicTokenIterator {
java.lang.Object next(); }'
      Maybe this is library method
'org.chromium.base.ObserverList$ObserverListIterator { java.lang.Object next();
}'
      Maybe this is library method
'org.chromium.chrome.browser.partnerbookmarks.PartnerBookmarksProviderIterator {
org.chromium.chrome.browser.partnerbookmarks.PartnerBookmarksReader$Bookmark
next(); }'
      Maybe this is library method
'org.chromium.chrome.browser.partnerbookmarks.PartnerBookmarksProviderIterator {
java.lang.Object next(); }'
      Maybe this is library method
'org.chromium.chrome.browser.preferences.website.WebsitePermissionsFetcher$TaskQueue
{ void next(); }'
      Maybe this is library method 'org.json.JSONTokener { char next(); }'
      Maybe this is library method 'org.xmlpull.v1.XmlPullParser { int next();
}'
Note: the configuration refers to the unknown class
'com.google.ipc.invalidation.external.client2.contrib.AndroidListener$AlarmReceiver'
      Maybe you meant the fully qualified name
'com.google.ipc.invalidation.external.client.contrib.AndroidListener$AlarmReceiver'?
Note: there were 1 references to unknown classes.
      You should check your configuration for typos.
Note: there were 1 accesses to class members by means of introspection.
      You should consider explicitly keeping the mentioned class members
      (using '-keep' or '-keepclassmembers').
Note: there were 7 duplicate class definitions.
Warning: org.chromium.chrome.browser.sync.OpenTabsTest: can't find referenced
method 'void loadUrlInNewTab(java.lang.String)' in class
org.chromium.chrome.browser.sync.OpenTabsTest
Warning: there were 1 unresolved references to program class members.
         Your input classes appear to be inconsistent.
         You may need to recompile them and try again.
         Alternatively, you may have to specify the option 
         '-dontskipnonpubliclibraryclassmembers'.
Error: Please correct the above warnings first..

Zhiqiang Zhang (Slow)

Description was changed from ========== Update media notification when page title changes This CL makes ...

5 years ago (2015-12-14 12:24:36 UTC) #54

Zhiqiang Zhang (Slow)

The patchset sent to the CQ was uploaded after l-g-t-m from tedchoc@chromium.org, avayvod@chromium.org Link to ...

5 years ago (2015-12-14 15:58:55 UTC) #56

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1417743005/590001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1417743005/590001

5 years ago (2015-12-14 16:00:09 UTC) #57

commit-bot: I haz the power

Description was changed from ========== Update media notification when page title changes This CL makes ...

5 years ago (2015-12-14 17:58:54 UTC) #58

commit-bot: I haz the power

Description was changed from ========== Update media notification when page title changes This CL makes ...

5 years ago (2015-12-14 18:00:35 UTC) #60

commit-bot: I haz the power

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

5 years ago (2015-12-14 18:00:36 UTC) #61

Michael Courage

5 years ago (2015-12-15 01:32:11 UTC) #62

Message was sent while issue was closed.

A revert of this CL (patchset #26 id:590001) has been created in
https://codereview.chromium.org/1528563003/ by courage@chromium.org.

The reason for reverting is: It looks to me like this change is causing failures
in the ChromeSyncShellTest, e.g.:
https://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg...
https://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg...

Not sure why this builds successfully, but the signature change to
ChromeActivityTestCaseBase.loadUrlInNewTab causes OpenTabsTest to fail at
runtime.

C   97.565s Main  [FAIL]
org.chromium.chrome.browser.sync.OpenTabsTest#testUploadAndCloseOpenTab:
C   97.565s Main  java.lang.NoSuchMethodError:
org.chromium.chrome.browser.sync.OpenTabsTest.loadUrlInNewTab
C   97.565s Main  	at
org.chromium.chrome.browser.sync.OpenTabsTest.testUploadAndCloseOpenTab(OpenTabsTest.java:108)
C   97.566s Main  	at java.lang.reflect.Method.invokeNative(Native Method)
C   97.566s Main  	at
android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214)
C   97.566s Main  	at
android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199)
.

Issue 1417743005: Update media notification when page title changes (Closed)

Description

Patch Set 1 #

Patch Set 2 : Re-implementation to avoid involving tabs into the manager #

Patch Set 3 : added TODO comments for future better implementation #

Patch Set 4 : added instrumentation tests (THE LAST TEST WILL FAIL) #

Patch Set 5 : merged with master branch (STILL ONE TEST FAILING) #

Patch Set 6 : simulate notification controls by sending intents, instead of calling manager.onPlay/Pause/Stop dir… #

Patch Set 7 : merged with TabTestUtils, disabled testSessionStateUncontrollable #

Patch Set 8 : running tests on the ui thread. notifications won't update when hidding #

Patch Set 9 : fixed tests, sending intents to services #

Patch Set 10 : fixed tests, added a test for multiple tabs #

Patch Set 11 : #

Patch Set 12 : #

Patch Set 13 : #

Patch Set 14 : #

Patch Set 15 : #

Patch Set 16 : now the tests simulates mediaSessionStateChange through WebContents #

Patch Set 17 : using JavaScriptUtils to update title, removed tests that simulate UI controls #

Patch Set 18 : remove simulateTitleUpdated from TabTestUtils #

Patch Set 19 : small fixes suggested by FindBugs #

Patch Set 20 : fix failed test #

Patch Set 21 : rebasing and cleaning up #

Patch Set 22 : exposing observers from WebContents(ObserverProxy), creating new Tab using loadUrlInNewTab, and som… #

Patch Set 23 : remove unread mContext field #

Patch Set 24 : small fixes #

Patch Set 25 : fix import #

Patch Set 26 : rebase before recommiting #

Messages