Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

c++ / mojo changes for 'external window mode' #1

Merged
merged 1 commit into from
Mar 22, 2017

Conversation

tonikitoo
Copy link
Member

This CL is the first step towards having 'external window mode' support
in Mus. The existing MusDemo is adapted to test the functionality,
and its respective unit test (mus_demo_unittests) is changed to
exercise multiple external windows creation in a follow up CL [2].

[2] https://codereview.chromium.org/2715533005/

New code flow:

  • MusDemoExternal creates and holds a WindowTreeClient instance.

Note: In Chrome, it also happens in
c/b/ui/views/chrome_browser_main_extra_parts_views.cc,
::ServiceManagerConnectionStarted, when MusClient is created.

  • MusDemoExternal calls WindowTreeClient::ConnectViaWindowTreeHostFactory.
    This "enters" WindowTreeClient in 'external window mode', and
    creates a WindowTreeHostFactoryRegistrar instance.
    Through this object, WindowTreeHostFactoryRegistrar::Register is
    called, creating WindowTree and WindowTreeHostFactory instances on
    the server side, and acquiring WindowTree and
    WindowTreeHostFactory mojo handles on the client side.

Note: In external window mode, the WindowTree and WindowTreeHostFactory
are unique instances, serving 0 or 'n' WindowTreeHostMus instances.

  • Yet from MusDemoExternal, WindowTreeHostMus instance(s) are
    created, in accordance to the value passed to command line
    parameter --external-window-count, or 1 by default.

Note: In Chrome, this happens in
c/b/ui/views/frame/browser_frame_mus.cc, ::GetWidgetParams when
DesktopWindowTreeHostMus is created for each outer platform window.

WindowTreeHostMus ctor takes a WindowTreeClient instance which
works as a "window tree host delegate".

  • During the creation chain of WindowTreeHostMus, WindowTreeClient::CreateWindowPortForTopLevel
    is called, and this is what calls the newly added method
    WindowTreeHostFactory::CreatePlatformWindow, when in 'external
    window mode'. That is the entry point of of ws::Display instance
    creation.

  • In the existing creation flow of ws::Display, the server
    actually calls back to the client via WindowTreeClient::OnEmbed
    when it is done. In 'external window mode' this callback is kept,
    but it passes a 'null' WindowTree object as parameter.
    Why? Because in ::ConnectViaWindowTreeHostFactory (above) the
    unique WindowTree instance was already created.

  • services/ui/ws/window_tree_host_factory_registrar.cc|h:

    This class implements WindowTreeHostFactoryRegistrar class.
    It exposes a ::Register method, that:

    (a) Binds the mojom::WindowTreeHostFactoryRequest
    (b) Binds the mojom::WindowTreeRequest
    (c) Stores the mojom::WindowTreeClientPtr to call out
    WindowTreeClient later on.
    (d) A ws::WindowTree instance is also created here. It ensures
    single WindowTree and WindowTreeClient instances serving
    various WindowTreeHost instances.

    The goal of the WindowTreeHostFactoryRegistrar interface is
    to ensure a WindowTreeHostFactory::CreatePlatformWindow can not
    be called before WindowTreeHostFactory being properly set up.

  • services/ui/ws/window_tree_host_factory.cc|h:

    Adds ::CreatePlatformWindow method so that it triggers the
    creation of per-outer-window ws::Display instances in external
    window mode.

TODO (in follow up CLs):

  1. Factor non-specific WindowManager specific bits out of
    ws::WindowManagerState, so that event dispatching code can be
    shared between internal and external window modes.

  2. Rename WindowManagerDisplayRoot since it is used in non-WindowManager
    path.

  3. Clean up mus_demo_external (launch instances in parallel?).

  4. Clean up WindowTreeClient::OnEmbed flow.

  5. Experiment with launching Chrome in external window mode.

BUG=666958

patch from issue 2712203002 at patchset 240001 (http://crrev.com/2712203002#ps240001)

This CL is the first step towards having 'external window mode' support
in Mus. The existing MusDemo is adapted to test the functionality,
and its respective unit test (mus_demo_unittests) is changed to
exercise  multiple external windows creation in a follow up CL [2].

[2] https://codereview.chromium.org/2715533005/

New code flow:

* MusDemoExternal creates and holds a WindowTreeClient instance.

Note: In Chrome, it also happens in
c/b/ui/views/chrome_browser_main_extra_parts_views.cc,
::ServiceManagerConnectionStarted, when MusClient is created.

* MusDemoExternal calls WindowTreeClient::ConnectViaWindowTreeHostFactory.
This "enters" WindowTreeClient in 'external window mode', and
creates a WindowTreeHostFactoryRegistrar instance.
Through this object, WindowTreeHostFactoryRegistrar::Register is
called, creating WindowTree and WindowTreeHostFactory instances on
the server side, and acquiring WindowTree and
WindowTreeHostFactory mojo handles on the client side.

Note: In external window mode, the WindowTree and WindowTreeHostFactory
are unique instances, serving 0 or 'n' WindowTreeHostMus instances.

* Yet from MusDemoExternal, WindowTreeHostMus instance(s) are
created, in accordance to the value passed to command line
parameter --external-window-count, or 1 by default.

Note: In Chrome, this happens in
c/b/ui/views/frame/browser_frame_mus.cc, ::GetWidgetParams when
DesktopWindowTreeHostMus is created for each outer platform window.

WindowTreeHostMus ctor takes a WindowTreeClient instance which
works as a "window tree host delegate".

* During the creation chain of WindowTreeHostMus, WindowTreeClient::CreateWindowPortForTopLevel
is called, and this is what calls the newly added method
WindowTreeHostFactory::CreatePlatformWindow, when in 'external
window mode'. That is the entry point of of ws::Display instance
creation.

* In the existing creation flow of ws::Display, the server
actually calls back to the client via WindowTreeClient::OnEmbed
when it is done. In 'external window mode' this callback is kept,
but it passes a 'null' WindowTree object as parameter.
Why? Because in ::ConnectViaWindowTreeHostFactory (above) the
unique WindowTree instance was already created.

* services/ui/ws/window_tree_host_factory_registrar.cc|h:

  This class implements WindowTreeHostFactoryRegistrar class.
  It exposes a ::Register method, that:

    (a) Binds the mojom::WindowTreeHostFactoryRequest
    (b) Binds the mojom::WindowTreeRequest
    (c) Stores the mojom::WindowTreeClientPtr to call out
        WindowTreeClient later on.
    (d) A ws::WindowTree instance is also created here. It ensures
        single WindowTree and WindowTreeClient instances serving
        various WindowTreeHost instances.

  The goal of the WindowTreeHostFactoryRegistrar interface is
  to ensure a WindowTreeHostFactory::CreatePlatformWindow can not
  be called before WindowTreeHostFactory being properly set up.

* services/ui/ws/window_tree_host_factory.cc|h:

  Adds ::CreatePlatformWindow method so that it triggers the
  creation of per-outer-window ws::Display instances in external
  window mode.

TODO (in follow up CLs):

  1) Factor non-specific WindowManager specific bits out of
  ws::WindowManagerState, so that event dispatching code can be
  shared between internal and external window modes.

  2) Rename WindowManagerDisplayRoot since it is used in non-WindowManager
  path.

  3) Clean up mus_demo_external (launch instances in parallel?).

  4) Clean up WindowTreeClient::OnEmbed flow.

  5) Experiment with launching Chrome in external window mode.

BUG=666958

patch from issue 2712203002 at patchset 240001 (http://crrev.com/2712203002#ps240001)
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 22, 2017
…t (patchset Igalia#1 id:1 of https://codereview.chromium.org/2764813004/ )

Reason for revert:
Reverting didn't fix
https://build.chromium.org/p/chromium.android/builders/Lollipop%20Tablet%20Tester/builds/7242
so this patch seems innocent.

Original issue's description:
> Revert of Ignore pointer capture target while calculating click target (patchset Igalia#1 id:1 of https://codereview.chromium.org/2755753003/ )
>
> Reason for revert:
> Tentative revert. Hit testing broke in the following tests, and this patch looks the most closely related:
>
> AccessibilityHitTestingBrowserTest.HitTestingInIframes
> AccessibilityHitTestingBrowserTest.CachingAsyncHitTestingInIframes
>
> Original issue's description:
> > Ignore pointer capture target while calculating click target
> >
> > After https://codereview.chromium.org/2681443003
> > it turns out we are creating some regressions.
> > So this change keeps the essence of the original
> > change for the purpose of keeping the metric but
> > ignores the target that is calculated from the
> > captured target and use the original click target
> > regardless of the capturing.
> >
> > BUG=701599, 699933
> >
> > Review-Url: https://codereview.chromium.org/2755753003
> > Cr-Commit-Position: refs/heads/master@{#458091}
> > Committed: https://chromium.googlesource.com/chromium/src/+/ddb951c0cc2912acd57e4238677b82e6360ac1ad
>
> TBR=dtapuska@chromium.org,mustaq@chromium.org,bokan@chromium.org,nzolghadr@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=701599, 699933
>
> Review-Url: https://codereview.chromium.org/2764813004
> Cr-Commit-Position: refs/heads/master@{#458382}
> Committed: https://chromium.googlesource.com/chromium/src/+/38052ae353aee8c943b570b5a2c10be82a48c1ae

TBR=dtapuska@chromium.org,mustaq@chromium.org,bokan@chromium.org,nzolghadr@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=701599, 699933

Review-Url: https://codereview.chromium.org/2765683003
Cr-Commit-Position: refs/heads/master@{#458431}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 22, 2017
…ttps://codereview.chromium.org/2752673002/ )

Reason for revert:
Enable SPInvalidation in M59.
We should have got enough performance data for non-SPInvalidation.

Original issue's description:
> SPInvalidation -> experimental
>
> We decided to delay launch until M59.
>
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
>
> Review-Url: https://codereview.chromium.org/2752673002
> Cr-Commit-Position: refs/heads/master@{#456857}
> Committed: https://chromium.googlesource.com/chromium/src/+/d6782e3f3ea554066b359c025cb6dc41fc3c3d3f

TBR=pdr@chromium.org,chrishtr@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.

Review-Url: https://codereview.chromium.org/2763033003
Cr-Commit-Position: refs/heads/master@{#458451}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 22, 2017
…t (patchset Igalia#1 id:1 of https://codereview.chromium.org/2755753003/ )

Reason for revert:
Reverting all the changes related to the click target in capturing as they are splitted across multiple commits.

Original issue's description:
> Ignore pointer capture target while calculating click target
>
> After https://codereview.chromium.org/2681443003
> it turns out we are creating some regressions.
> So this change keeps the essence of the original
> change for the purpose of keeping the metric but
> ignores the target that is calculated from the
> captured target and use the original click target
> regardless of the capturing.
>
> BUG=701599, 699933
>
> Review-Url: https://codereview.chromium.org/2755753003
> Cr-Commit-Position: refs/heads/master@{#458091}
> Committed: https://chromium.googlesource.com/chromium/src/+/ddb951c0cc2912acd57e4238677b82e6360ac1ad

TBR=dtapuska@chromium.org,mustaq@chromium.org,bokan@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=701599, 699933

Review-Url: https://codereview.chromium.org/2762913003
Cr-Commit-Position: refs/heads/master@{#458506}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 22, 2017
…tps://codereview.chromium.org/2762203002/ )

Reason for revert:
The fix relanded in https://codereview.chromium.org/2764613002/.

Original issue's description:
> Mark 3 tests flaky on android
>
> The following tests are flaky:
> - SecurityExploitBrowserTest.AttemptDuplicateRenderWidgetHost
> - SecurityExploitBrowserTest.AttemptDuplicateRenderViewHost
> - RenderFrameHostManagerTest.DontShowLoadingURLIfNotInitialNav
>
> NOTREECHECKS=true
> NOTRY=true
> BUG=703657,702584
> TBR=boliu@chromium.org
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
>
> Review-Url: https://codereview.chromium.org/2762203002
> Cr-Commit-Position: refs/heads/master@{#458413}
> Committed: https://chromium.googlesource.com/chromium/src/+/d705708b78acf0c3399475f6bcd46433bdc65eee

TBR=boliu@chromium.org,johnme@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=703657,702584

Review-Url: https://codereview.chromium.org/2767733002
Cr-Commit-Position: refs/heads/master@{#458534}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 22, 2017
…ng destroyed (patchset Igalia#1 id:1 of https://codereview.chromium.org/2760343002/ )

Reason for revert:
Looks like this breaks PointerLockBrowserTest.PointerLockChildFrameDetached: https://uberchromegw.corp.google.com/i/chromium.memory/builders/Mac%20ASan%2064%20Tests%20%281%29/builds/28142

Original issue's description:
> Only release the pointer lock when the RenderWidgetHost being destroyed
> is the one holding the pointer lock.
>
> This fixes a crash where the WebContents would stop tracking which
> RenderWidgetHost was holding the pointer lock.
>
> BUG=703284
>
> Review-Url: https://codereview.chromium.org/2760343002
> Cr-Commit-Position: refs/heads/master@{#458446}
> Committed: https://chromium.googlesource.com/chromium/src/+/bf846a94da5c2b4f8ff93a7942d3d50799381675

TBR=creis@chromium.org,alexmos@chromium.org,lfg@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=703284

Review-Url: https://codereview.chromium.org/2763953003
Cr-Commit-Position: refs/heads/master@{#458539}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 22, 2017
…ng destroyed (patchset Igalia#1 id:1 of https://codereview.chromium.org/2763953003/ )

Reason for revert:
Relanding with the test disabled on Mac.

Original issue's description:
> Revert of Only release the pointer lock when the RenderWidgetHost being destroyed (patchset Igalia#1 id:1 of https://codereview.chromium.org/2760343002/ )
>
> Reason for revert:
> Looks like this breaks PointerLockBrowserTest.PointerLockChildFrameDetached: https://uberchromegw.corp.google.com/i/chromium.memory/builders/Mac%20ASan%2064%20Tests%20%281%29/builds/28142
>
> Original issue's description:
> > Only release the pointer lock when the RenderWidgetHost being destroyed
> > is the one holding the pointer lock.
> >
> > This fixes a crash where the WebContents would stop tracking which
> > RenderWidgetHost was holding the pointer lock.
> >
> > BUG=703284
> >
> > Review-Url: https://codereview.chromium.org/2760343002
> > Cr-Commit-Position: refs/heads/master@{#458446}
> > Committed: https://chromium.googlesource.com/chromium/src/+/bf846a94da5c2b4f8ff93a7942d3d50799381675
>
> TBR=creis@chromium.org,alexmos@chromium.org,lfg@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=703284
>
> Review-Url: https://codereview.chromium.org/2763953003
> Cr-Commit-Position: refs/heads/master@{#458539}
> Committed: https://chromium.googlesource.com/chromium/src/+/a2fda5591832a96af3146f6a3db8cb9b947c2a86

TBR=creis@chromium.org,alexmos@chromium.org,dgozman@chromium.org
BUG=703284

Review-Url: https://codereview.chromium.org/2764703004
Cr-Commit-Position: refs/heads/master@{#458692}
@fred-wang
Copy link
Member

lgtm

(I've haven't checked if there are many differences compared to 2712203002)

@tonikitoo
Copy link
Member Author

there is a merge conflict fix, only.

@tonikitoo tonikitoo merged commit 105b9f4 into Igalia:ozone-wayland-dev Mar 22, 2017
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 27, 2017
…." (patchset Igalia#1 id:1 of https://codereview.chromium.org/2757883002/ )

Reason for revert:
Experiment finished, we do want bb9b04f (cf. https://bugs.chromium.org/p/chromium/issues/detail?id=682945#c48 )

Original issue's description:
> Revert "Pull AudioDestinationConsumer off the Blink GC heap."
>
> This reverts commit bb9b04f,
> attempting to diagnose root crash cause.
>
> R=rtoy,haraken
> BUG=682945
>
> Review-Url: https://codereview.chromium.org/2757883002
> Cr-Commit-Position: refs/heads/master@{#457833}
> Committed: https://chromium.googlesource.com/chromium/src/+/937ebc1a2de7ac71ec1de4c6bb1ed7a6331e4683

TBR=haraken@chromium.org,hongchan@chromium.org,rtoy@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=682945

Review-Url: https://codereview.chromium.org/2762413004
Cr-Commit-Position: refs/heads/master@{#458700}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 27, 2017
…onsumers. (patchset Igalia#1 id:1 of https://codereview.chromium.org/2761463002/ )

Reason for revert:
Revert this check, no longer needed (cf. https://bugs.chromium.org/p/chromium/issues/detail?id=682945#c48 )

Original issue's description:
> Lock out GCs while iterating over MediaStreamSource audio consumers.
>
> Attempt to diagnose a crash condition by locking out main thread GCs
> while the audio thread propagates consumeAudio() updates.
>
> R=haraken
> BUG=682945
>
> Review-Url: https://codereview.chromium.org/2761463002
> Cr-Commit-Position: refs/heads/master@{#457753}
> Committed: https://chromium.googlesource.com/chromium/src/+/e78674d2467706fa8634f8542251591d1d358b57

TBR=haraken@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=682945

Review-Url: https://codereview.chromium.org/2768683004
Cr-Commit-Position: refs/heads/master@{#458716}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 27, 2017
…patchset Igalia#1 id:1 of https://codereview.chromium.org/2754053002/ )

Reason for revert:
Root cause should be fixed.

Original issue's description:
> Disable key_mobile_sites benchmarks which are timing out.
>
> BUG=702194
> TBR=nednguyen@google.com
>
> Review-Url: https://codereview.chromium.org/2754053002
> Cr-Commit-Position: refs/heads/master@{#457435}
> Committed: https://chromium.googlesource.com/chromium/src/+/2b9c05fb215e52b9b360eb11118e3ee5c3b657e6

TBR=sullivan@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=702194

Review-Url: https://codereview.chromium.org/2764323002
Cr-Commit-Position: refs/heads/master@{#458729}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 27, 2017
…ia#1 id:1 of https://codereview.chromium.org/2758533003/ )

Reason for revert:
Root cause should be fixed.

Original issue's description:
> Disable failing blink_style.key_mobile_sites
>
> BUG=702194
>
> Review-Url: https://codereview.chromium.org/2758533003
> Cr-Commit-Position: refs/heads/master@{#457432}
> Committed: https://chromium.googlesource.com/chromium/src/+/4d954851ba9c129b4529302f77862f6b812165d8

TBR=sullivan@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=702194

Review-Url: https://codereview.chromium.org/2768683005
Cr-Commit-Position: refs/heads/master@{#458730}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 27, 2017
Changes: https://chromium.googlesource.com/external/webrtc/trunk/webrtc.git/+log/43634fa..fd3f94e

$ git log 43634fa..fd3f94e --date=short --no-merges --format=%ad %ae %s
2017-03-22 perkj@webrtc.org Reduce CPU usage in test::FrameGenerator::SquareGenerator. This cl change the drawing to  use memset per row in each square instead of setting each pixel individually.
2017-03-22 nisse@webrtc.org Reland of Delete class MockCongestionController. (patchset Igalia#1 id:1 of https://codereview.webrtc.org/2762133003/ )
2017-03-22 henrikg@webrtc.org Revert of Add DesktopCapturerId and attach it to DesktopFrame (patchset Igalia#4 id:100002 of https://codereview.webrtc.org/2759493002/ )
2017-03-22 kthelgason@webrtc.org Don't reset quality scaler on resolution change.
2017-03-21 skvlad@webrtc.org Revert of Add framerate to VideoSinkWants and ability to signal on overuse (patchset Igalia#14 id:250001 of https://codereview.webrtc.org/2716643002/ )
2017-03-21 skvlad@webrtc.org Revert of Delete class MockCongestionController. (patchset Igalia#4 id:60001 of https://codereview.webrtc.org/2762023004/ )
2017-03-21 zijiehe@chromium.org Add DesktopCapturerId and attach it to DesktopFrame
2017-03-21 kwiberg@webrtc.org Don't expect factory to create iLBC decoder if it isn't supported
2017-03-21 sprang@webrtc.org Add framerate to VideoSinkWants and ability to signal on overuse
2017-03-21 zhihuang@webrtc.org Parse the connection data in SDP (c= line).
2017-03-21 nisse@webrtc.org Delete class MockCongestionController.
2017-03-21 pbos@webrtc.org Remove dead code in vp8_impl files.
2017-03-21 oprypin@webrtc.org Make lint errors fatal in presubmit and fix files in whitelisted paths
2017-03-21 elad.alon@webrtc.org TransportFeedbackPacketLossTracker to receive std::vector<PacketFeedback> in place of the entire feedback
2017-03-21 nisse@webrtc.org Split CongestionController into send- and receive-side classes.
2017-03-21 elad.alon@webrtc.org TransportFeedbackPacketLossTrackerTest cosmetic modification
2017-03-21 philipel@webrtc.org Allow padding packet in video streams.
2017-03-21 minyue@webrtc.org Adding audio DTX to video loopback test.
2017-03-21 philipel@webrtc.org Probing EndToEndTests.
2017-03-21 tommi@webrtc.org Add thread check to ModuleProcessThread::DeRegisterModule and remove all unnecessary locking that was there due to one implementation calling from a different thread.
2017-03-21 nisse@webrtc.org Delete VP8 feedback mode.

TBR=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng
BUG=

Review-Url: https://codereview.chromium.org/2765233002
Cr-Commit-Position: refs/heads/master@{#458733}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 27, 2017
…patchset Igalia#1 id:1 of https://codereview.chromium.org/2741663004/ )

Reason for revert:
Diagnosis completed, reverting (cf. https://bugs.chromium.org/p/chromium/issues/detail?id=682945#c48 )

Original issue's description:
> MediaStreamSource: verify unlocked state when finalizing.
>
> To diagnose an audio thread crash condition, verify that the lock
> over audio consumers that MediaStreamSource keeps, isn't held when it is
> being finalized. If it is, then the audio thread is active using the
> MediaStreamSource object..which is not a well-formed state to be in.
>
> R=
> BUG=682945
>
> Review-Url: https://codereview.chromium.org/2741663004
> Cr-Commit-Position: refs/heads/master@{#456029}
> Committed: https://chromium.googlesource.com/chromium/src/+/c662576c8bb7cecef0dd9a699112fa5cc4b6ab79

TBR=hongchan@chromium.org,haraken@chromium.org,rtoy@chromium.org,guidou@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=682945

Review-Url: https://codereview.chromium.org/2765073004
Cr-Commit-Position: refs/heads/master@{#458744}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 27, 2017
…aitMany (patchset Igalia#1 id:1 of https://codereview.chromium.org/2761993004/ )

Reason for revert:
Even if this did negatively impact telemetry performance, we'd want the WaitMany change for the sake of API correctness.

However, after leaving the revert in place for a day, it doesn't look like this CL was the culprit anyway.

Original issue's description:
> Revert of Ensure consistent return ordering in base::WaitableEvent::WaitMany (patchset Igalia#5 id:80001 of https://codereview.chromium.org/2758853002/ )
>
> Reason for revert:
> Causing extra time for chrome_public_test_apk, telemetry_perf_unittests etc.
>
> Original issue's description:
> > Ensure consistent return ordering in base::WaitableEvent::WaitMany
> >
> > The Windows implementation (via WaitForMultipleObjects) guarantees that
> > if multiple objects are signaled, the one with the smallest index is
> > returned on wakeup. This is a useful guarantee to make, as callers are
> > granted the ability to rotate objects as needed to avoid potential
> > starvation of lower-frequency events.
> >
> > The POSIX implementation of WaitMany does not support this behavior, as
> > precedence is determined exclusively by WaitableEvent address order
> > rather than input array order. The behavior today is a result of the
> > implementation detail that we must lock the WaitableEvent kernels in a
> > globally consistent order, and this makes it effectively impossible for
> > a caller to control signaling priority.
> >
> > This CL changes the POSIX implementation to guarantee the same return
> > behavior as the Windows implementation, allowing the WaitMany API to in
> > turn guarantee left-to-right precedence of the WaitableEvents with which
> > it's called.
> >
> > This has some minor additional cost as we no longer short-circuit on the
> > first signaled event we find, and we always have to iterate over the array
> > twice now. This cost is negligible in practice as WaitMany is only ever
> > used in production with a small (n < 5) number of events.
> >
> > BUG=698460
> >
> > Review-Url: https://codereview.chromium.org/2758853002
> > Cr-Commit-Position: refs/heads/master@{#457967}
> > Committed: https://chromium.googlesource.com/chromium/src/+/926b72bf8aba665d906fe411a11a9f587335c1bb
>
> TBR=dcheng@chromium.org,rockot@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=698460
>
> Review-Url: https://codereview.chromium.org/2761993004
> Cr-Commit-Position: refs/heads/master@{#458602}
> Committed: https://chromium.googlesource.com/chromium/src/+/a0a9840a39b61caca179486dcf20927fe99ad4a6

TBR=dcheng@chromium.org,hzl@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=698460

Review-Url: https://codereview.chromium.org/2773463002
Cr-Commit-Position: refs/heads/master@{#458843}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 27, 2017
…galia#1 id:20001 of https://codereview.chromium.org/2768793002/ )

Reason for revert:
Looks like it's still failing :( It was worth a shot!

https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.chromiumos%2FLinux_ChromiumOS_Tests__dbg__1_%2F23893%2F%2B%2Frecipes%2Fsteps%2Fbrowser_tests%2F0%2Flogs%2FExtensionApiTest.ChromeRuntimeReload%2F0

Original issue's description:
> Enable ExtensionApiTest.ChromeRuntimeReload test.
>
> From revision ea9d0bc, extension id is being used to listen exact
> extension's loading and unloading. It prevents  the flakiness caused
> by unexpected extension's loading and unloading so that we can enable test again.
>
> Bug=366181
> TEST=browser_tests --gtest_filter=ExtensionApiTest.ChromeRuntimeReload
>
> Review-Url: https://codereview.chromium.org/2768793002
> Cr-Commit-Position: refs/heads/master@{#458737}
> Committed: https://chromium.googlesource.com/chromium/src/+/d38b95f21eebdcc6a01eaee260e94ffa0cc746a0

TBR=limasdf@gmail.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Bug=366181

Review-Url: https://codereview.chromium.org/2766333003
Cr-Commit-Position: refs/heads/master@{#458874}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 27, 2017
…ps://codereview.chromium.org/2766953003/ )

Reason for revert:
Suspecting this roll broke layout tests:
https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac10.11%20%28dbg%29/builds/8002

Example crash log with check triggered:
https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Mac10_11__dbg_/8002/layout-test-results/http/tests/notifications/serviceworker-notification-properties-crash-log.txt

Original issue's description:
> Update V8 to version 5.9.71.
>
> Summary of changes available at:
> https://chromium.googlesource.com/v8/v8/+log/3755ef9f..17defabd
>
> Please follow these instructions for assigning/CC'ing issues:
> https://github.com/v8/v8/wiki/Triaging%20issues
>
> Please close rolling in case of a roll revert:
> https://v8-roll.appspot.com/
> This only works with a Google account.
>
> CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
>
> TBR=hablich@chromium.org,machenbach@chromium.org,kozyatinskiy@chromium.org,vogelheim@chromium.org
>
> Review-Url: https://codereview.chromium.org/2766953003
> Cr-Commit-Position: refs/heads/master@{#458776}
> Committed: https://chromium.googlesource.com/chromium/src/+/cd4a0360b748ad1d7c0898474552144d9aa09a03

TBR=hablich@chromium.org,kozyatinskiy@chromium.org,machenbach@chromium.org,vogelheim@chromium.org,v8-autoroll@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review-Url: https://codereview.chromium.org/2763393003
Cr-Commit-Position: refs/heads/master@{#458940}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 27, 2017
… NotifyDumpTriggered" (patchset Igalia#1 id:1 of https://codereview.chromium.org/2766933004/ )

Reason for revert:
Reland with extra asserts to debug the failure in waterfall.

Original issue's description:
> Revert "[memory-infra] Add unittests for peak detection and NotifyDumpTriggered"
>
> Reason: breaks CFI buildbots
>
> https://build.chromium.org/p/chromium.fyi/builders/CFI%20Linux/builds/7700
> https://build.chromium.org/p/chromium.fyi/builders/CFI%20Linux%20ToT/builds/5852
>
> This reverts the CL https://codereview.chromium.org/2743663005
>
> BUG=607533
> TBR=dcheng
>
> Review-Url: https://codereview.chromium.org/2766933004
> Cr-Commit-Position: refs/heads/master@{#458787}
> Committed: https://chromium.googlesource.com/chromium/src/+/68d51c6dd7715dcb357341845d7f8103b82febb1

TBR=primiano@chromium.org,dcheng@chromium.org,krasin@chromium.org
BUG=607533

Review-Url: https://codereview.chromium.org/2766303002
Cr-Commit-Position: refs/heads/master@{#458953}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 27, 2017
…atchset Igalia#1 id:1 of https://codereview.chromium.org/2769813004/ )

Reason for revert:
The issue fixed itself without a revert.

Original issue's description:
> Revert of Simplify ExtensionInstallChecker into a single-use class (patchset Igalia#7 id:120001 of https://codereview.chromium.org/2751013002/ )
>
> Reason for revert:
> Speculative revert for ServiceWorkerTest.ServiceWorkerSuspensionOnExtensionUnload failure on the following bot:
> https://uberchromegw.corp.google.com/i/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/58372
>
> Original issue's description:
> > Simplify ExtensionInstallChecker into a single-use class
> >
> > ExtensionInstallChecker does asynchronous checks, and its set of checks
> > can be started more than once. This functionality isn't being used and
> > makes the class more complicated. Remove
> > ExtensionInstallChecker::ResetResults().
> >
> > It's also unclear why the users of this class (CrxInstaller and
> > UnpackedInstaller) are intentionally storing their Profile and Extension
> > pointers in ExtensionInstallChecker class instead of remembering them
> > locally, so update that and remove
> > ExtensionInstallChecker::set_extension().
> >
> > Lastly, allocate the ExtensionInstallChecker dynamically since we don't
> > always use it.
> >
> > BUG=none
> > R=rdevlin.cronin@chromium.org
> >
> > Review-Url: https://codereview.chromium.org/2751013002
> > Cr-Commit-Position: refs/heads/master@{#458883}
> > Committed: https://chromium.googlesource.com/chromium/src/+/b24adc5bcbfde7fd67c6b23c3b0721610c77f63b
>
> TBR=rdevlin.cronin@chromium.org,michaelpg@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=none
>
> Review-Url: https://codereview.chromium.org/2769813004
> Cr-Commit-Position: refs/heads/master@{#458932}
> Committed: https://chromium.googlesource.com/chromium/src/+/9cc4c29fd455f27d818bcca95934a2da2332459e

TBR=rdevlin.cronin@chromium.org,michaelpg@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=none

Review-Url: https://codereview.chromium.org/2766303003
Cr-Commit-Position: refs/heads/master@{#458955}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 27, 2017
…rker code (patchset Igalia#1 id:1 of https://codereview.chromium.org/2767113002/ )

Reason for revert:
I have confirmed that the codepath is being hit in the latest Canary.

Original issue's description:
> PlzNavigate: add temporary DumpWithoutCrashing to ServiceWorker code
>
> This CL adds a temporary DumpWithoutCrashing to
> ServiceWorkerDispatcherHost::OnProviderCreated. There is one case when
> PlzNavigate is enabled where we'll exit the function without creating a
> ServiceWorkerProviderHost. I'd like to know if we're hitting it in the
> wild, and if it could be causing the renderer kills in crbug.com/703972,
> where the renderer gets killed because it tries to add a ServiceWorker
> for ServiceWorkerProviderHost that is not found.
>
> BUG=703972
>
> Review-Url: https://codereview.chromium.org/2767113002
> Cr-Commit-Position: refs/heads/master@{#458856}
> Committed: https://chromium.googlesource.com/chromium/src/+/5cbb188b9d0ad7851eb50ca807c6bb37e02128e6

TBR=nasko@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=703972

Review-Url: https://codereview.chromium.org/2769223002
Cr-Commit-Position: refs/heads/master@{#459100}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 27, 2017
…reams (patchset Igalia#1 id:1 of https://codereview.chromium.org/2632503004/ )

Reason for revert:
Speculative revert to identify the cause of Media.AudioRendererAudioGlitches. Should be relanded after we collect enough info to make a conclusion if this CL is a culprit or not.

BUG=686689

Original issue's description:
> Increases default maximum number of audio input & output streams.
> The increase is from 16 to 32.
>
> BUG=679215
>
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
>
> Review-Url: https://codereview.chromium.org/2632503004
> Cr-Commit-Position: refs/heads/master@{#443589}
> Committed: https://chromium.googlesource.com/chromium/src/+/1bb13224524862e5d3be4ac68c6377a4e54c9433

TBR=tommi@chromium.org,henrika@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=679215

Review-Url: https://codereview.chromium.org/2772723003
Cr-Commit-Position: refs/heads/master@{#459110}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Mar 27, 2017
…ia#1 id:1 of https://codereview.chromium.org/2768123002/ )

Reason for revert:
The race was caused by https://codereview.chromium.org/2736283003/
Fixed the post tasks.

Original issue's description:
> Revert of Add process uptime metadata event in tracing (patchset Igalia#4 id:80001 of https://codereview.chromium.org/2753133002/ )
>
> Reason for revert:
> Looks like this broke base_unittests on Linux TSan: https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20TSan%20Tests/builds/2916
>
> Original issue's description:
> > Add process uptime metadata event in tracing
> >
> > 1. Make CurrentProcessInfo::CreationTime consistent on Linux with Win
> >    and Mac implementation: return null value instead of crashing on
> >    failure.
> > 2. Use process_info_linux.cc in Android too, it works the same.
> > 3. Add the total process uptime as meadata event in traces and
> >    whitelist in filtered mode.
> >
> > BUG=654667
> >
> > Review-Url: https://codereview.chromium.org/2753133002
> > Cr-Commit-Position: refs/heads/master@{#458799}
> > Committed: https://chromium.googlesource.com/chromium/src/+/a714d7dc0f1b8ecdee128db04e24b0da1087df8a
>
> TBR=oysteine@chromium.org,thakis@chromium.org,primiano@chromium.org,ssid@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=654667
>
> Review-Url: https://codereview.chromium.org/2768123002
> Cr-Commit-Position: refs/heads/master@{#458869}
> Committed: https://chromium.googlesource.com/chromium/src/+/958a4bb090a8f57113d094be99b573c036ecb1e0

TBR=oysteine@chromium.org,thakis@chromium.org,dgozman@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
BUG=654667

Review-Url: https://codereview.chromium.org/2768933002
Cr-Commit-Position: refs/heads/master@{#459167}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Apr 3, 2017
…ia#1 id:1 of https://codereview.chromium.org/2772503005/ )

Reason for revert:
expected_deps_x64_jessie changes are causing a failure on the official builder

https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.chrome%2FGoogle_Chrome_Linux_x64%2F16997%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout

Original issue's description:
> Reland of "Update linux sysroot from Wheezy to Jessie"
>
> The last version of this change was:
> https://codereview.chromium.org/2748183005
>
> Which was reverted in:
> https://codereview.chromium.org/2776503002
>
> I've updated the expected package deps again, this
> time being sure to use is_chrome_branded=true when
> testing (which oddly seems to effect the deps).
>
> TBR=thestig (since this is effectively a reland)
> BUG=701894
>
> Review-Url: https://codereview.chromium.org/2772503005
> Cr-Commit-Position: refs/heads/master@{#459492}
> Committed: https://chromium.googlesource.com/chromium/src/+/7f43e46a7830cb92c08b8245f7f48f0d3a2fab47

TBR=thakis@chromium.org,dpranke@chromium.org,thestig@chromium.org,sbc@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=701894

Review-Url: https://codereview.chromium.org/2772113002
Cr-Commit-Position: refs/heads/master@{#459553}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Apr 3, 2017
…ia#1 id:1 of https://codereview.chromium.org/2772113002/ )

Reason for revert:
Relanding with fixed deps

Original issue's description:
> Revert of "Update linux sysroot from Wheezy to Jessie" (patchset Igalia#1 id:1 of https://codereview.chromium.org/2772503005/ )
>
> Reason for revert:
> expected_deps_x64_jessie changes are causing a failure on the official builder
>
> https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.chrome%2FGoogle_Chrome_Linux_x64%2F16997%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout
>
> Original issue's description:
> > Reland of "Update linux sysroot from Wheezy to Jessie"
> >
> > The last version of this change was:
> > https://codereview.chromium.org/2748183005
> >
> > Which was reverted in:
> > https://codereview.chromium.org/2776503002
> >
> > I've updated the expected package deps again, this
> > time being sure to use is_chrome_branded=true when
> > testing (which oddly seems to effect the deps).
> >
> > TBR=thestig (since this is effectively a reland)
> > BUG=701894
> >
> > Review-Url: https://codereview.chromium.org/2772503005
> > Cr-Commit-Position: refs/heads/master@{#459492}
> > Committed: https://chromium.googlesource.com/chromium/src/+/7f43e46a7830cb92c08b8245f7f48f0d3a2fab47
>
> TBR=thakis@chromium.org,dpranke@chromium.org,thestig@chromium.org,sbc@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=701894
>
> Review-Url: https://codereview.chromium.org/2772113002
> Cr-Commit-Position: refs/heads/master@{#459553}
> Committed: https://chromium.googlesource.com/chromium/src/+/f2296e4fcaef7fdced40ebf3ca736d5dc58c66cd

TBR=thakis@chromium.org,dpranke@chromium.org,thestig@chromium.org,thomasanderson@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=701894

Review-Url: https://codereview.chromium.org/2776773002
Cr-Commit-Position: refs/heads/master@{#459584}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Apr 3, 2017
…(patchset Igalia#1 id:1 of https://codereview.chromium.org/2752083002/ )

Reason for revert:
Speculative, looks like this broke a bunch of devtool tests on msan, first failure at https://build.chromium.org/p/chromium.memory.full/builders/Linux%20MSan%20Tests/builds/6404 but still broken 80 builds later.

Original issue's description:
> DevTools: fix mixed arguments in Tracing.bufferUsage event
>
> Originally regressed by https://codereview.chromium.org/2500093002
>
> BUG=b/36176396
>
> Review-Url: https://codereview.chromium.org/2752083002
> Cr-Commit-Position: refs/heads/master@{#457227}
> Committed: https://chromium.googlesource.com/chromium/src/+/c8dc5d159a4716659d8698b093794f7c9eaa24ed

TBR=dgozman@chromium.org,caseq@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=b/36176396

Review-Url: https://codereview.chromium.org/2776043002
Cr-Commit-Position: refs/heads/master@{#459668}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Apr 3, 2017
…(patchset Igalia#1 id:1 of https://codereview.chromium.org/2776043002/ )

Reason for revert:
Re-land, see comments in https://codereview.chromium.org/2752083002 for details.
BUG=705306

Original issue's description:
> Revert of DevTools: fix mixed arguments in Tracing.bufferUsage event (patchset Igalia#1 id:1 of https://codereview.chromium.org/2752083002/ )
>
> Reason for revert:
> Speculative, looks like this broke a bunch of devtool tests on msan, first failure at https://build.chromium.org/p/chromium.memory.full/builders/Linux%20MSan%20Tests/builds/6404 but still broken 80 builds later.
>
> Original issue's description:
> > DevTools: fix mixed arguments in Tracing.bufferUsage event
> >
> > Originally regressed by https://codereview.chromium.org/2500093002
> >
> > BUG=b/36176396
> >
> > Review-Url: https://codereview.chromium.org/2752083002
> > Cr-Commit-Position: refs/heads/master@{#457227}
> > Committed: https://chromium.googlesource.com/chromium/src/+/c8dc5d159a4716659d8698b093794f7c9eaa24ed
>
> TBR=dgozman@chromium.org,caseq@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=b/36176396
>
> Review-Url: https://codereview.chromium.org/2776043002
> Cr-Commit-Position: refs/heads/master@{#459668}
> Committed: https://chromium.googlesource.com/chromium/src/+/e14291f0adaf71cd42ef1c1a9d70ace4e309a1ba

TBR=dgozman@chromium.org,thakis@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=b/36176396

Review-Url: https://codereview.chromium.org/2763413011
Cr-Commit-Position: refs/heads/master@{#459681}
tonikitoo pushed a commit to tonikitoo/chromium that referenced this pull request Apr 3, 2017
…//codereview.chromium.org/2773333002/ )

Reason for revert:
Compile failed on WinClang64.

https://uberchromegw.corp.google.com/i/chromium.win/builders/WinClang64%20%28dbg%29/builds/11236

https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.win%2FWinClang64__dbg_%2F11236%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout

[479/12131] CXX obj/sandbox/win/pocdll/handles.obj
FAILED: obj/sandbox/win/pocdll/handles.obj
C:\b\c\goma_client/gomacc.exe ../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo /showIncludes /FC @obj/sandbox/win/pocdll/handles.obj.rsp /c ../../sandbox/win/sandbox_poc/pocdll/handles.cc /Foobj/sandbox/win/pocdll/handles.obj /Fd"obj/sandbox/win/pocdll_cc.pdb"
C:\b\c\b\win_clang\src\sandbox\win\sandbox_poc\pocdll\handles.cc(7,10):  fatal error: 'sandbox/win/tools/finder/ntundoc.h' file not found
#include "sandbox/win/tools/finder/ntundoc.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

Original issue's description:
> Remove sandbox/win/tools.
>
> The files therein look unused, haven't been really touched since 2012, aren't
> referenced in any build files, and have some maintenance cost.
>
> BUG=661774
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
>
> Review-Url: https://codereview.chromium.org/2773333002
> Cr-Commit-Position: refs/heads/master@{#459696}
> Committed: https://chromium.googlesource.com/chromium/src/+/6625b2f764ae4efb0637215660500ba721b8a43d

TBR=wfh@chromium.org,thakis@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=661774

Review-Url: https://codereview.chromium.org/2775123002
Cr-Commit-Position: refs/heads/master@{#459698}
jkim-julie pushed a commit that referenced this pull request Nov 5, 2018
This reverts commit 2903236.

Reason for revert: ChromiumOS MSAN got flaky/failing crbug.com/899664

Original change's description:
> Change NavigationListModel to support MyFiles as Volume.
> 
> Add MYFILES_VOLUME_ENABLED in private strings to be able to have the
> feature flag in the JS/UI code.
> 
> Change VolumeManager to use MyFiles folder for Downloads volume, the
> method |GetMyFilesFolderForProfile| takes care of returning the right
> value based on the feature flag MyFilesVolume.
> 
> Change NavigationListModel to support MyFiles as Volume when
> MyFilesVolume feature flag is enabled:
> 1. MyFiles volume is the Downloads volume (after the feature flag is
> removed we can rename it accordingly). Setup Downloads volume as
> VolumeEntry,
> 2. Create a NavigationModelFakeItem with "My files" label, based on the
> VolumeEntry (step #1).
> 3. Skip adding "Downloads" to Myfiles, since now MyFiles is in fact
> Downloads volume.
> 4. Continue to add Linux and Play files to Myfiles.
> 
> Test: unit_tests --gtest_filter='VolumeManagerTest.GetVolumeListMyFilesVolume' and browser_test --gtest_filter='FileManagerJsTest.DirectoryTreeTest:FileManagerJsTest.NavigationList*'
> Bug: 873539
> Change-Id: Ia242d52a1e4d7b4fb3c7ca219d9cfdc4fce72543
> Reviewed-on: https://chromium-review.googlesource.com/c/1308235
> Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
> Reviewed-by: Joel Hockey <joelhockey@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#604143}

TBR=joelhockey@chromium.org,lucmult@chromium.org,ioanap@chromium.org

Change-Id: I0c5b1fd5f23b2ec2b56ab13ab93a2d3a1ffd277a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 873539
Reviewed-on: https://chromium-review.googlesource.com/c/1309569
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604193}
jkim-julie pushed a commit that referenced this pull request Nov 5, 2018
We are looking for the minimum displacement of the PIP window that makes
it not intersect any system trays or the floating keyboard.

There are four cases for how the PIP window could move.
Case #1: Touches 0 obstacles. This corresponds to not moving.
Case #2: Touches 1 obstacle.
  The PIP window will be touching one edge of the obstacle.
Case #3: Touches 2 obstacles.
  The PIP window will be touching one horizontal and one vertical edge
  from two different obstacles.
Case #4: Touches more than 2 obstacles. This is handled in case #3.

To handle all these cases, we just need to check a few candidate points.
If the PIP window is displaced by one obstacle, the shortest
displacement is to move along the horizontal or vertical axis to be
directly adjacent to one of the edges of that obstacle.

If it is displaced by two obstacles, the shortest displacement is to
move to be directly adjacent to a horizontal and vertical edge - one
from each obstacle.

keyboard and unified system tray update, and the PIP window moved out of
the way of both the floating virtual keyboard and unified system tray.

Bug: 883118
Bug: 841886
Bug: b/115291749
Test: Added unittest
Test: Patched in code calling GetPositionAfterMovementAreaChange on
Change-Id: I2b500d5ba4a67fe2e309b809ec667248a19518ca
Reviewed-on: https://chromium-review.googlesource.com/c/1221427
Commit-Queue: Eliot Courtney <edcourtney@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604523}
jkim-julie pushed a commit that referenced this pull request Nov 5, 2018
This is a reland of 2903236

Original change's description:
> Change NavigationListModel to support MyFiles as Volume.
> 
> Add MYFILES_VOLUME_ENABLED in private strings to be able to have the
> feature flag in the JS/UI code.
> 
> Change VolumeManager to use MyFiles folder for Downloads volume, the
> method |GetMyFilesFolderForProfile| takes care of returning the right
> value based on the feature flag MyFilesVolume.
> 
> Change NavigationListModel to support MyFiles as Volume when
> MyFilesVolume feature flag is enabled:
> 1. MyFiles volume is the Downloads volume (after the feature flag is
> removed we can rename it accordingly). Setup Downloads volume as
> VolumeEntry,
> 2. Create a NavigationModelFakeItem with "My files" label, based on the
> VolumeEntry (step #1).
> 3. Skip adding "Downloads" to Myfiles, since now MyFiles is in fact
> Downloads volume.
> 4. Continue to add Linux and Play files to Myfiles.
> 
> Test: unit_tests --gtest_filter='VolumeManagerTest.GetVolumeListMyFilesVolume' and browser_test --gtest_filter='FileManagerJsTest.DirectoryTreeTest:FileManagerJsTest.NavigationList*'
> Bug: 873539
> Change-Id: Ia242d52a1e4d7b4fb3c7ca219d9cfdc4fce72543
> Reviewed-on: https://chromium-review.googlesource.com/c/1308235
> Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
> Reviewed-by: Joel Hockey <joelhockey@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#604143}

Bug: 873539
Change-Id: I396cda87781c21856a736c6f5eae8f76efb4e62c
Reviewed-on: https://chromium-review.googlesource.com/c/1314012
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604836}
jkim-julie pushed a commit that referenced this pull request Nov 12, 2018
This reverts commit 4df3f6e.

Reason for revert: This CL causes crash in RasterCommandBufferStub.
The raster decoder still uses GL context and GL APIs even with vulkan.
Before removing the GL contect for vulkan, we need remove all
unnecessary GL context usage and GL calls for raster decoder.

#0 0x55d1ec37f6ef base::debug::StackTrace::StackTrace()
#1 0x55d1ec37f261 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#2 0x7fe55e62d0c0 <unknown>
#3 0x55d1ed30103e gpu::RasterCommandBufferStub::Initialize()
#4 0x55d1ed25f35c gpu::GpuChannel::OnCreateCommandBuffer()
#5 0x55d1ed25ed0a bool IPC::MessageT<GpuChannelMsg_CreateCommandBuffer_Meta, std::__1::tuple<GPUCreateCommandBufferConfig, int, base::UnsafeSharedMemoryRegion>, std::__1::tuple<gpu::ContextResult, gpu::Capabilities> >::Dispatch<gpu::GpuChannel, gpu::GpuChannel, void, void (gpu::GpuChannel::*)(GPUCreateCommandBufferConfig const&, int, base::UnsafeSharedMemoryRegion, gpu::ContextResult*, gpu::Capabilities*)>(IPC::Message const*, gpu::GpuChannel*, gpu::GpuChannel*, void*, void (gpu::GpuChannel::*)(GPUCreateCommandBufferConfig const&, int, base::UnsafeSharedMemoryRegion, gpu::ContextResult*, gpu::Capabilities*))
#6 0x55d1ed25fcc4 gpu::GpuChannel::HandleMessageHelper()
#7 0x55d1ec3058f1 base::debug::TaskAnnotator::RunTask()
#8 0x55d1ec3050af base::MessageLoop::RunTask()
#9 0x55d1ec305432 base::MessageLoop::DoWork()
#10 0x55d1ec30743f base::(anonymous namespace)::WorkSourceDispatch()
#11 0x7fe55def6fc7 g_main_context_dispatch
#12 0x7fe55def7200 <unknown>
#13 0x7fe55def728c g_main_context_iteration
#14 0x55d1ec3072f2 base::MessagePumpGlib::Run()
#15 0x55d1ec31fba5 base::RunLoop::Run()
#16 0x55d1eeaedd3c content::GpuMain()
#17 0x55d1ebb80562 content::ContentMainRunnerImpl::Run()
#18 0x55d1ed68984c service_manager::Main()
#19 0x55d1eb1d6081 content::ContentMain()
#20 0x55d1ea8e316b main
#21 0x7fe559a992b1 __libc_start_main
#22 0x55d1ea8e302a _start

Original change's description:
> Add VulkanContextProvider to RasterDecoderContextState.
> 
> Add VulkanContextProvider to RasterDecoderContextState which is needed
> to get VkDevice in shareable image. Refactor RasterDecoderContextState
> class to have seperate constructors for Vulkan and GL.
> 
> Bug: 891060
> Change-Id: I0ca7e657d33fdcfa62ab6465f7023a914610b7ce
> Reviewed-on: https://chromium-review.googlesource.com/c/1312194
> Commit-Queue: vikas soni <vikassoni@chromium.org>
> Reviewed-by: Eric Karl <ericrk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#605146}

TBR=ericrk@chromium.org,vikassoni@chromium.org

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

Bug: 891060
Change-Id: I4ceebc7033b4bd74e643a5f89bbbd6791b3eb6c0
Reviewed-on: https://chromium-review.googlesource.com/c/1318073
Reviewed-by: Peng Huang <penghuang@chromium.org>
Commit-Queue: Peng Huang <penghuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605376}
jkim-julie pushed a commit that referenced this pull request Nov 12, 2018
…orParts"

This is a reland of 14754da with some
fixes (see the ps #1..#2 diff).

The problem in the previous CL was that the LocalDB WCObserver was
sometime created despite not having a PageSignalReceiver, this caused
some access violation because this WCO relies on some signal sent by the
PSR. I've added a check to prevent creating this WCO when the PSR isn't
available (and fixed some tests to make them create one).

// Confirmed with sky@ that he's ok with me relanding this without his
// +1 as none of the files he owns have changed.
TBR=sky@chromium.org

Original change's description:
> RC: Encapsulate most of the singletons into ResourceCoordinatorParts
>
> There's several singletons / global instances in c/b/rc that are leaked
> at shutdown, this is causing some issues in runs of unit_tests because
> we end up re-using them between different test runs that should be
> independent. This is currently preventing some feature to be enabled.
>
> This CL add a new ResourceCoordinatorParts class that encapsulate all
> these objects that should be created only once, an instance of this
> class is owned by the browser process.
>
> Change-Id: Ia64618b6f47052815d861ff6820d994f8a65cc64
> Reviewed-on: https://chromium-review.googlesource.com/c/1290775
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Chris Hamilton <chrisha@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#606061}

Change-Id: I71feb4ea233d02cb5cc001c12e3cdd7f6ddd795b
Reviewed-on: https://chromium-review.googlesource.com/c/1324134
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606548}
jkim-julie pushed a commit that referenced this pull request Nov 12, 2018
…mode"

This reverts commit dcb55ce.

Reason for revert: Appears to cause crashes in LoginCursorTest.CursorHidden. See https://chromium-swarm.appspot.com/task?id=410f91874d359810&refresh=10&show_raw=1:
BrowserTestBase received signal: Segmentation fault. Backtrace:
#0 0x55c75309100f base::debug::StackTrace::StackTrace()
#1 0x55c752aa8075 content::(anonymous namespace)::DumpStackTraceSignalHandler()
#2 0x7f17e8b21cb0 <unknown>
#3 0x55c751991a1d chromeos::OobeUIDialogDelegate::~OobeUIDialogDelegate()
#4 0x55c751991c8e chromeos::OobeUIDialogDelegate::~OobeUIDialogDelegate()
#5 0x55c7553a6063 views::WebDialogView::OnDialogClosed()
#6 0x55c7553a5d92 views::WebDialogView::WindowClosing()
#7 0x55c752f81eb0 views::Widget::OnNativeWidgetDestroying()
#8 0x55c7548080ff views::DesktopWindowTreeHostMus::CloseNow()

Original change's description:
> Fix a crash when switching to tablet mode in Unified Desktop mode
> 
> 1) The Home Launcher used to use the wrong display ID when in
>    Unified Desktop mode.
> 2) If (1) is fixed, we hit https://crbug.com/902601. The captive
>    portal dialog widget used to be leaked and never destroyed.
> 3) If (2) is fixed, we crash on the first attempt to press the
>    app list button. The reason is tablet mode triggers a switch
>    to mirror mode. This switch happens asynchronously after the
>    Home Launcher had already been created. Switching from Unified
>    to mirror mode destroys the Unified host and the Home Launcher.
>    That's why we need to ensure that the Home Launcher is
>    recreated.
> 
> BUG=900956, 902601
> TEST=Added a test that crashes without the fix.
> 
> Change-Id: If6eb9c2255dfa9d442aa115a3274db2d8a4110d7
> Reviewed-on: https://chromium-review.googlesource.com/c/1325389
> Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
> Reviewed-by: Weidong Guo <weidongg@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Jacob Dufault <jdufault@chromium.org>
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#606687}

TBR=xiyuan@chromium.org,oshima@chromium.org,afakhry@chromium.org,jdufault@chromium.org,weidongg@chromium.org

Change-Id: I2e0cacc2c29bbc44e8e8c9dfcb86fd8106c008ff
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 900956, 902601
Reviewed-on: https://chromium-review.googlesource.com/c/1329004
Reviewed-by: Patrik Höglund <phoglund@chromium.org>
Commit-Queue: Patrik Höglund <phoglund@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606808}
jkim-julie pushed a commit that referenced this pull request Nov 19, 2018
This reverts commit 2687587.

Reason for revert: This triggers a use of initialized value on the msan bots. I believe you forgot to initialize is_removing_imm_entry_

https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8929963931985091872/+/steps/unit_tests/0/logs/ArcInputMethodManagerServiceTest.EnableIme/0

==14387==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x5638655272d9 in arc::ArcInputMethodManagerService::ImeMenuListChanged() ./../../chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_service.cc:394:7
    #1 0x56385b307d20 in arc::ArcInputMethodManagerServiceTest_EnableIme_Test::TestBody() ./../../chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_service_unittest.cc:313:14
    #2 0x56385dbb3cb2 in HandleExceptionsInMethodIfSupported<testing::Test, void> ./../../third_party/googletest/src/googletest/src/gtest.cc:0:0
    #3 0x56385dbb3cb2 in testing::Test::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2522:0
    #4 0x56385dbb7a6b in testing::TestInfo::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2698:11
    #5 0x56385dbb9559 in testing::TestCase::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2816:28
    #6 0x56385dbf2614 in testing::internal::UnitTestImpl::RunAllTests() ./../../third_party/googletest/src/googletest/src/gtest.cc:5182:43
    #7 0x56385dbf0ee7 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> ./../../third_party/googletest/src/googletest/src/gtest.cc:0:0
    #8 0x56385dbf0ee7 in testing::UnitTest::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:4791:0
    #9 0x56386b81af20 in RUN_ALL_TESTS ./../../third_party/googletest/src/googletest/include/gtest/gtest.h:2333:46
    #10 0x56386b81af20 in base::TestSuite::Run() ./../../base/test/test_suite.cc:294:0
    #11 0x56386b822e8a in Run ./../../base/callback.h:99:12
    #12 0x56386b822e8a in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, bool, base::OnceCallback<void ()>) ./../../base/test/launcher/unit_test_launcher.cc:225:0
    #13 0x56386b822607 in base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) ./../../base/test/launcher/unit_test_launcher.cc:575:10
    #14 0x56386b7e9154 in main ./../../chrome/test/base/run_all_unittests.cc:30:10
    #15 0x7fd6f88f4f44 in __libc_start_main ??:0:0
    #16 0x5638521e2b09 in _start ??:0:0

Original change's description:
> Disable Android IMEs according to given ImeInfo.
> 
> Android IMEs can be disabled in Android side by using 'ime' command.
> This CL ensures that disabled IMEs are also disabled in Chrome OS's
> InputMethodManager.
> 
> Bug: b/119274469
> Change-Id: I46c2996a41327221470d69b778da2b7270c73cd2
> Reviewed-on: https://chromium-review.googlesource.com/c/1331294
> Reviewed-by: Yusuke Sato <yusukes@chromium.org>
> Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#607473}

TBR=yusukes@chromium.org,yhanada@chromium.org

Change-Id: I5fd93607e711cd90edc4eb0ccf656b4c82ad5555
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b/119274469
Reviewed-on: https://chromium-review.googlesource.com/c/1333892
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607610}
jkim-julie pushed a commit that referenced this pull request Nov 19, 2018
… Chrome OS."

This reverts commit 605cb27.

Reason for revert: causes a crash immediately on initialization on ChromeOS Linux build:

[85464:85464:1113/105356.761188:FATAL:shell.cc(281)] Check failed: instance_. 
#0 0x7f6445c3668f base::debug::StackTrace::StackTrace()
#1 0x7f6445b6624b logging::LogMessage::~LogMessage()
#2 0x7f643f32b5c2 ash::Shell::Get()
#3 0x561d123191ba ChromeBrowserMainExtraPartsAsh::PreProfileInit()
#4 0x561d10f06f2a ChromeBrowserMainParts::PreProfileInit()
#5 0x561d10f07c8e ChromeBrowserMainPartsLinux::PreProfileInit()
#6 0x561d1089bc26 chromeos::ChromeBrowserMainPartsChromeos::PreProfileInit()
#7 0x561d10f05fee ChromeBrowserMainParts::PreMainMessageLoopRunImpl()
#8 0x561d10f05925 ChromeBrowserMainParts::PreMainMessageLoopRun()
#9 0x561d1089b755 chromeos::ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun()
#10 0x7f64434b766a content::BrowserMainLoop::PreMainMessageLoopRun()
#11 0x7f64439b1055 content::StartupTaskRunner::RunAllTasksNow()
#12 0x7f64434b6191 content::BrowserMainLoop::CreateStartupTasks()
#13 0x7f64434b9d30 content::BrowserMainRunnerImpl::Initialize()
#14 0x7f64434b3d62 content::BrowserMain()
#15 0x7f6443fed756 content::ContentMainRunnerImpl::Run()
#16 0x7f6434c3ff46 service_manager::Main()
#17 0x7f6443feba44 content::ContentMain()
#18 0x561d10094e63 ChromeMain
#19 0x7f64352042b1 __libc_start_main
#20 0x561d10094cda _start

Original change's description:
> ui_devtools: Encapsulate views server init; use Shell Env for Chrome OS.
> 
> Use the ash::Shell aura::Env in single-process mash ui_devtools.
> Keep a TODO for initializing ui_devtools in Ash for multi-process Mash.
> Encapsulate server init, and the switch and port definitions.
> 
> Bug: 896977
> Test: ui_devtools works well on Chrome OS single-process Mash.
> Change-Id: I3cc285d5f8ee465d45795a3521e7e4eafa354414
> Reviewed-on: https://chromium-review.googlesource.com/c/1324373
> Commit-Queue: Michael Wasserman <msw@chromium.org>
> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#607408}

TBR=sadrul@chromium.org,msw@chromium.org

Change-Id: Iae50c4a7d67de9602596b0522da3fec0f472e092
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 896977
Reviewed-on: https://chromium-review.googlesource.com/c/1334001
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607689}
jkim-julie pushed a commit that referenced this pull request Nov 26, 2018
Tests involving TaskScheduler accessed the members of
SameThreadTaskSource from different threads without synchronization.
This CL fixes the issue by making sure that the initial invocation of
TestTask() happens on the same thread as other invocations.

TSAN report before this CL:

WARNING: ThreadSanitizer: data race (pid=126426)
  Write of size 4 at 0x7b1400001c0c by thread T4:
    #0 base::sequence_manager::SameThreadTaskSource::TestTask() base/task/sequence_manager/sequence_manager_perftest.cc:311:25
    #1 Invoke<void (base::sequence_manager::SameThreadTaskSource::*)(), base::sequence_manager::SameThreadTaskSource *> base/bind_internal.h:516:12
    #2 MakeItSo<void (base::sequence_manager::SameThreadTaskSource::*const &)(), base::sequence_manager::SameThreadTaskSource *> base/bind_internal.h:616
    #3 RunImpl<void (base::sequence_manager::SameThreadTaskSource::*const &)(), const std::__1::tuple<base::internal::UnretainedWrapper<base::sequence_manager::SameThreadTaskSource> > &, 0> base/bind_internal.h:689
    #4 base::internal::Invoker<base::internal::BindState<void (base::sequence_manager::SameThreadTaskSource::*)(), base::internal::UnretainedWrapper<base::sequence_manager::SameThreadTaskSource> >, void ()>::Run(base::internal::BindStateBase*) base/bind_internal.h:671
    #5 Run base/callback.h:99:12
    #6 base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) base/debug/task_annotator.cc:99
    #7 base::internal::TaskTracker::RunOrSkipTask(base::internal::Task, base::internal::Sequence*, bool) base/task/task_scheduler/task_tracker.cc:641:23
    #8 base::internal::TaskTrackerPosix::RunOrSkipTask(base::internal::Task, base::internal::Sequence*, bool) base/task/task_scheduler/task_tracker_posix.cc:23:16
    #9 base::internal::TaskTracker::RunAndPopNextTask(scoped_refptr<base::internal::Sequence>, base::internal::CanScheduleSequenceObserver*) base/task/task_scheduler/task_tracker.cc:496:3
    #10 base::internal::SchedulerWorker::RunWorker() base/task/task_scheduler/scheduler_worker.cc:333:24
    #11 base::internal::SchedulerWorker::RunSharedWorker() base/task/task_scheduler/scheduler_worker.cc:237:3
    #12 base::internal::SchedulerWorker::ThreadMain() base/task/task_scheduler/scheduler_worker.cc:207:7
    #13 base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:81:13

  Previous write of size 4 at 0x7b1400001c0c by main thread:
    #0 TestTask base/task/sequence_manager/sequence_manager_perftest.cc:330:27
    #1 base::sequence_manager::SameThreadTaskSource::Start() base/task/sequence_manager/sequence_manager_perftest.cc:297
    #2 base::sequence_manager::SingleThreadImmediateTestCase::Start() base/task/sequence_manager/sequence_manager_perftest.cc:408:41
    #3 base::sequence_manager::SequenceManagerPerfTest::Benchmark(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, base::sequence_manager::TestCase*) base/task/sequence_manager/sequence_manager_perftest.cc:636:15
    #4 base::sequence_manager::SequenceManagerPerfTest_PostImmediateTasks_OneQueue_Test::TestBody() base/task/sequence_manager/sequence_manager_perftest.cc:714:3
    #5 HandleExceptionsInMethodIfSupported<testing::Test, void> third_party/googletest/src/googletest/src/gtest.cc
    #6 testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc:2522
    #7 testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2703:11
    #8 testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2825:28
    #9 testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:5227:43
    #10 HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> third_party/googletest/src/googletest/src/gtest.cc
    #11 testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4835
    #12 RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2369:46
    #13 base::TestSuite::Run() base/test/test_suite.cc:294
    #14 main base/test/run_all_perftests.cc:8:42

TSAN report after this CL:
  No races found.

Change-Id: I03b816a01fe33eed86966f89cb1ad1b9ab34c2e3
Reviewed-on: https://chromium-review.googlesource.com/c/1340534
Reviewed-by: Alex Clarke <alexclarke@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609276}
jkim-julie pushed a commit that referenced this pull request Dec 10, 2018
… file"

This reverts commit aa21b3f.

Reason for revert: Suspect of introducing consistent failure on Mac ASAN 64 tests.

First failed run: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac%20ASan%2064%20Tests%20%281%29/46988

Sample logs:
unit_tests Run on OS: 'Mac-10.13'
Shard duration: 0:07:56.378119
failures:
FileAnalyzerTest.ArchivedArchiveSetForZipNoArchive
FileAnalyzerTest.ArchivedExecutableFalseForZipNoExecutable
DownloadProtectionServiceTest.CheckClientDownloadZip
SandboxedZipAnalyzerTest.ZippedAppWithUnsignedAndSignedExecutable
SandboxedZipAnalyzerTest.NoBinaries
FileAnalyzerTest.ArchivedBinariesSkipsSafeFiles

==34531==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x700015467c14 at pc 0x00013e99c44d bp 0x700015467950 sp 0x7000154670f8
READ of size 5 at 0x700015467c14 thread T16
    #0 0x13e99c44c in __asan_after_dynamic_init ??:0:0
    #1 0x10e3ac931 in safe_browsing::UpdateArchiveAnalyzerResultsWithFile(base::FilePath, base::File*, bool, safe_browsing::ArchiveAnalyzerResults*) ??:0:0
    #2 0x10e3a6707 in safe_browsing::zip_analyzer::AnalyzeZipFile(base::File, base::File, safe_browsing::ArchiveAnalyzerResults*) ??:0:0
    #3 0x115236586 in SafeArchiveAnalyzer::AnalyzeZipFile(base::File, base::File, base::OnceCallback<void (safe_browsing::ArchiveAnalyzerResults const&)>) ??:0:0
    #4 0x10bc0b332 in chrome::mojom::SafeArchiveAnalyzerStubDispatch::AcceptWithResponder(chrome::mojom::SafeArchiveAnalyzer*, mojo::Message*, std::__1::unique_ptr<mojo::MessageReceiverWithStatus, std::__1::default_delete<mojo::MessageReceiverWithStatus> >) ??:0:0
    #5 0x115234a40 in chrome::mojom::SafeArchiveAnalyzerStub<mojo::RawPtrImplRefTraits<chrome::mojom::SafeArchiveAnalyzer> >::AcceptWithResponder(mojo::Message*, std::__1::unique_ptr<mojo::MessageReceiverWithStatus, std::__1::default_delete<mojo::MessageReceiverWithStatus> >) ??:0:0
    #6 0x1165eeb32 in mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message*) ??:0:0
    #7 0x1165ed1e0 in mojo::FilterChain::Accept(mojo::Message*) ??:0:0
    #8 0x1165f278b in mojo::InterfaceEndpointClient::HandleIncomingMessage(mojo::Message*) ??:0:0
    #9 0x116605686 in mojo::internal::MultiplexRouter::ProcessIncomingMessage(mojo::internal::MultiplexRouter::MessageWrapper*, mojo::internal::MultiplexRouter::ClientCallBehavior, base::SequencedTaskRunner*) ??:0:0
    #10 0x11660372b in mojo::internal::MultiplexRouter::Accept(mojo::Message*) ??:0:0
    #11 0x1165ed1e0 in mojo::FilterChain::Accept(mojo::Message*) ??:0:0
    #12 0x1165dcea8 in mojo::Connector::ReadSingleMessage(unsigned int*) ??:0:0
    #13 0x1165df32a in mojo::Connector::ReadAllAvailableMessages() ??:0:0
    #14 0x1165deda1 in mojo::Connector::OnHandleReadyInternal(unsigned int) ??:0:0
    #15 0x10d429d44 in mojo::SimpleWatcher::DiscardReadyState(base::RepeatingCallback<void (unsigned int)> const&, unsigned int, mojo::HandleSignalsState const&) ??:0:0
    #16 0x11a802cf5 in mojo::SimpleWatcher::OnHandleReady(int, unsigned int, mojo::HandleSignalsState const&) ??:0:0
    #17 0x11a803cbd in void base::internal::Invoker<base::internal::BindState<void (mojo::SimpleWatcher::*)(int, unsigned int, mojo::HandleSignalsState const&), base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState>, void ()>::RunImpl<void (mojo::SimpleWatcher::* const&)(int, unsigned int, mojo::HandleSignalsState const&), std::__1::tuple<base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState> const&, 0ul, 1ul, 2ul, 3ul>(void (mojo::SimpleWatcher::* const&)(int, unsigned int, mojo::HandleSignalsState const&), std::__1::tuple<base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState> const&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul>) ??:0:0
    #18 0x11890131c in base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) ??:0:0
    #19 0x118995271 in base::MessageLoopImpl::RunTask(base::PendingTask*) ??:0:0
    #20 0x11899691f in base::MessageLoopImpl::DoWork() ??:0:0
    #21 0x118999eb3 in base::MessagePumpDefault::Run(base::MessagePump::Delegate*) ??:0:0
    #22 0x118994259 in base::MessageLoopImpl::Run(bool) ??:0:0
    #23 0x118a49a1c in base::RunLoop::Run() ??:0:0
    #24 0x118b962e2 in base::Thread::Run(base::RunLoop*) ??:0:0
    #25 0x118b96e9d in base::Thread::ThreadMain() ??:0:0
    #26 0x118c72a0d in base::(anonymous namespace)::ThreadFunc(void*) ??:0:0
    #27 0x7fff74564660 in _pthread_body ??:0:0
    #28 0x7fff7456450c in _pthread_start ??:0:0


Original change's description:
> Add util method to update ArchiveAnalyzerResults for a single file
> 
> This CL breaks out the code that inspects an individual file within a
> ZIP archive. This will be shared between the ZIP and RAR inspection,
> when RAR files begin doing content inspection.
> 
> Bug: 909778
> Change-Id: I7acf1cabd472f112f2ed7c31735688cae7a6d122
> Reviewed-on: https://chromium-review.googlesource.com/c/1354103
> Commit-Queue: Daniel Rubery <drubery@chromium.org>
> Reviewed-by: Brian White <bcwhite@chromium.org>
> Reviewed-by: Jay Civelli <jcivelli@chromium.org>
> Reviewed-by: Varun Khaneja <vakh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#612799}

TBR=jcivelli@chromium.org,bcwhite@chromium.org,vakh@chromium.org,drubery@chromium.org

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

Bug: 909778
Change-Id: I92e193707da4a28cee94aa1277a6ca567650760f
Reviewed-on: https://chromium-review.googlesource.com/c/1358491
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613073}
jkim-julie pushed a commit that referenced this pull request Dec 10, 2018
…ure"

This reverts commit 3f40244.

Reason for revert: Caused a data race running components_unittests with ScopedFeatureList.

Sample failure: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20TSan%20Tests/30398

WARNING: ThreadSanitizer: data race (pid=9805)
  Write of size 8 at 0x55b63bfba2b0 by main thread:
    #0 base::FeatureList::ClearInstanceForTesting() base/feature_list.cc:284:27 (components_unittests+0x85bec63)
    #1 base::test::ScopedFeatureList::~ScopedFeatureList() base/test/scoped_feature_list.cc:98:3 (components_unittests+0x9a65d52)
    #2 content::UnitTestTestSuite::~UnitTestTestSuite() content/public/test/unittest_test_suite.cc:61:1 (components_unittests+0xb03cf21)
    #3 operator() buildtools/third_party/libc++/trunk/include/memory:2325:5 (components_unittests+0x8256779)
    #4 reset buildtools/third_party/libc++/trunk/include/memory:2638 (components_unittests+0x8256779)
    #5 ~unique_ptr buildtools/third_party/libc++/trunk/include/memory:2592 (components_unittests+0x8256779)
    #6 ~__tuple_leaf buildtools/third_party/libc++/trunk/include/tuple:171 (components_unittests+0x8256779)
    #7 ~tuple buildtools/third_party/libc++/trunk/include/tuple:470 (components_unittests+0x8256779)
    #8 ~BindState base/bind_internal.h:871 (components_unittests+0x8256779)
    #9 base::internal::BindState<int (content::UnitTestTestSuite::*)(), std::__1::unique_ptr<content::UnitTestTestSuite, std::__1::default_delete<content::UnitTestTestSuite> > >::Destroy(base::internal::BindStateBase const*) base/bind_internal.h:874 (components_unittests+0x8256779)
    #10 Destruct base/callback_internal.cc:29:3 (components_unittests+0x85b80c7)
    #11 Release base/memory/ref_counted.h:403 (components_unittests+0x85b80c7)
    #12 Release base/memory/scoped_refptr.h:284 (components_unittests+0x85b80c7)
    #13 ~scoped_refptr base/memory/scoped_refptr.h:208 (components_unittests+0x85b80c7)
    #14 base::internal::CallbackBase::~CallbackBase() base/callback_internal.cc:84 (components_unittests+0x85b80c7)
    #15 base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) base/test/launcher/unit_test_launcher.cc:575:3 (components_unittests+0x9a71272)
    #16 main components/test/run_all_unittests.cc:8:10 (components_unittests+0x4e49ad5)

  Previous read of size 8 at 0x55b63bfba2b0 by thread T12 (mutexes: write M19294):
    #0 base::FeatureList::IsEnabled(base::Feature const&) base/feature_list.cc:200:8 (components_unittests+0x85be82d)
    #1 CanCleanupLockRequired base/task/task_scheduler/scheduler_worker_pool_impl.cc:672:12 (components_unittests+0x866a5e8)
    #2 base::internal::SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::GetWork(base::internal::SchedulerWorker*) base/task/task_scheduler/scheduler_worker_pool_impl.cc:538 (components_unittests+0x866a5e8)
    #3 base::internal::SchedulerWorker::RunWorker() base/task/task_scheduler/scheduler_worker.cc:324:51 (components_unittests+0x866f81d)
    #4 base::internal::SchedulerWorker::RunPooledWorker() base/task/task_scheduler/scheduler_worker.cc:229:3 (components_unittests+0x866f481)
    #5 base::internal::SchedulerWorker::ThreadMain() base/task/task_scheduler/scheduler_worker.cc:208:7 (components_unittests+0x866f2f1)
    #6 base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:81:13 (components_unittests+0x86daf04)

  Location is global 'base::(anonymous namespace)::g_feature_list_instance' of size 8 at 0x55b63bfba2b0 (components_unittests+0x00000ebe52b0)

Original change's description:
> [TaskScheduler]: Create no detach below initial capacity feature
> 
> Under this experiment, scheduler workers are only detached if the pool is
> above its initial capacity (threads that are created to replace blocked threads).
> 
> 2 options were considered:
> Option A: Detach only when over initial capacity.
> 
> Option B: Detach only when over current capacity (includes currently blocked threads in capacity).
> This might better handle the following case: At any given time, there is at least 1 blocked thread.
> On top of that, some periodic work uses all worker every 30s or so. The current capacity will
> encompass for the blocked thread and avoid detaching it periodically.
> 
> Option A was picked because it is more conservative. Initial capacity is smaller or
> equal to current capacity, so detaching is closer to current behavior. We want to avoid having
> too many threads that aren't used.
> 
> Bug: 847501
> Change-Id: I0b116db54095767768b158d92f5f146249720b45
> Reviewed-on: https://chromium-review.googlesource.com/c/1348863
> Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#612698}

TBR=fdoray@chromium.org,etiennep@chromium.org

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

Bug: 847501
Change-Id: I951f5c5701e2d296b2c4edef37648105c4911cf9
Reviewed-on: https://chromium-review.googlesource.com/c/1359127
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613229}
jkim-julie pushed a commit that referenced this pull request Dec 10, 2018
Reason for revert: the failing test was flaky without this change and has since been disabled (crbug.com/911154)

Original change's description:
> Revert "cros: Remove WizardInProcessBrowserTest"
> 
> This reverts commit 92925c6.
> 
> Reason for revert: WizardControllerDeviceState*Test timing out on linux-chromeos-dbg
> 
> https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-chromeos-dbg/9248
> 
> rowserTestBase received signal: Terminated. Backtrace:
> #0 0x7feb2e8b3fed base::debug::StackTrace::StackTrace()
> #1 0x7feb2e5addfa base::debug::StackTrace::StackTrace()
> #2 0x563fac6dc0f2 content::(anonymous namespace)::DumpStackTraceSignalHandler()
> #3 0x7feafb349cb0 <unknown>
> #4 0x7feafb4116d3 epoll_wait
> #5 0x7feb2e96353f epoll_dispatch
> #6 0x7feb2e956cc5 event_base_loop
> #7 0x7feb2e9073c9 base::MessagePumpLibevent::Run()
> #8 0x7feb2e63fbe6 base::MessageLoopImpl::Run()
> #9 0x7feb2e6eb322 base::RunLoop::Run()
> #10 0x563fa6018304 chromeos::WizardControllerDeviceStateTest::WaitForAutoEnrollmentState()
> #11 0x563fa5ffeb8f chromeos::WizardControllerDeviceStateExplicitRequirementTest_ControlFlowForcedReEnrollment_Test::RunTestOnMainThread()
> 
> Original change's description:
> > cros: Remove WizardInProcessBrowserTest
> > 
> > The base class convoluted the inheritance structure and it is simpler to
> > directly derive from InProcessBrowserTest.
> > 
> > Bug: 899777
> > Change-Id: I04b8e4d48d0a28a8d4481891d5379456df32d0d6
> > Reviewed-on: https://chromium-review.googlesource.com/c/1345244
> > Commit-Queue: Jacob Dufault <jdufault@chromium.org>
> > Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
> > Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#612394}
> 
> TBR=xiyuan@chromium.org,emaxx@chromium.org,jdufault@chromium.org
> 
> Change-Id: I06cfea778c8246795a8bb8fdc9878812a1ab5d52
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 899777
> Reviewed-on: https://chromium-review.googlesource.com/c/1356168
> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
> Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#612523}

TBR=xiyuan@chromium.org,emaxx@chromium.org,jdufault@chromium.org,ortuno@chromium.org

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

Bug: 899777
Change-Id: Id6a54d588736969830db39963f5cc4560d1068b9
Reviewed-on: https://chromium-review.googlesource.com/c/1359135
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Commit-Queue: Jacob Dufault <jdufault@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613244}
jkim-julie pushed a commit that referenced this pull request Dec 10, 2018
This reverts commit 5684820.

Reason for revert: WizardControllerDeviceStateTest.ControlFlowDeviceDisabled timing out on linux-chromeos-dbg

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-chromeos-dbg/9330

Original change's description:
> Reland "cros: Remove WizardInProcessBrowserTest"
> 
> Reason for revert: the failing test was flaky without this change and has since been disabled (crbug.com/911154)
> 
> Original change's description:
> > Revert "cros: Remove WizardInProcessBrowserTest"
> > 
> > This reverts commit 92925c6.
> > 
> > Reason for revert: WizardControllerDeviceState*Test timing out on linux-chromeos-dbg
> > 
> > https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-chromeos-dbg/9248
> > 
> > rowserTestBase received signal: Terminated. Backtrace:
> > #0 0x7feb2e8b3fed base::debug::StackTrace::StackTrace()
> > #1 0x7feb2e5addfa base::debug::StackTrace::StackTrace()
> > #2 0x563fac6dc0f2 content::(anonymous namespace)::DumpStackTraceSignalHandler()
> > #3 0x7feafb349cb0 <unknown>
> > #4 0x7feafb4116d3 epoll_wait
> > #5 0x7feb2e96353f epoll_dispatch
> > #6 0x7feb2e956cc5 event_base_loop
> > #7 0x7feb2e9073c9 base::MessagePumpLibevent::Run()
> > #8 0x7feb2e63fbe6 base::MessageLoopImpl::Run()
> > #9 0x7feb2e6eb322 base::RunLoop::Run()
> > #10 0x563fa6018304 chromeos::WizardControllerDeviceStateTest::WaitForAutoEnrollmentState()
> > #11 0x563fa5ffeb8f chromeos::WizardControllerDeviceStateExplicitRequirementTest_ControlFlowForcedReEnrollment_Test::RunTestOnMainThread()
> > 
> > Original change's description:
> > > cros: Remove WizardInProcessBrowserTest
> > > 
> > > The base class convoluted the inheritance structure and it is simpler to
> > > directly derive from InProcessBrowserTest.
> > > 
> > > Bug: 899777
> > > Change-Id: I04b8e4d48d0a28a8d4481891d5379456df32d0d6
> > > Reviewed-on: https://chromium-review.googlesource.com/c/1345244
> > > Commit-Queue: Jacob Dufault <jdufault@chromium.org>
> > > Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
> > > Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#612394}
> > 
> > TBR=xiyuan@chromium.org,emaxx@chromium.org,jdufault@chromium.org
> > 
> > Change-Id: I06cfea778c8246795a8bb8fdc9878812a1ab5d52
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug: 899777
> > Reviewed-on: https://chromium-review.googlesource.com/c/1356168
> > Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
> > Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#612523}
> 
> TBR=xiyuan@chromium.org,emaxx@chromium.org,jdufault@chromium.org,ortuno@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug: 899777
> Change-Id: Id6a54d588736969830db39963f5cc4560d1068b9
> Reviewed-on: https://chromium-review.googlesource.com/c/1359135
> Reviewed-by: Jacob Dufault <jdufault@chromium.org>
> Commit-Queue: Jacob Dufault <jdufault@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#613244}

TBR=xiyuan@chromium.org,emaxx@chromium.org,jdufault@chromium.org,ortuno@chromium.org

Change-Id: Ifeec16d8f954fd9a9aede38a2fa0bffcc107dab4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 899777
Reviewed-on: https://chromium-review.googlesource.com/c/1359815
Reviewed-by: Ovidio Henriquez <odejesush@chromium.org>
Commit-Queue: Ovidio Henriquez <odejesush@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613359}
jkim-julie pushed a commit that referenced this pull request Dec 10, 2018
This reverts commit 3bb44fe.

Reason for revert: Suspect of introducing consistent failure on 
 Linux Chromium OS ASan LSan Tests (1) bot.

First failure:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/30399

Sample logs:
views_mus_unittests Run on OS: 'Ubuntu-14.04'
Shard duration: 0:01:41.022687
failures:
AXTreeSourceViewsRootTest.Serialize
AXTreeSourceViewsRootTest.Focus
AXTreeSourceViewsRootTest.Accessors
AXTreeSourceViewsRootTest.SerializeWindowSetsClipsChildren
AXTreeSourceViewsRootTest.DoDefault

  ==7985==ERROR: LeakSanitizer: detected memory leaks
  Direct leak of 56 byte(s) in 1 object(s) allocated from:
      #0 0x55ec150a7ed2 in operator new(unsigned long) /b/swarming/w/ir/kitchen-workdir/src/third_party/llvm/compiler-rt/lib/asan/asan_new_delete.cc:106:3
      #1 0x55ec158f3f9e in ui::AXTreeSerializer<views::AXAuraObjWrapper*, ui::AXNodeData, ui::AXTreeData>::SerializeChangedNodes(views::AXAuraObjWrapper*, ui::AXTreeUpdateBase<ui::AXNodeData, ui::AXTreeData>*) ui/accessibility/ax_tree_serializer.h:598:35
      #2 0x55ec158f41c2 in ui::AXTreeSerializer<views::AXAuraObjWrapper*, ui::AXNodeData, ui::AXTreeData>::SerializeChangedNodes(views::AXAuraObjWrapper*, ui::AXTreeUpdateBase<ui::AXNodeData, ui::AXTreeData>*) ui/accessibility/ax_tree_serializer.h:604:12
      #3 0x55ec158f41c2 in ui::AXTreeSerializer<views::AXAuraObjWrapper*, ui::AXNodeData, ui::AXTreeData>::SerializeChangedNodes(views::AXAuraObjWrapper*, ui::AXTreeUpdateBase<ui::AXNodeData, ui::AXTreeData>*) ui/accessibility/ax_tree_serializer.h:604:12
      #4 0x55ec158f09de in ui::AXTreeSerializer<views::AXAuraObjWrapper*, ui::AXNodeData, ui::AXTreeData>::SerializeChanges(views::AXAuraObjWrapper*, ui::AXTreeUpdateBase<ui::AXNodeData, ui::AXTreeData>*) ui/accessibility/ax_tree_serializer.h:422:8
      #5 0x55ec158ee9d9 in views::(anonymous namespace)::AXTreeSourceViewsRootTest_Serialize_Test::TestBody() ui/views/accessibility/ax_tree_source_views_unittest.cc:318:17
      #6 0x55ec17bdd342 in HandleExceptionsInMethodIfSupported<testing::Test, void> third_party/googletest/src/googletest/src/gtest.cc
      #7 0x55ec17bdd342 in testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc:2522
      #8 0x55ec17bdf428 in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2703:11
      #9 0x55ec17be08e6 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2825:28
      #10 0x55ec17c08f46 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:5227:43
      #11 0x55ec17c082c5 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> third_party/googletest/src/googletest/src/gtest.cc
      #12 0x55ec17c082c5 in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4835
      #13 0x55ec180eb7da in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2369:46
      #14 0x55ec180eb7da in base::TestSuite::Run() base/test/test_suite.cc:294
      #15 0x55ec180f1a34 in Run base/callback.h:99:12
      #16 0x55ec180f1a34 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, bool, base::OnceCallback<void ()>) base/test/launcher/unit_test_launcher.cc:225
      #17 0x55ec180f1500 in base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) base/test/launcher/unit_test_launcher.cc:575:10
      #18 0x55ec15220e7e in views::ViewsTestSuite::RunTests() ui/views/views_test_suite.cc:33:10
      #19 0x55ec150f46a3 in main ui/views/mus/run_all_unittests_mus.cc:8:47
      #20 0x7fd6ce976f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
  Indirect leak of 8 byte(s) in 1 object(s) allocated from:
      #0 0x55ec150a7ed2 in operator new(unsigned long) /b/swarming/w/ir/kitchen-workdir/src/third_party/llvm/compiler-rt/lib/asan/asan_new_delete.cc:106:3
      #1 0x55ec158f6f15 in __libcpp_allocate buildtools/third_party/libc++/trunk/include/new:254:10
      #2 0x55ec158f6f15 in allocate buildtools/third_party/libc++/trunk/include/memory:1800
      #3 0x55ec158f6f15 in allocate buildtools/third_party/libc++/trunk/include/memory:1549
      #4 0x55ec158f6f15 in __split_buffer buildtools/third_party/libc++/trunk/include/__split_buffer:311
      #5 0x55ec158f6f15 in std::__1::vector<ui::ClientTreeNode*, std::__1::allocator<ui::ClientTreeNode*> >::reserve(unsigned long) buildtools/third_party/libc++/trunk/include/vector:1576
      #6 0x55ec158f3a1b in ui::AXTreeSerializer<views::AXAuraObjWrapper*, ui::AXNodeData, ui::AXTreeData>::SerializeChangedNodes(views::AXAuraObjWrapper*, ui::AXTreeUpdateBase<ui::AXNodeData, ui::AXTreeData>*) ui/accessibility/ax_tree_serializer.h:573:25
      #7 0x55ec158f41c2 in ui::AXTreeSerializer<views::AXAuraObjWrapper*, ui::AXNodeData, ui::AXTreeData>::SerializeChangedNodes(views::AXAuraObjWrapper*, ui::AXTreeUpdateBase<ui::AXNodeData, ui::AXTreeData>*) ui/accessibility/ax_tree_serializer.h:604:12
      #8 0x55ec158f41c2 in ui::AXTreeSerializer<views::AXAuraObjWrapper*, ui::AXNodeData, ui::AXTreeData>::SerializeChangedNodes(views::AXAuraObjWrapper*, ui::AXTreeUpdateBase<ui::AXNodeData, ui::AXTreeData>*) ui/accessibility/ax_tree_serializer.h:604:12
      #9 0x55ec158f41c2 in ui::AXTreeSerializer<views::AXAuraObjWrapper*, ui::AXNodeData, ui::AXTreeData>::SerializeChangedNodes(views::AXAuraObjWrapper*, ui::AXTreeUpdateBase<ui::AXNodeData, ui::AXTreeData>*) ui/accessibility/ax_tree_serializer.h:604:12
      #10 0x55ec158f09de in ui::AXTreeSerializer<views::AXAuraObjWrapper*, ui::AXNodeData, ui::AXTreeData>::SerializeChanges(views::AXAuraObjWrapper*, ui::AXTreeUpdateBase<ui::AXNodeData, ui::AXTreeData>*) ui/accessibility/ax_tree_serializer.h:422:8
      #11 0x55ec158ee9d9 in views::(anonymous namespace)::AXTreeSourceViewsRootTest_Serialize_Test::TestBody() ui/views/accessibility/ax_tree_source_views_unittest.cc:318:17
      #12 0x55ec17bdd342 in HandleExceptionsInMethodIfSupported<testing::Test, void> third_party/googletest/src/googletest/src/gtest.cc
      #13 0x55ec17bdd342 in testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc:2522
      #14 0x55ec17bdf428 in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2703:11
      #15 0x55ec17be08e6 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2825:28
      #16 0x55ec17c08f46 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:5227:43
      #17 0x55ec17c082c5 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> third_party/googletest/src/googletest/src/gtest.cc
      #18 0x55ec17c082c5 in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4835
      #19 0x55ec180eb7da in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2369:46
      #20 0x55ec180eb7da in base::TestSuite::Run() base/test/test_suite.cc:294
      #21 0x55ec180f1a34 in Run base/callback.h:99:12
      #22 0x55ec180f1a34 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, bool, base::OnceCallback<void ()>) base/test/launcher/unit_test_launcher.cc:225
      #23 0x55ec180f1500 in base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) base/test/launcher/unit_test_launcher.cc:575:10
      #24 0x55ec15220e7e in views::ViewsTestSuite::RunTests() ui/views/views_test_suite.cc:33:10
      #25 0x55ec150f46a3 in main ui/views/mus/run_all_unittests_mus.cc:8:47
      #26 0x7fd6ce976f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)



Original change's description:
> Move AXTreeSourceAura tests into AXTreeSourceViews tests
> 
> AXTreeSourceAura doesn't exist any more. The tests exercise the
> integration of AXRootObjWrapper with AXTreeSourceViews, so rename the
> tests ot AXTreeSourceViewsRootTest.
> 
> Note that the tests lived in //chrome/browser/ui/ash. If we need an
> ash-specific tree source we could introduce an AXTreeSourceAsh either
> in //ash (for mash) or //chrome/browser/ui/ash (for SingleProcessMash).
> 
> Bug: 910672
> Test: views_unittests
> Change-Id: I79438345e3ad9bd1aa69d74b3c34cc35efe87142
> Reviewed-on: https://chromium-review.googlesource.com/c/1358972
> Reviewed-by: David Tseng <dtseng@chromium.org>
> Commit-Queue: James Cook <jamescook@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#613308}

TBR=jamescook@chromium.org,dtseng@chromium.org

Change-Id: If77dfe8f468d8f5e8514e2dbe57c9aff3cbf5a3e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 910672
Reviewed-on: https://chromium-review.googlesource.com/c/1360591
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613487}
jkim-julie pushed a commit that referenced this pull request Dec 10, 2018
Data race happens between SetBehindCaptivePortal on BrowserThread::IO
and CaptivePortalBrowserTest::OnIntercept line 981 on main thread

Change-Id: I58163d96f66f8ca3b2d257d1481b0d12f980c679

ThreadSanitizer Report

WARNING: ThreadSanitizer: data race (pid=37545)
  Read of size 1 at 0x7b44000004d8 by main thread:
    #0 CaptivePortalBrowserTest::OnIntercept(content::URLLoaderInterceptor::RequestParams*) /src/chrome/browser/captive_portal/captive_portal_browsertest.cc:984:9 (browser_tests+0x4ea3f84)
    #1 Invoke<bool (CaptivePortalBrowserTest::*)(content::URLLoaderInterceptor::RequestParams *), CaptivePortalBrowserTest *, content::URLLoaderInterceptor::RequestParams *> /src/base/bind_internal.h:507:12 (browser_tests+0x4ed3a2f)
    #2 MakeItSo<bool (CaptivePortalBrowserTest::*const &)(content::URLLoaderInterceptor::RequestParams *), CaptivePortalBrowserTest *, content::URLLoaderInterceptor::RequestParams *> /src/base/bind_internal.h:607 (browser_tests+0x4ed3a2f)
    #3 RunImpl<bool (CaptivePortalBrowserTest::*const &)(content::URLLoaderInterceptor::RequestParams *), const std::__1::tuple<base::internal::UnretainedWrapper<CaptivePortalBrowserTest> > &, 0> /src/base/bind_internal.h:680 (browser_tests+0x4ed3a2f)
    #4 base::internal::Invoker<base::internal::BindState<bool (CaptivePortalBrowserTest::*)(content::URLLoaderInterceptor::RequestParams*), base::internal::UnretainedWrapper<CaptivePortalBrowserTest> >, bool (content::URLLoaderInterceptor::RequestParams*)>::Run(base::internal::BindStateBase*, content::URLLoaderInterceptor::RequestParams*) /src/base/bind_internal.h:662 (browser_tests+0x4ed3a2f)
    #5 Run /src/base/callback.h:129:12 (browser_tests+0xa8cbbcc)
    #6 content::URLLoaderInterceptor::Intercept(content::URLLoaderInterceptor::RequestParams*) /src/content/public/test/url_loader_interceptor.cc:377 (browser_tests+0xa8cbbcc)
    #7 content::URLLoaderInterceptor::Interceptor::CreateLoaderAndStart(mojo::InterfaceRequest<network::mojom::URLLoader>, int, int, unsigned int, network::ResourceRequest const&, mojo::InterfacePtr<network::mojom::URLLoaderClient>, net::MutableNetworkTrafficAnnotationTag const&) /src/content/public/test/url_loader_interceptor.cc:92:18 (browser_tests+0xa8cd294)
    #8 network::mojom::URLLoaderFactoryProxy_CreateLoaderAndStart_Message::Dispatch(network::mojom::URLLoaderFactory*) /src/out/Default/gen/services/network/public/mojom/url_loader_factory.mojom.cc:145:11 (browser_tests+0x581c781)
    #9 network::mojom::URLLoaderFactoryStubDispatch::Accept(network::mojom::URLLoaderFactory*, mojo::Message*) /src/out/Default/gen/services/network/public/mojom/url_loader_factory.mojom.cc:353:20 (browser_tests+0x581c626)
    #10 network::mojom::URLLoaderFactoryStub<mojo::RawPtrImplRefTraits<network::mojom::URLLoaderFactory> >::Accept(mojo::Message*) /src/out/Default/gen/services/network/public/mojom/url_loader_factory.mojom.h:155:12 (browser_tests+0x69ccb3f)
    #11 mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message*) /src/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:423:32 (browser_tests+0xb97e563)
    #12 mojo::InterfaceEndpointClient::HandleIncomingMessageThunk::Accept(mojo::Message*) /src/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:133:18 (browser_tests+0xb97df6a)
    #13 mojo::FilterChain::Accept(mojo::Message*) /src/mojo/public/cpp/bindings/lib/filter_chain.cc:40:17 (browser_tests+0xb9823ba)
    #14 mojo::InterfaceEndpointClient::HandleIncomingMessage(mojo::Message*) /src/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:306:19 (browser_tests+0xb9801ba)
    #15 mojo::internal::MultiplexRouter::ProcessIncomingMessage(mojo::internal::MultiplexRouter::MessageWrapper*, mojo::internal::MultiplexRouter::ClientCallBehavior, base::SequencedTaskRunner*) /src/mojo/public/cpp/bindings/lib/multiplex_router.cc:869:42 (browser_tests+0xb98ba4b)
    #16 mojo::internal::MultiplexRouter::Accept(mojo::Message*) /src/mojo/public/cpp/bindings/lib/multiplex_router.cc:590:38 (browser_tests+0xb98a86a)
    #17 mojo::FilterChain::Accept(mojo::Message*) /src/mojo/public/cpp/bindings/lib/filter_chain.cc:40:17 (browser_tests+0xb9823ba)
    #18 mojo::Connector::ReadSingleMessage(unsigned int*) /src/mojo/public/cpp/bindings/lib/connector.cc:457:51 (browser_tests+0xb97a2be)
    #19 mojo::Connector::ReadAllAvailableMessages() /src/mojo/public/cpp/bindings/lib/connector.cc:486:10 (browser_tests+0xb97b0e7)
    #20 mojo::Connector::OnHandleReadyInternal(unsigned int) /src/mojo/public/cpp/bindings/lib/connector.cc:387:3 (browser_tests+0xb97ae8e)
    #21 mojo::Connector::OnWatcherHandleReady(unsigned int) /src/mojo/public/cpp/bindings/lib/connector.cc:364:3 (browser_tests+0xb97ade0)
    #22 Invoke<void (mojo::Connector::*)(unsigned int), mojo::Connector *, unsigned int> /src/base/bind_internal.h:507:12 (browser_tests+0xb97bbcf)
    #23 MakeItSo<void (mojo::Connector::*const &)(unsigned int), mojo::Connector *, unsigned int> /src/base/bind_internal.h:607 (browser_tests+0xb97bbcf)
    #24 RunImpl<void (mojo::Connector::*const &)(unsigned int), const std::__1::tuple<base::internal::UnretainedWrapper<mojo::Connector> > &, 0> /src/base/bind_internal.h:680 (browser_tests+0xb97bbcf)
    #25 base::internal::Invoker<base::internal::BindState<void (mojo::Connector::*)(unsigned int), base::internal::UnretainedWrapper<mojo::Connector> >, void (unsigned int)>::Run(base::internal::BindStateBase*, unsigned int) /src/base/bind_internal.h:662 (browser_tests+0xb97bbcf)
    #26 Run /src/base/callback.h:129:12 (browser_tests+0x66996b8)
    #27 mojo::SimpleWatcher::DiscardReadyState(base::RepeatingCallback<void (unsigned int)> const&, unsigned int, mojo::HandleSignalsState const&) /src/mojo/public/cpp/system/simple_watcher.h:194 (browser_tests+0x66996b8)
    #28 Invoke<void (*const &)(const base::RepeatingCallback<void (unsigned int)> &, unsigned int, const mojo::HandleSignalsState &), const base::RepeatingCallback<void (unsigned int)> &, unsigned int, const mojo::HandleSignalsState &> /src/base/bind_internal.h:407:12 (browser_tests+0x6699715)
    #29 MakeItSo<void (*const &)(const base::RepeatingCallback<void (unsigned int)> &, unsigned int, const mojo::HandleSignalsState &), const base::RepeatingCallback<void (unsigned int)> &, unsigned int, const mojo::HandleSignalsState &> /src/base/bind_internal.h:607 (browser_tests+0x6699715)
    #30 RunImpl<void (*const &)(const base::RepeatingCallback<void (unsigned int)> &, unsigned int, const mojo::HandleSignalsState &), const std::__1::tuple<base::RepeatingCallback<void (unsigned int)> > &, 0> /src/base/bind_internal.h:680 (browser_tests+0x6699715)
    #31 base::internal::Invoker<base::internal::BindState<void (*)(base::RepeatingCallback<void (unsigned int)> const&, unsigned int, mojo::HandleSignalsState const&), base::RepeatingCallback<void (unsigned int)> >, void (unsigned int, mojo::HandleSignalsState const&)>::Run(base::internal::BindStateBase*, unsigned int, mojo::HandleSignalsState const&) /src/base/bind_internal.h:662 (browser_tests+0x6699715)
    #32 Run /src/base/callback.h:129:12 (browser_tests+0xa96dece)
    #33 mojo::SimpleWatcher::OnHandleReady(int, unsigned int, mojo::HandleSignalsState const&) /src/mojo/public/cpp/system/simple_watcher.cc:273 (browser_tests+0xa96dece)
    #34 Invoke<void (mojo::SimpleWatcher::*)(int, unsigned int, const mojo::HandleSignalsState &), const base::WeakPtr<mojo::SimpleWatcher> &, const int &, const unsigned int &, const mojo::HandleSignalsState &> /src/base/bind_internal.h:507:12 (browser_tests+0xa96e625)
    #35 MakeItSo<void (mojo::SimpleWatcher::*const &)(int, unsigned int, const mojo::HandleSignalsState &), const base::WeakPtr<mojo::SimpleWatcher> &, const int &, const unsigned int &, const mojo::HandleSignalsState &> /src/base/bind_internal.h:627 (browser_tests+0xa96e625)
    #36 void base::internal::Invoker<base::internal::BindState<void (mojo::SimpleWatcher::*)(int, unsigned int, mojo::HandleSignalsState const&), base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState>, void ()>::RunImpl<void (mojo::SimpleWatcher::* const&)(int, unsigned int, mojo::HandleSignalsState const&), std::__1::tuple<base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState> const&, 0ul, 1ul, 2ul, 3ul>(void (mojo::SimpleWatcher::* const&&&)(int, unsigned int, mojo::HandleSignalsState const&), std::__1::tuple<base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState> const&&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul>) /src/base/bind_internal.h:680 (browser_tests+0xa96e625)
    #37 base::internal::Invoker<base::internal::BindState<void (mojo::SimpleWatcher::*)(int, unsigned int, mojo::HandleSignalsState const&), base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState>, void ()>::Run(base::internal::BindStateBase*) /src/base/bind_internal.h:662:12 (browser_tests+0xa96e4ce)
    #38 Run /src/base/callback.h:99:12 (browser_tests+0x9a92ee8)
    #39 base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) /src/base/debug/task_annotator.cc:101 (browser_tests+0x9a92ee8)
    #40 base::MessageLoop::RunTask(base::PendingTask*) /src/base/message_loop/message_loop.cc:432:46 (browser_tests+0x55f355a)
    #41 base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) /src/base/message_loop/message_loop.cc:443:5 (browser_tests+0x55f3e7c)
    #42 base::MessageLoop::DoWork() /src/base/message_loop/message_loop.cc:487:16 (browser_tests+0x55f41c5)
    #43 base::MessagePumpGlib::Run(base::MessagePump::Delegate*) /src/base/message_loop/message_pump_glib.cc:309:49 (browser_tests+0x9ad27bb)
    #44 base::MessageLoop::Run(bool) /src/base/message_loop/message_loop.cc:374:12 (browser_tests+0x55f2c67)
    #45 non-virtual thunk to base::MessageLoop::Run(bool) /src/base/message_loop/message_loop.cc (browser_tests+0x55f2d54)
    #46 base::RunLoop::Run() /src/base/run_loop.cc:102:14 (browser_tests+0x55f6adf)
    #47 RunThisRunLoop /src/content/public/test/test_utils.cc:132:13 (browser_tests+0xa8c5bf8)
    #48 content::RunMessageLoop() /src/content/public/test/test_utils.cc:127 (browser_tests+0xa8c5bf8)
    #49 (anonymous namespace)::CaptivePortalObserver::WaitForResults(int) /src/chrome/browser/captive_portal/captive_portal_browsertest.cc:432:5 (browser_tests+0x4ea7325)
    #50 CaptivePortalBrowserTest_ShowCaptivePortalInterstitialOnCertError_Test::RunTestOnMainThread() /src/chrome/browser/captive_portal/captive_portal_browsertest.cc:1891:25 (browser_tests+0x4eb9cc3)
    #51 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() /src/content/public/test/browser_test_base.cc:406:5 (browser_tests+0xa863ddd)
    #52 Invoke<void (content::BrowserTestBase::*)(), content::BrowserTestBase *> /src/base/bind_internal.h:507:12 (browser_tests+0xa864f28)
    #53 MakeItSo<void (content::BrowserTestBase::*const &)(), content::BrowserTestBase *> /src/base/bind_internal.h:607 (browser_tests+0xa864f28)
    #54 RunImpl<void (content::BrowserTestBase::*const &)(), const std::__1::tuple<base::internal::UnretainedWrapper<content::BrowserTestBase> > &, 0> /src/base/bind_internal.h:680 (browser_tests+0xa864f28)
    #55 base::internal::Invoker<base::internal::BindState<void (content::BrowserTestBase::*)(), base::internal::UnretainedWrapper<content::BrowserTestBase> >, void ()>::Run(base::internal::BindStateBase*) /src/base/bind_internal.h:662 (browser_tests+0xa864f28)
    #56 Run /src/base/callback.h:129:12 (browser_tests+0x9ce722d)
    #57 ChromeBrowserMainParts::PreMainMessageLoopRunImpl() /src/chrome/browser/chrome_browser_main.cc:3181 (browser_tests+0x9ce722d)
    #58 ChromeBrowserMainParts::PreMainMessageLoopRun() /src/chrome/browser/chrome_browser_main.cc:2259:18 (browser_tests+0x9ce52eb)
    #59 content::BrowserMainLoop::PreMainMessageLoopRun() /src/content/browser/browser_main_loop.cc:1021:13 (browser_tests+0x6a854c0)
    #60 Invoke<int (content::BrowserMainLoop::*)(), content::BrowserMainLoop *> /src/base/bind_internal.h:507:12 (browser_tests+0x6a88df8)
    #61 MakeItSo<int (content::BrowserMainLoop::*const &)(), content::BrowserMainLoop *> /src/base/bind_internal.h:607 (browser_tests+0x6a88df8)
    #62 RunImpl<int (content::BrowserMainLoop::*const &)(), const std::__1::tuple<base::internal::UnretainedWrapper<content::BrowserMainLoop> > &, 0> /src/base/bind_internal.h:680 (browser_tests+0x6a88df8)
    #63 base::internal::Invoker<base::internal::BindState<int (content::BrowserMainLoop::*)(), base::internal::UnretainedWrapper<content::BrowserMainLoop> >, int ()>::Run(base::internal::BindStateBase*) /src/base/bind_internal.h:662 (browser_tests+0x6a88df8)
    #64 Run /src/base/callback.h:129:12 (browser_tests+0x726d05a)
    #65 content::StartupTaskRunner::RunAllTasksNow() /src/content/browser/startup_task_runner.cc:43 (browser_tests+0x726d05a)
    #66 content::BrowserMainLoop::CreateStartupTasks() /src/content/browser/browser_main_loop.cc:932:25 (browser_tests+0x6a8379a)
    #67 content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&) /src/content/browser/browser_main_runner_impl.cc:143:15 (browser_tests+0x6a893ea)
    #68 content::BrowserMain(content::MainFunctionParams const&) /src/content/browser/browser_main.cc:45:32 (browser_tests+0x6a80539)
    #69 RunBrowserProcessMain /src/content/app/content_main_runner_impl.cc:613:10 (browser_tests+0x99a6eeb)
    #70 content::ContentMainRunnerImpl::Run(bool) /src/content/app/content_main_runner_impl.cc:973 (browser_tests+0x99a6eeb)
    #71 content::ContentServiceManagerMainDelegate::RunEmbedderProcess() /src/content/app/content_service_manager_main_delegate.cc:53:32 (browser_tests+0x99a414f)
    #72 service_manager::Main(service_manager::MainParams const&) /src/services/service_manager/embedder/main.cc:472:29 (browser_tests+0xd205c76)
    #73 content::ContentMain(content::ContentMainParams const&) /src/content/app/content_main.cc:19:10 (browser_tests+0x99a494b)
    #74 content::BrowserTestBase::SetUp() /src/content/public/test/browser_test_base.cc:322:3 (browser_tests+0xa863831)
    #75 InProcessBrowserTest::SetUp() /src/chrome/test/base/in_process_browser_test.cc:272:20 (browser_tests+0x9c3a89d)
    #76 testing::Test::Run() /src/googletest (browser_tests+0x562b70b)
    #77 testing::TestInfo::Run() /src/googletest (browser_tests+0x562c96c)
    #78 testing::TestCase::Run() /src/googletest (browser_tests+0x562d1f6)
    #79 testing::internal::UnitTestImpl::RunAllTests() /src/googletest (browser_tests+0x563d796)
    #80 testing::UnitTest::Run() /src/googletest (browser_tests+0x563d07a)
    #81 RUN_ALL_TESTS /src/googletest/include/gtest/gtest.h:2329:46 (browser_tests+0x9c5d7f5)
    #82 base::TestSuite::Run() /src/base/test/test_suite.cc:277 (browser_tests+0x9c5d7f5)
    #83 ChromeTestSuiteRunner::RunTestSuite(int, char**) /src/chrome/test/base/chrome_test_launcher.cc:65:38 (browser_tests+0x9a490f6)
    #84 ChromeTestLauncherDelegate::RunTestSuite(int, char**) /src/chrome/test/base/chrome_test_launcher.cc:74:19 (browser_tests+0x9a491ff)
    #85 content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) /src/content/public/test/test_launcher.cc:650:31 (browser_tests+0xa8c08de)
    #86 LaunchChromeTests(unsigned long, content::TestLauncherDelegate*, int, char**) /src/chrome/test/base/chrome_test_launcher.cc:170:10 (browser_tests+0x9a49784)
    #87 main /src/chrome/test/base/browser_tests_main.cc:36:10 (browser_tests+0x9a48fdc)

  Previous write of size 1 at 0x7b44000004d8 by thread T2:
    #0 CaptivePortalBrowserTest::SetBehindCaptivePortal(bool) /src/chrome/browser/captive_portal/captive_portal_browsertest.cc:745:28 (browser_tests+0x4eb12d8)
    #1 Invoke<void (CaptivePortalBrowserTest::*)(bool), CaptivePortalBrowserTest *, bool> /src/base/bind_internal.h:507:12 (browser_tests+0x4ed0549)
    #2 MakeItSo<void (CaptivePortalBrowserTest::*)(bool), CaptivePortalBrowserTest *, bool> /src/base/bind_internal.h:607 (browser_tests+0x4ed0549)
    #3 RunImpl<void (CaptivePortalBrowserTest::*)(bool), std::__1::tuple<base::internal::UnretainedWrapper<CaptivePortalBrowserTest>, bool>, 0, 1> /src/base/bind_internal.h:680 (browser_tests+0x4ed0549)
    #4 base::internal::Invoker<base::internal::BindState<void (CaptivePortalBrowserTest::*)(bool), base::internal::UnretainedWrapper<CaptivePortalBrowserTest>, bool>, void ()>::RunOnce(base::internal::BindStateBase*) /src/base/bind_internal.h:649 (browser_tests+0x4ed0549)
    #5 Run /src/base/callback.h:99:12 (browser_tests+0x9a92ee8)
    #6 base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) /src/base/debug/task_annotator.cc:101 (browser_tests+0x9a92ee8)
    #7 base::MessageLoop::RunTask(base::PendingTask*) /src/base/message_loop/message_loop.cc:432:46 (browser_tests+0x55f355a)
    #8 base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) /src/base/message_loop/message_loop.cc:443:5 (browser_tests+0x55f3e7c)
    #9 base::MessageLoop::DoWork() /src/base/message_loop/message_loop.cc:487:16 (browser_tests+0x55f41c5)
    #10 base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) /src/base/message_loop/message_pump_libevent.cc:210:31 (browser_tests+0x9bfd9b4)
    #11 base::MessageLoop::Run(bool) /src/base/message_loop/message_loop.cc:374:12 (browser_tests+0x55f2c67)
    #12 non-virtual thunk to base::MessageLoop::Run(bool) /src/base/message_loop/message_loop.cc (browser_tests+0x55f2d54)
    #13 base::RunLoop::Run() /src/base/run_loop.cc:102:14 (browser_tests+0x55f6adf)
    #14 base::Thread::Run(base::RunLoop*) /src/base/threading/thread.cc:255:13 (browser_tests+0x9b8d22b)
    #15 content::BrowserProcessSubThread::IOThreadRun(base::RunLoop*) /src/content/browser/browser_process_sub_thread.cc:184:11 (browser_tests+0x6a95a06)
    #16 content::BrowserProcessSubThread::Run(base::RunLoop*) /src/content/browser/browser_process_sub_thread.cc:134:7 (browser_tests+0x6a95913)
    #17 base::Thread::ThreadMain() /src/base/threading/thread.cc:337:3 (browser_tests+0x9b8d80b)
    #18 base::(anonymous namespace)::ThreadFunc(void*) /src/base/threading/platform_thread_posix.cc:76:13 (browser_tests+0x9bf6458)

  Location is heap block of size 288 at 0x7b44000003c0 allocated by main thread:
    #0 operator new(unsigned long) /src/third_party/llvm/compiler-rt/lib/tsan/rtl/tsan_new_delete.cc:65:3 (browser_tests+0x330871d)
    #1 testing::internal::TestFactoryImpl<CaptivePortalBrowserTest_ShowCaptivePortalInterstitialOnCertError_Test>::CreateTest() /src/googletest/include/gtest/internal/gtest-internal.h:466:39 (browser_tests+0x4ed1d69)
    #2 testing::TestInfo::Run() /src/googletest (browser_tests+0x562c8ae)
    #3 testing::TestCase::Run() /src/googletest (browser_tests+0x562d1f6)
    #4 testing::internal::UnitTestImpl::RunAllTests() /src/googletest (browser_tests+0x563d796)
    #5 testing::UnitTest::Run() /src/googletest (browser_tests+0x563d07a)
    #6 RUN_ALL_TESTS /src/googletest/include/gtest/gtest.h:2329:46 (browser_tests+0x9c5d7f5)
    #7 base::TestSuite::Run() /src/base/test/test_suite.cc:277 (browser_tests+0x9c5d7f5)
    #8 ChromeTestSuiteRunner::RunTestSuite(int, char**) /src/chrome/test/base/chrome_test_launcher.cc:65:38 (browser_tests+0x9a490f6)
    #9 ChromeTestLauncherDelegate::RunTestSuite(int, char**) /src/chrome/test/base/chrome_test_launcher.cc:74:19 (browser_tests+0x9a491ff)
    #10 content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) /src/content/public/test/test_launcher.cc:650:31 (browser_tests+0xa8c08de)
    #11 LaunchChromeTests(unsigned long, content::TestLauncherDelegate*, int, char**) /src/chrome/test/base/chrome_test_launcher.cc:170:10 (browser_tests+0x9a49784)
    #12 main /src/chrome/test/base/browser_tests_main.cc:36:10 (browser_tests+0x9a48fdc)

  Thread T2 'Chrome_IOThread' (tid=37629, running) created by main thread at:
    #0 pthread_create /src/third_party/llvm/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:965:3 (browser_tests+0x329e755)
    #1 base::(anonymous namespace)::CreateThread(unsigned long, bool, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) /src/base/threading/platform_thread_posix.cc:115:13 (browser_tests+0x9bf5d96)
    #2 base::PlatformThread::CreateWithPriority(unsigned long, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) /src/base/threading/platform_thread_posix.cc:200:10 (browser_tests+0x9bf5c55)
    #3 base::Thread::StartWithOptions(base::Thread::Options const&) /src/base/threading/thread.cc:112:15 (browser_tests+0x9b8c8c5)
    #4 content::BrowserProcessSubThread::CreateIOThread() /src/content/browser/browser_process_sub_thread.cc:97:19 (browser_tests+0x6a9560e)
    #5 content::ContentMainRunnerImpl::Run(bool) /src/content/app/content_main_runner_impl.cc:953:29 (browser_tests+0x99a6cc8)
    #6 content::ContentServiceManagerMainDelegate::RunEmbedderProcess() /src/content/app/content_service_manager_main_delegate.cc:53:32 (browser_tests+0x99a414f)
    #7 service_manager::Main(service_manager::MainParams const&) /src/services/service_manager/embedder/main.cc:472:29 (browser_tests+0xd205c76)
    #8 content::ContentMain(content::ContentMainParams const&) /src/content/app/content_main.cc:19:10 (browser_tests+0x99a494b)
    #9 content::BrowserTestBase::SetUp() /src/content/public/test/browser_test_base.cc:322:3 (browser_tests+0xa863831)
    #10 InProcessBrowserTest::SetUp() /src/chrome/test/base/in_process_browser_test.cc:272:20 (browser_tests+0x9c3a89d)
    #11 testing::Test::Run() /src/googletest (browser_tests+0x562b70b)
    #12 testing::TestInfo::Run() /src/googletest (browser_tests+0x562c96c)
    #13 testing::TestCase::Run() /src/googletest (browser_tests+0x562d1f6)
    #14 testing::internal::UnitTestImpl::RunAllTests() /src/googletest (browser_tests+0x563d796)
    #15 testing::UnitTest::Run() /src/googletest (browser_tests+0x563d07a)
    #16 RUN_ALL_TESTS /src/googletest/include/gtest/gtest.h:2329:46 (browser_tests+0x9c5d7f5)
    #17 base::TestSuite::Run() /src/base/test/test_suite.cc:277 (browser_tests+0x9c5d7f5)
    #18 ChromeTestSuiteRunner::RunTestSuite(int, char**) /src/chrome/test/base/chrome_test_launcher.cc:65:38 (browser_tests+0x9a490f6)
    #19 ChromeTestLauncherDelegate::RunTestSuite(int, char**) /src/chrome/test/base/chrome_test_launcher.cc:74:19 (browser_tests+0x9a491ff)
    #20 content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) /src/content/public/test/test_launcher.cc:650:31 (browser_tests+0xa8c08de)
    #21 LaunchChromeTests(unsigned long, content::TestLauncherDelegate*, int, char**) /src/chrome/test/base/chrome_test_launcher.cc:170:10 (browser_tests+0x9a49784)
    #22 main /src/chrome/test/base/browser_tests_main.cc:36:10 (browser_tests+0x9a48fdc)

SUMMARY: ThreadSanitizer: data race chrome/browser/captive_portal/captive_portal_browsertest.cc:984:9 in CaptivePortalBrowserTest::OnIntercept(content::URLLoaderInterceptor::RequestParams*)
Change-Id: I58163d96f66f8ca3b2d257d1481b0d12f980c679
Reviewed-on: https://chromium-review.googlesource.com/c/1356582
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613677}
jkim-julie pushed a commit that referenced this pull request Dec 10, 2018
This CL is putting back the |AbortScenario| code to the previous state.

Landed here:
  https://chromium-review.googlesource.com/c/chromium/src/+/1289672

The "if (is_tracing_)" is required to avoid calling the tracing controller
when it's not instantiated. This is only happening in the unittest.

[ RUN      ] BackgroundTracingTest.SetupBackgroundTracingFieldTrial
[184899:184899:1204/184233.049256:3597383076062:FATAL:tracing_controller_impl.cc(283)] Check failed: g_tracing_controller.
#0 0x7fefd86edf5d base::debug::StackTrace::StackTrace()
#1 0x7fefd83ded3a base::debug::StackTrace::StackTrace()
#2 0x7fefd8450a4b logging::LogMessage::~LogMessage()
#3 0x7fefd461c1e1 content::TracingControllerImpl::GetInstance()
#4 0x7fefd460e198 content::BackgroundTracingManagerImpl::AbortScenario()
#5 0x55ec45d76fa2 BackgroundTracingTest::TearDown()
#6 0x55ec473ff12e _ZN7testing8internal12InvokeHelperIvNSt3__15tupleIJEEEE12InvokeMethodIN10extensions27ExtensionDownloaderDelegateEMS8_FvvEEEvPT_T0_RKS4_
#7 0x55ec4846fc42 testing::internal::HandleExceptionsInMethodIfSupported<>()
#8 0x55ec4844f2f8 testing::Test::Run()
#9 0x55ec4844ff52 testing::TestInfo::Run()
#10 0x55ec48450e5f testing::TestCase::Run()
#11 0x55ec48464fcb testing::internal::UnitTestImpl::RunAllTests()
#12 0x55ec4847946e testing::internal::HandleSehExceptionsInMethodIfSupported<>()
#13 0x55ec48471472 testing::internal::HandleExceptionsInMethodIfSupported<>()
#14 0x55ec48464c07 testing::UnitTest::Run()
#15 0x55ec4aaa2bc1 RUN_ALL_TESTS()



R=oysteine@chromium.org

Change-Id: I9198c6db87a03daba5f75ece529f104ecbfe8609
Reviewed-on: https://chromium-review.googlesource.com/c/1361830
Reviewed-by: oysteine <oysteine@chromium.org>
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614056}
jkim-julie pushed a commit that referenced this pull request Dec 10, 2018
This reverts commit a612879.

Reason for revert: Suspect Failure single_process_mash_ash_unittests Failure ash_unittests:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-chromeos-rel/16943
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8927839167159249024/+/steps/single_process_mash_ash_unittests/0/logs/LoginExpandedPublicAccountViewTest.ChangeMenuSelection/0
---
[ RUN      ] LoginExpandedPublicAccountViewTest.ChangeMenuSelection
Received signal 11 SEGV_MAPERR ffffe03f000008fe
#0 0x5631a3caedbf base::debug::StackTrace::StackTrace()
#1 0x5631a3cae941 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#2 0x7faa9920f330 <unknown>
#3 0x5631a3aafbbf ash::LoginBubble::~LoginBubble()
#4 0x5631a3ab806c ash::RightPaneView::~RightPaneView()
#5 0x5631a3ab810e ash::RightPaneView::~RightPaneView()
#6 0x5631a40385d4 views::View::~View()
#7 0x5631a3ab6df9 ash::LoginExpandedPublicAccountView::~LoginExpandedPublicAccountView()
#8 0x5631a40385d4 views::View::~View()
#9 0x5631a300afce ash::(anonymous namespace)::DragTestView::~DragTestView()
#10 0x5631a4039b42 views::View::DoRemoveChildView()
#11 0x5631a403a175 views::View::RemoveAllChildViews()
#12 0x5631a4043b81 views::internal::RootView::~RootView()
#13 0x5631a400ef0e views::MenuHostRootView::~MenuHostRootView()
#14 0x5631a404615f views::Widget::~Widget()
#15 0x5631a34d645e exo::(anonymous namespace)::ShellSurfaceWidget::~ShellSurfaceWidget()
#16 0x5631a30c36f0 ash::LoginTestBase::TearDown()

Original change's description:
> cros: Move most event/widget handling logic out of LoginBubble
> 
> Changes made:
> - Move the (keyboard/click/tap) event handling logic into a
>   LoginBubbleHandler class.
> - Move the widget handling into LoginBaseBubbleView itself
> 
> Bug: 912658
> Change-Id: Ia1a9bf4b8d847ef74ef1716e73dfe1b16b8d19b2
> Reviewed-on: https://chromium-review.googlesource.com/c/1366516
> Commit-Queue: Quan Nguyen <qnnguyen@chromium.org>
> Reviewed-by: Jacob Dufault <jdufault@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#614544}

TBR=jdufault@chromium.org,qnnguyen@chromium.org

Change-Id: Ia9b9059f24f6aa6a8551c5c6bc5c76caafe592df
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 912658
Reviewed-on: https://chromium-review.googlesource.com/c/1367070
Reviewed-by: Takashi Sakamoto <tasak@google.com>
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Cr-Commit-Position: refs/heads/master@{#614579}
jkim-julie pushed a commit that referenced this pull request Dec 10, 2018
This change further changes direct calls to Time::Now or the default
base::Clock instance to an OfflineClock module within classes which
need a Clock pointer to be passed into the class. This eliminates the
need for testing-only functions and further allows ease of testing.
This change is specific to production files and their related tests in:

chrome/browser/android/explore_sites/
components/offline_pages/core/model/

Bug: 906903
Change-Id: I02b84f65b77dc395310995c34ded41d92721aaeb
Reviewed-on: https://chromium-review.googlesource.com/c/1352813
Reviewed-by: Cathy Li <chili@chromium.org>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Reviewed-by: Dan H <harringtond@google.com>
Commit-Queue: Mark Lieu <mtlieuu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614797}
msisov pushed a commit that referenced this pull request Dec 23, 2018
# This is an automated release commit.
# Do not revert without consulting chrome-pmo@google.com.
NOAUTOREVERT=true
TBR=abdulsyed@chromiue.org

Change-Id: I442bf0a70cbe18efa744d1b2724cfe01522a8873
Reviewed-on: https://chromium-review.googlesource.com/c/1278329
Reviewed-by: chrome-release-bot@chromium.org <chrome-release-bot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#1}
Cr-Branched-From: 4226ddf-refs/heads/master@{#599034}
msisov pushed a commit that referenced this pull request Dec 23, 2018
Pressing the skip button leads to two LoginDisplayHost::Finalize() calls, which
causes a double deletion eventually leading to a crash.

The PIN setup skip button triggers two 'discover-done' events (fired in
discover_ui.js::end_), and the observer in screen_discover.js sends a finished
user action whenver it receives a discover-done event.

Discover should not be sending discover-done twice. This CL just prevents the crash.

Finalize #1
    at HTMLElement.end_
    at HTMLElement.fire
    at HTMLElement.onSkipButton_
    at HTMLElement.onAuthTokenChanged_
    at HTMLElement._observerEffect
    at HTMLElement._effectEffects
    at HTMLElement._propertySetter
    at HTMLElement.setter
    at HTMLElement.onSkipButton_
    at HTMLElement.handler

Finalize #2
    at HTMLElement.end_
    at HTMLElement.fire
    at HTMLElement.onSkipButton_
    at HTMLElement.handler
    at HTMLElement.fire
    at Object.fire
    at Object.forward
    at Object.click
    at HTMLElement.handleNative

TBR=jdufault@google.com

(cherry picked from commit 41254eb)

Bug: 895250
Change-Id: I784ee5ef0ecfc5bffad515973451331bfc0aee3f
Reviewed-on: https://chromium-review.googlesource.com/c/1299145
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Commit-Queue: Jacob Dufault <jdufault@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#602880}
Reviewed-on: https://chromium-review.googlesource.com/c/1308414
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#409}
Cr-Branched-From: 4226ddf-refs/heads/master@{#599034}
msisov pushed a commit that referenced this pull request Jan 7, 2019
r589029 changed bucketing of some navigation-timing histograms,
and broke data continuity.  To fix things going forward, the
re-bucketed histograms need to be renamed.

The old CL changed the overflow-bucket boundary of the following
histograms to 3 minutes:
1. Navigation.TimeToReadyToCommit.*
2. Navigation.StartToCommit.*
3. Navigation.ReadyToCommitUntilCommit.*

Histogram #2 above wasn't affected (the old overflow-bucket boundary
was already at 3 minutes).  Histograms #1 and #3 need to have their
names changed - the current CL is adding a "2" suffix:
- Navigation.TimeToReadyToCommit2.*
- Navigation.ReadyToCommitUntilCommit2.*

Bug: 848821
Change-Id: I6029d88df863ba3ed3edd2327c46f4ec20e6ce6d
Reviewed-on: https://chromium-review.googlesource.com/c/1394783
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620098}
msisov pushed a commit that referenced this pull request Jan 31, 2019
# This is an automated release commit.
# Do not revert without consulting chrome-pmo@google.com.
NOAUTOREVERT=true
TBR=kariah@chromium.org

Change-Id: I70bf284f70c8215f10e5313e976953df625cba24
Reviewed-on: https://chromium-review.googlesource.com/c/1356036
Reviewed-by: chrome-release-bot@chromium.org <chrome-release-bot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#1}
Cr-Branched-From: d897fb1-refs/heads/master@{#612437}
msisov pushed a commit that referenced this pull request Feb 11, 2019
This is step #1 of the MessagePump::DoWork/DoDelayedWork merge.

This staging step is required to avoid modifying all of the MessagePump
impls at once in this CL as well. DoSomeWork() will ultimately be
renamed back to DoWork() when the old DoWork()/DoDelayedWork() are
removed.

The fancy NextTaskInfo return value was chosen because:
 1) While TimeDelta would be ideal for most pumps, some pumps
    install a native timer and getting TimeTicks will allow them
    to avoid reinstalling a redundant timer when they wakeup
    mid-wait and go back to sleep on the same delayed_run_time.
    (TimeTicks is fixed and allows dedup while TimeDelta
     wouldn't be).
 2) Returning TimeTicks is simpler because PendingTask already stores
    |delayed_run_time| in TimeTicks. The current impl resolves in
    redundant conversions because of DelayTillNextTask() being a
    TimeDelta intermediary but we will remedy this after this CL.
    (LazyNow makes it such that it's not resampling so it's a big
     waste of resources).
 3) The downside to returning TimeTicks is that it forces pumps that
    care about a timeout-till-next-task to resample Now() (which is
    known to be expensive on some platforms). Solution: also return
    a |recent_now|.

ThreadControllerWithMessagePumpImpl::* will be added as a "boring" stack
frame to avoid many crashes being bucketed into this magic signature:
http://cl/232009344.

Bug: 885371
Change-Id: I87f7e37b2facba70bf6a6dcce7e688cb847d7e78
Reviewed-on: https://chromium-review.googlesource.com/c/1232518
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Alex Clarke <alexclarke@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628802}
msisov pushed a commit that referenced this pull request Feb 25, 2019
This reverts commit d82ad3d.

Reason for revert: This change is suspected of causing the #1 browser process crash on  Windows canary 74.0.3709.0.

Original change's description:
> Quota: Add SequenceChecker to QuotaManager
>
> Change-Id: I7caa60bf82fdcd63469d58afa0e29f893795ae32
> Reviewed-on: https://chromium-review.googlesource.com/c/1457000
> Commit-Queue: Victor Costan <pwnall@chromium.org>
> Reviewed-by: Victor Costan <pwnall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#631891}

TBR=pwnall@chromium.org,jarrydg@chromium.org

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

Change-Id: Ie6833ffd46083e84fb0bcfa2d38c80050ffc2e8e
Bug: 933014
Reviewed-on: https://chromium-review.googlesource.com/c/1477764
Commit-Queue: Jarryd Goodman <jarrydg@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#633450}
msisov pushed a commit that referenced this pull request Feb 25, 2019
This reverts commit b16e332.

Reason for revert: Suspect: causing webkit_layout_tests failure on WebKit Linux Trusty MSAN:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/WebKit%20Linux%20Trusty%20MSAN/12879

Unexpected Failures:
* compositing/composited-iframe-hidden.html
* compositing/force-compositing-mode/overflow-iframe-enter-compositing.html
* compositing/geometry/layer-due-to-layer-children-switch.html
* compositing/gestures/gesture-tapHighlight-1-iframe-composited-scrolled-late-noncomposite.html
* compositing/gestures/gesture-tapHighlight-1-overflow-div-scrolled-late-composite.html
...
STDERR: ==1==WARNING: MemorySanitizer: use-of-uninitialized-value
STDERR:     #0 0x55e7a80f6e40 in operator== ./../../cc/paint/node_holder.cc:26:7
STDERR:     #1 0x55e7a80f6e40 in cc::operator!=(cc::NodeHolder const&, cc::NodeHolder const&) ./../../cc/paint/node_holder.cc:37:0
STDERR:     #2 0x55e7a814deec in cc::DrawTextBlobOp::AreEqual(cc::PaintOp const*, cc::PaintOp const*) ./../../cc/paint/paint_op_buffer.cc:1761:25
STDERR:     #3 0x55e7a815a0a5 in operator== ./../../cc/paint/paint_op_buffer.cc:1864:10
STDERR:     #4 0x55e7a815a0a5 in operator!= ./../../cc/paint/paint_op_buffer.h:146:0
STDERR:     #5 0x55e7a815a0a5 in cc::PaintOpBuffer::operator==(cc::PaintOpBuffer const&) const ./../../cc/paint/paint_op_buffer.cc:2530:0
...

Original change's description:
> Add ContentCapture in blink core
> 
> This patch includes
> - ContentCaptureManager, it creates NodeHolder, starts ContentCaptureTask when
>   Node is added, removed or scroll position changed.
> - ContentCaptureClient, it defines the interface to send the captured content,
>   removed node.
> - ContentCaptureTask, it is best effort task to captures the on-screen
>   content and send them out through ContentCaptureClient.
> 
> Also, added test for ContentCapture.
> 
> Bug: 924681
> Change-Id: I286797f4395adc80384675130ad304eea7085811
> Reviewed-on: https://chromium-review.googlesource.com/c/1455812
> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
> Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
> Reviewed-by: enne <enne@chromium.org>
> Reviewed-by: Philip Rogers <pdr@chromium.org>
> Commit-Queue: Tao Bai <michaelbai@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#633631}

TBR=michaelbai@chromium.org,wangxianzhu@chromium.org,pdr@chromium.org,enne@chromium.org,skyostil@chromium.org

Change-Id: I17c120a251a3f62eb9aa227db545f35e9062a8c8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 924681
Reviewed-on: https://chromium-review.googlesource.com/c/1478663
Reviewed-by: Takashi Sakamoto <tasak@google.com>
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Cr-Commit-Position: refs/heads/master@{#633652}
msisov pushed a commit that referenced this pull request Feb 25, 2019
This reverts commit 91e9f91.

Reason for revert: suspect chromeos_components_unittests failure on Linux ChromiumOS MSan Tests
E.g.
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20ChromiumOS%20MSan%20Tests/11405
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8920945756740062352/+/steps/chromeos_components_unittests/0/logs/ProximityAuthUnlockManagerImplTest.SetRemoteDeviceLifeCycle_ConnectingRemoteDeviceLifeCycle_StopsProximityMonitor__status_CRASH_/0

[ RUN      ] ProximityAuthUnlockManagerImplTest.SetRemoteDeviceLifeCycle_ConnectingRemoteDeviceLifeCycle_StopsProximityMonitor
==28143==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x560dbbd3637f in proximity_auth::ProximityAuthUnlockManagerImplTest_SetRemoteDeviceLifeCycle_ConnectingRemoteDeviceLifeCycle_StopsProximityMonitor_Test::TestBody() ./../../chromeos/components/proximity_auth/unlock_manager_impl_unittest.cc:455:3
    #1 0x560dbc63c150 in HandleExceptionsInMethodIfSupported<testing::Test, void> ./../../third_party/googletest/src/googletest/src/gtest.cc:0:0
    #2 0x560dbc63c150 in testing::Test::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2499:0
...

Original change's description:
> Smart Lock: Clear stale ProximityMonitor on disconnect.
> 
> UnlockManager was previously holding onto a stale
> ProximityMonitor from its first connection. That meant
> that if a disconnection and subsequent new connection
> occurred, the old ProximityMonitor, with an old reference
> to the previous connection (represented as a ClientChannel
> object) was used. This always resulted in a Smart Lock
> failure for a second connection, and occasionally, a crash.
> 
> In order to correctly reset the ProximityMonitor object,
> the way that it is called had to be refactored (remove
> completely unneeded ScreenLockBridge logic), in order to
> consolidate calls to one place.
> 
> Bug: 931929
> Change-Id: I9b9a65ad0f0752d58d680623815ea60b5369c45c
> Reviewed-on: https://chromium-review.googlesource.com/c/1479941
> Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#634011}

TBR=khorimoto@chromium.org,hansberry@chromium.org

Change-Id: Ie7c50832625a3d23edb1d02b94e17410146602b7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 931929
Reviewed-on: https://chromium-review.googlesource.com/c/1480309
Reviewed-by: Takashi Sakamoto <tasak@google.com>
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Cr-Commit-Position: refs/heads/master@{#634088}
msisov pushed a commit that referenced this pull request Feb 25, 2019
This reverts commit aacb3ec.

Fix the use-of-uninitialized-value issue

Original change's description:
> Revert "Add ContentCapture in blink core"
> 
> This reverts commit b16e332.
> 
> Reason for revert: Suspect: causing webkit_layout_tests failure on WebKit Linux Trusty MSAN:
> https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/WebKit%20Linux%20Trusty%20MSAN/12879
> 
> Unexpected Failures:
> * compositing/composited-iframe-hidden.html
> * compositing/force-compositing-mode/overflow-iframe-enter-compositing.html
> * compositing/geometry/layer-due-to-layer-children-switch.html
> * compositing/gestures/gesture-tapHighlight-1-iframe-composited-scrolled-late-noncomposite.html
> * compositing/gestures/gesture-tapHighlight-1-overflow-div-scrolled-late-composite.html
> ...
> STDERR: ==1==WARNING: MemorySanitizer: use-of-uninitialized-value
> STDERR:     #0 0x55e7a80f6e40 in operator== ./../../cc/paint/node_holder.cc:26:7
> STDERR:     #1 0x55e7a80f6e40 in cc::operator!=(cc::NodeHolder const&, cc::NodeHolder const&) ./../../cc/paint/node_holder.cc:37:0
> STDERR:     #2 0x55e7a814deec in cc::DrawTextBlobOp::AreEqual(cc::PaintOp const*, cc::PaintOp const*) ./../../cc/paint/paint_op_buffer.cc:1761:25
> STDERR:     #3 0x55e7a815a0a5 in operator== ./../../cc/paint/paint_op_buffer.cc:1864:10
> STDERR:     #4 0x55e7a815a0a5 in operator!= ./../../cc/paint/paint_op_buffer.h:146:0
> STDERR:     #5 0x55e7a815a0a5 in cc::PaintOpBuffer::operator==(cc::PaintOpBuffer const&) const ./../../cc/paint/paint_op_buffer.cc:2530:0
> ...
> 
> Original change's description:
> > Add ContentCapture in blink core
> > 
> > This patch includes
> > - ContentCaptureManager, it creates NodeHolder, starts ContentCaptureTask when
> >   Node is added, removed or scroll position changed.
> > - ContentCaptureClient, it defines the interface to send the captured content,
> >   removed node.
> > - ContentCaptureTask, it is best effort task to captures the on-screen
> >   content and send them out through ContentCaptureClient.
> > 
> > Also, added test for ContentCapture.
> > 
> > Bug: 924681
> > Change-Id: I286797f4395adc80384675130ad304eea7085811
> > Reviewed-on: https://chromium-review.googlesource.com/c/1455812
> > Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
> > Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
> > Reviewed-by: enne <enne@chromium.org>
> > Reviewed-by: Philip Rogers <pdr@chromium.org>
> > Commit-Queue: Tao Bai <michaelbai@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#633631}
> 
> TBR=michaelbai@chromium.org,wangxianzhu@chromium.org,pdr@chromium.org,enne@chromium.org,skyostil@chromium.org
> 
> Change-Id: I17c120a251a3f62eb9aa227db545f35e9062a8c8
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 924681
> Reviewed-on: https://chromium-review.googlesource.com/c/1478663
> Reviewed-by: Takashi Sakamoto <tasak@google.com>
> Commit-Queue: Takashi Sakamoto <tasak@google.com>
> Cr-Commit-Position: refs/heads/master@{#633652}

TBR=michaelbai@chromium.org,wangxianzhu@chromium.org,pdr@chromium.org,tasak@google.com,enne@chromium.org,skyostil@chromium.org

Change-Id: Ia55b79c6b7eb22bb05087ea49016ea3860eed127
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 924681
Reviewed-on: https://chromium-review.googlesource.com/c/1481014
Reviewed-by: Tao Bai <michaelbai@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: Tao Bai <michaelbai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634258}
msisov pushed a commit that referenced this pull request Feb 25, 2019
This CL makes the following changes:

Product code changes:

- Moving most of verification of CommonNavigationParams that used to be
  done in RenderFrameHostImpl::BeginNavigation into a new helper
  function in frame_host/ipc_utils.cc - VerifyCommonNavigationParams.

- Making sure that VerifyCommonNavigationParams verifies things that used
  to be covered by RenderFrameHostImpl::BeginNavigation and *also*
  covers verification of |initiator_origin| (reusing other verification
  helpers present in frame_host/ipc_utils.cc).

- Auditing callers of SetRequestorOrigin and making sure they use the right
  origin - they should use the initiating frame's origin.  The following
  functions have been tweaked:
     1) blink::FrameLoader::ResourceRequestForReload
     2) blink::HTMLAnchorElement::HandleClick

  - Without tweak #1 some browser tests would have failed
    (e.g. the existing DNSErrorPageTest.StaleCacheStatus or the new
    ErrorPageNavigationReload_InSubframe_BlockedByClient in the
    RenderFrameHostManagerTest suite).

  - I also note that the tweaked line in
    blink::FrameLoader::ResourceRequestForReload was added in r529246
    which also added a regression test which still passes:
    RenderFrameDevToolsAgentHostBrowserTest.ReloadDinoPage

Test changes:

- Moving IsolateOrigin[ForTesting] from security_exploit_browsertest.cc
  into content/public/test/content_browser_test_utils.h

- Renaming DidCommitProvisionalLoadInterceptor into FrameHostInterceptor
  and supporting intercepting one more method of FrameHost -
  BeginNavigation (in addition to the old support for intercepting
  DidCommitProvisionalLoad).

- Adding test coverage for terminating a renderer that provides an
  invalid value in the |initiator_origin| argument of BeginNavigation.

- Adding test coverage for renderer-initiated reloading of an
  erorred-out subframe.

Bug: 919144
Change-Id: I2de74a634e6691b490cd94df1ac8494248f7c363
Reviewed-on: https://chromium-review.googlesource.com/c/1455470
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634486}
msisov pushed a commit that referenced this pull request Mar 14, 2019
# This is an automated release commit.
# Do not revert without consulting chrome-pmo@google.com.
NOAUTOREVERT=true
TBR=kariah@chromium.org

Change-Id: Ide9f9388ebe20c3b4d364cd7902802185a723685
Reviewed-on: https://chromium-review.googlesource.com/c/1436409
Reviewed-by: chrome-release-bot@chromium.org <chrome-release-bot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3683@{#1}
Cr-Branched-From: e510299-refs/heads/master@{#625896}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants