Assigning to author and cc'ing owners.

Actually, assigning to fbeaufort@ as it may be a regression from one of their recent CL.

Thanks Mounir!
Here's the crash:

  [13546:13546:1004/131702.568408:FATAL:picture_in_picture_window_controller_impl.cc(160)] Check failed: media_player_id_.has_value().
  #0 0x000004faffdf base::debug::StackTrace::StackTrace()
  #1 0x000004f15ebb logging::LogMessage::~LogMessage()
  #2 0x0000037fab41 content::PictureInPictureWindowControllerImpl::UpdatePlaybackState()
  #3 0x0000037b3afe content::MediaWebContentsObserver::OnMediaPaused()
  #4 0x0000037b3919 _ZN3IPC8MessageTI45MediaPlayerDelegateHostMsg_OnMediaPaused_MetaNSt3__15tupleIJibEEEvE8DispatchIN7content24MediaWebContentsObserverES8_NS7_15RenderFrameHostEMS8_FvPS9_ibEEEbPKNS_7MessageEPT_PT0_PT1_T2_
  #5 0x0000037b35f6 content::MediaWebContentsObserver::OnMessageReceived()
  #6 0x0000039e66ff content::WebContentsImpl::OnMessageReceived()
  #7 0x000003697474 content::RenderFrameHostImpl::OnMessageReceived()
  #8 0x000003893bfb content::RenderProcessHostImpl::OnMessageReceived()
  #9 0x000006096001 IPC::ChannelProxy::Context::OnDispatchMessage()
  #10 0x000001dbea6b _ZN4base8internal7InvokerINS0_9BindStateIMN13safe_browsing12_GLOBAL__N_125FakeSafeBrowsingUIManagerEFvRKNSt3__112basic_stringIcNS6_11char_traitsIcEENS6_9allocatorIcEEEEEJ13scoped_refptrIS5_ESC_EEEFvvEE7RunOnceEPNS0_13BindStateBaseE
  #11 0x000004fd3735 base::debug::TaskAnnotator::RunTask()
  #12 0x000004f1e3fe base::MessageLoop::RunTask()
  #13 0x000004f1e783 base::MessageLoop::DoWork()
  #14 0x000004fce299 base::MessagePumpLibevent::Run()
  #15 0x000004f1df94 base::MessageLoop::Run()
  #16 0x000004f46829 base::RunLoop::Run()
  #17 0x0000055fe8f3 content::MessageLoopRunner::Run()
  #18 0x0000055fd2d9 content::TestNavigationObserver::Wait()
  #19 0x0000050324b1 ui_test_utils::NavigateToURLWithDispositionBlockUntilNavigationsComplete()
  #20 0x000000eef104 PictureInPictureWindowControllerBrowserTest_TabIconUpdated_Test::RunTestOnMainThread()

I wonder, according to the code below, if media_player_id_ is sometimes reset before OnMediaPaused() is reached because IPC.


void PictureInPictureWindowControllerImpl::OnLeavingPictureInPicture(
    bool should_pause_video,
    bool should_reset_pip_player) {
  if (IsPlayerActive() && should_pause_video) {
    // Pause the current video so there is only one video playing at a time.
    media_player_id_->render_frame_host->Send(new MediaPlayerDelegateMsg_Pause(
        media_player_id_->render_frame_host->GetRoutingID(),
        media_player_id_->delegate_id));
  }

  if (media_player_id_.has_value()) {
    media_player_id_->render_frame_host->Send(
        new MediaPlayerDelegateMsg_EndPictureInPictureMode(
            media_player_id_->render_frame_host->GetRoutingID(),
            media_player_id_->delegate_id));
    if (should_reset_pip_player)
      media_web_contents_observer_->ResetPictureInPictureVideoMediaPlayerId();
  }
}

Could https://chromium-review.googlesource.com/c/chromium/src/+/1244361 fix this situation?
UpdatePlaybackState would only be called if pip_player_ fully matches player_id.
WDYT?

This is being worked on -> removing from sherrifs' queue. Thanks, fbeaufort@!

PictureInPictureWindowControllerBrowserTest.TabIconUpdated is flaky.

Findit has detected 4 new flake occurrences of this test. List
of all flake occurrences can be found at:
https://findit-for-me.appspot.com/flake/occurrences?key=ag9zfmZpbmRpdC1mb3ItbWVyXAsSBUZsYWtlIlFjaHJvbWl1bUBicm93c2VyX3Rlc3RzQFBpY3R1cmVJblBpY3R1cmVXaW5kb3dDb250cm9sbGVyQnJvd3NlclRlc3QuVGFiSWNvblVwZGF0ZWQM.

Since this test is still flaky, this issue has been moved back onto the Sheriff
Bug Queue if it's not already there.

If the result above is wrong, please file a bug using this link:
https://bugs.chromium.org/p/chromium/issues/entry?status=Unconfirmed&labels=Pri-1,Test-Findit-Wrong&components=Tools%3ETest%3EFindit%3EFlakiness&summary=%5BFindit%5D%20Flake%20Detection%20-%20Wrong%20result%20for%20PictureInPictureWindowControllerBrowserTest.TabIconUpdated&comment=Link%20to%20flake%20occurrences%3A%20https://findit-for-me.appspot.com/flake/occurrences?key=ag9zfmZpbmRpdC1mb3ItbWVyXAsSBUZsYWtlIlFjaHJvbWl1bUBicm93c2VyX3Rlc3RzQFBpY3R1cmVJblBpY3R1cmVXaW5kb3dDb250cm9sbGVyQnJvd3NlclRlc3QuVGFiSWNvblVwZGF0ZWQM

Automatically posted by the findit-for-me app (https://goo.gl/Ot9f7N).

PictureInPictureWindowControllerBrowserTest.TabIconUpdated is flaky.

Findit has detected 3 new flake occurrences of this test. List
of all flake occurrences can be found at:
https://findit-for-me.appspot.com/flake/occurrences?key=ag9zfmZpbmRpdC1mb3ItbWVyXAsSBUZsYWtlIlFjaHJvbWl1bUBicm93c2VyX3Rlc3RzQFBpY3R1cmVJblBpY3R1cmVXaW5kb3dDb250cm9sbGVyQnJvd3NlclRlc3QuVGFiSWNvblVwZGF0ZWQM.

Since this test is still flaky, this issue has been moved back onto the Sheriff
Bug Queue if it's not already there.

If the result above is wrong, please file a bug using this link:
https://bugs.chromium.org/p/chromium/issues/entry?status=Unconfirmed&labels=Pri-1,Test-Findit-Wrong&components=Tools%3ETest%3EFindit%3EFlakiness&summary=%5BFindit%5D%20Flake%20Detection%20-%20Wrong%20result%20for%20PictureInPictureWindowControllerBrowserTest.TabIconUpdated&comment=Link%20to%20flake%20occurrences%3A%20https://findit-for-me.appspot.com/flake/occurrences?key=ag9zfmZpbmRpdC1mb3ItbWVyXAsSBUZsYWtlIlFjaHJvbWl1bUBicm93c2VyX3Rlc3RzQFBpY3R1cmVJblBpY3R1cmVXaW5kb3dDb250cm9sbGVyQnJvd3NlclRlc3QuVGFiSWNvblVwZGF0ZWQM

Automatically posted by the findit-for-me app (https://goo.gl/Ot9f7N).

No progress since last week; disabling the test.

Disable at https://chromium-review.googlesource.com/c/chromium/src/+/1269339 - removing Sheriff label.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7bf7db361be78fd0b46b93f252531d720bdf883e

commit 7bf7db361be78fd0b46b93f252531d720bdf883e
Author: Joshua Pawlicki <waffles@chromium.org>
Date: Mon Oct 08 21:00:16 2018

Disable TabIconUpdated test on ChromeOS (is flaky).

TBR=alph@chromium.org

Bug:  892310 
Change-Id: I6cefa7018520128cdf81d79a1ee467d5ef5859fe
Reviewed-on: https://chromium-review.googlesource.com/c/1269339
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Joshua Pawlicki <waffles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597679}
[modify] https://crrev.com/7bf7db361be78fd0b46b93f252531d720bdf883e/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc

Now that https://chromium-review.googlesource.com/c/chromium/src/+/1275665 has landed, I'd like to revert the previous CL and mark test as non flaky again. See https://chromium-review.googlesource.com/c/chromium/src/+/1275665

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/490d3d816fe11e9b11899d3340f074aa2d11253e

commit 490d3d816fe11e9b11899d3340f074aa2d11253e
Author: François Beaufort <beaufort.francois@gmail.com>
Date: Thu Oct 11 07:32:25 2018

Revert "Disable TabIconUpdated test on ChromeOS (is flaky)."

This reverts commit 7bf7db361be78fd0b46b93f252531d720bdf883e.

Reason for revert: https://chromium-review.googlesource.com/c/1273515 has landed.

Original change's description:
> Disable TabIconUpdated test on ChromeOS (is flaky).
> 
> TBR=alph@chromium.org
> 
> Bug:  892310 
> Change-Id: I6cefa7018520128cdf81d79a1ee467d5ef5859fe
> Reviewed-on: https://chromium-review.googlesource.com/c/1269339
> Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
> Commit-Queue: Joshua Pawlicki <waffles@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#597679}

TBR=alph@chromium.org,waffles@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  892310 
Change-Id: Ibb6d9ffdfe2372e165eb0abb7db9a2f4a0d57a2f
Reviewed-on: https://chromium-review.googlesource.com/c/1275665
Reviewed-by: François Beaufort <beaufort.francois@gmail.com>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: François Beaufort <beaufort.francois@gmail.com>
Cr-Commit-Position: refs/heads/master@{#598694}
[modify] https://crrev.com/490d3d816fe11e9b11899d3340f074aa2d11253e/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc

I'm gonna monitor https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-chromeos-rel in case flakyness comes back