New issue
Advanced search Search tips

Issue 837684 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug


Sign in to add a comment

Create standalone library providing server implementation of WindowService on top of aura

Project Member Reported by sky@chromium.org, Apr 27 2018

Issue description

Initially this will live in services/ui/ws2. Ash will consume this to directly implement the WindowService server side mojoms.
 

Comment 1 by sky@chromium.org, Apr 27 2018

Blocking: 837686

Comment 2 by sky@chromium.org, Apr 27 2018

Blocking: 837689

Comment 3 by sky@chromium.org, Apr 27 2018

Blocking: 837692

Comment 4 by sky@chromium.org, Apr 27 2018

Blockedon: 837695

Comment 5 by sky@chromium.org, Apr 27 2018

Blockedon: 837696

Comment 6 by sky@chromium.org, Apr 27 2018

Blocking: -837692

Comment 7 by sky@chromium.org, Apr 27 2018

Blockedon: 837692

Comment 8 by sky@chromium.org, Apr 27 2018

Blockedon: 837698

Comment 9 by sky@chromium.org, Apr 27 2018

Blockedon: 837699

Comment 10 by sky@chromium.org, Apr 27 2018

Blockedon: 837700

Comment 11 by sky@chromium.org, Apr 27 2018

Blockedon: 837702

Comment 12 by sky@chromium.org, Apr 27 2018

Blockedon: 837703

Comment 13 by sky@chromium.org, Apr 27 2018

Blockedon: 837705

Comment 14 by sky@chromium.org, Apr 27 2018

Blockedon: 837710

Comment 15 by sky@chromium.org, Apr 27 2018

Blocking: 837713

Comment 16 by sky@chromium.org, Apr 27 2018

Blockedon: 837715

Comment 17 by sky@chromium.org, Apr 27 2018

Blockedon: 837716

Comment 18 by sky@chromium.org, May 3 2018

Blockedon: 839591

Comment 19 by sky@chromium.org, May 3 2018

Blockedon: 839592
Blockedon: 838597
Blockedon: 841941

Comment 22 by msw@chromium.org, May 11 2018

Blockedon: 842295

Comment 23 by msw@chromium.org, May 11 2018

Blockedon: 842298

Comment 24 by msw@chromium.org, May 11 2018

Blockedon: 842301

Comment 25 by msw@chromium.org, May 14 2018

Blockedon: 836449
Project Member

Comment 26 by bugdroid1@chromium.org, May 17 2018

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

commit 9b59f744ac7869c3b35c330162a83df6341f91a2
Author: Scott Violet <sky@chromium.org>
Date: Thu May 17 18:01:09 2018

chromeos: makes WindowService correctly deal with deletion

There are two issues here:
. Clients call DeleteWindow() to delete a window. DeleteWindowImpl()
  calls 'delete', which ends up in OnWindowDestroyed(), which then
  erased from the map that owned the window. Hence double delete.
. OnWindowDestroyed() didn't deal with the case of the window being
  deleted by some other means.

I think the logic of having an external caller (not the client) delete
the window may happen. So, the code is slightly more complex.

Also, I moved to a vector because sets are FRUSTRATING. In particular
the iterator returns a const foo, which means I can't call std::move()
or release.

BUG= 837684 
TEST=covered by tests

Change-Id: Ifd541676d1039997ce0929e4c6986388dbbc5ba4
Reviewed-on: https://chromium-review.googlesource.com/1063315
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559602}
[modify] https://crrev.com/9b59f744ac7869c3b35c330162a83df6341f91a2/services/ui/ws2/window_service_client.cc
[modify] https://crrev.com/9b59f744ac7869c3b35c330162a83df6341f91a2/services/ui/ws2/window_service_client.h
[modify] https://crrev.com/9b59f744ac7869c3b35c330162a83df6341f91a2/services/ui/ws2/window_service_client_test_helper.cc
[modify] https://crrev.com/9b59f744ac7869c3b35c330162a83df6341f91a2/services/ui/ws2/window_service_client_test_helper.h
[modify] https://crrev.com/9b59f744ac7869c3b35c330162a83df6341f91a2/services/ui/ws2/window_service_client_unittest.cc

Project Member

Comment 27 by bugdroid1@chromium.org, May 17 2018

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

commit a6bc87d5961c8f1c6274c809d5c334cb9604a384
Author: Scott Violet <sky@chromium.org>
Date: Thu May 17 21:51:38 2018

Fixs shutdown crash in test_ws

test_ws had a shutdown crash because the Screen was getting deleted
before ScreenProvider. This means when ~ScreenProvider tried to remove
the observer it was accessing a null object.

BUG= 837684 
TEST=this is a test only change

Change-Id: Id9330f8981de9fe9ad33a9f6f5cabc2fa318091f
Reviewed-on: https://chromium-review.googlesource.com/1064931
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559697}
[modify] https://crrev.com/a6bc87d5961c8f1c6274c809d5c334cb9604a384/services/ui/test_ws/test_ws.cc

Comment 28 by sky@chromium.org, May 18 2018

Blockedon: 844789
Project Member

Comment 29 by bugdroid1@chromium.org, May 19 2018

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

commit e984b395bc3ee46a6cf15ffe8e1a53d89a58c2d0
Author: Scott Violet <sky@chromium.org>
Date: Sat May 19 22:01:55 2018

chromeos: ensure client is notified of LocalSurfaceId

The server assigns LocalSurfaceIds (each window may have a
LocalSurfaceId). LocalSurfaceIds change any time the size
changes. This means the server needs to notify the client any time the
bounds change. Normally when a client requests a change that
succeeds the server acks it, and that is it. What we've done with
bounds of roots (where LocalSurfaceIds matter), is when the
client requests the change the server responds with
OnWindowBoundsChanged(), which includes the LocalSurfaceId, and then
the server returns false. By returning false the client applies the
LocalSurfaceId that was just sent from the server.

services/ui/ws had this logic, this makes ws2 have the same logic.

This is at least part of the problem as to why painting isn't working.

BUG= 837684 
TEST=covered by test

Change-Id: Ia66d3e5b4120adce2a5d033563a0c387c3bd7b7e
Reviewed-on: https://chromium-review.googlesource.com/1066828
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560177}
[modify] https://crrev.com/e984b395bc3ee46a6cf15ffe8e1a53d89a58c2d0/services/ui/ws2/client_root.cc
[modify] https://crrev.com/e984b395bc3ee46a6cf15ffe8e1a53d89a58c2d0/services/ui/ws2/window_service_client.cc
[modify] https://crrev.com/e984b395bc3ee46a6cf15ffe8e1a53d89a58c2d0/services/ui/ws2/window_service_client_unittest.cc

Project Member

Comment 31 by bugdroid1@chromium.org, May 21 2018

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

commit b6fc4f1e5ce1becbf35ea7607945e757f6d1c363
Author: Scott Violet <sky@chromium.org>
Date: Mon May 21 16:29:11 2018

chromeos: trivial test to ensure Embed() works

BUG= 837684 
TEST=test only change

Change-Id: I206ed5e1bae668285638fde7aff56f70c75c3cec
Reviewed-on: https://chromium-review.googlesource.com/1066786
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560281}
[modify] https://crrev.com/b6fc4f1e5ce1becbf35ea7607945e757f6d1c363/services/ui/ws2/test_change_tracker.cc
[modify] https://crrev.com/b6fc4f1e5ce1becbf35ea7607945e757f6d1c363/services/ui/ws2/test_change_tracker.h
[modify] https://crrev.com/b6fc4f1e5ce1becbf35ea7607945e757f6d1c363/services/ui/ws2/window_service_client_unittest.cc

Project Member

Comment 32 by bugdroid1@chromium.org, May 21 2018

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

commit 61cd791d359a73bec8b9db4d39634798c47b75ea
Author: Scott Violet <sky@chromium.org>
Date: Mon May 21 17:48:12 2018

chromeos: adds some helper functions to reduce event testing boilerplate

BUG= 837684 
TEST=test only changes

Change-Id: I34a2d9327d9a3771bfc953383e3551ebbcba33aa
Reviewed-on: https://chromium-review.googlesource.com/1067437
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560294}
[modify] https://crrev.com/61cd791d359a73bec8b9db4d39634798c47b75ea/services/ui/ws2/BUILD.gn
[add] https://crrev.com/61cd791d359a73bec8b9db4d39634798c47b75ea/services/ui/ws2/event_test_utils.cc
[add] https://crrev.com/61cd791d359a73bec8b9db4d39634798c47b75ea/services/ui/ws2/event_test_utils.h
[modify] https://crrev.com/61cd791d359a73bec8b9db4d39634798c47b75ea/services/ui/ws2/window_service_client_unittest.cc

Project Member

Comment 33 by bugdroid1@chromium.org, May 22 2018

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

commit 28448b13a43eeba9b49d39528002b62c5ead5f7b
Author: Scott Violet <sky@chromium.org>
Date: Tue May 22 22:53:45 2018

chromeos: extracts WindowService setup into separate header

This way it can be used with other tests.

BUG= 837684 
TEST=covered by test

Change-Id: Iac388d27fa5be1bcd2b6d7f937dabdc4949dec34
Reviewed-on: https://chromium-review.googlesource.com/1069303
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560823}
[modify] https://crrev.com/28448b13a43eeba9b49d39528002b62c5ead5f7b/services/ui/ws2/BUILD.gn
[modify] https://crrev.com/28448b13a43eeba9b49d39528002b62c5ead5f7b/services/ui/ws2/window_service_client_unittest.cc
[add] https://crrev.com/28448b13a43eeba9b49d39528002b62c5ead5f7b/services/ui/ws2/window_service_test_setup.cc
[add] https://crrev.com/28448b13a43eeba9b49d39528002b62c5ead5f7b/services/ui/ws2/window_service_test_setup.h

Project Member

Comment 34 by bugdroid1@chromium.org, May 24 2018

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

commit 4235ac0aea0b0bdfbac6e6b20e27e2980f64986f
Author: Scott Violet <sky@chromium.org>
Date: Thu May 24 00:22:12 2018

chromeos: makes SetWindowBounds() check LocalSurfaceId too

If the bounds don't change, but the LocalSurfaceId does, then the request needs
to be processed. This is important for windows that have LocalSurfaceId, such as
at the embedding.

This bug resulted in the ksv not drawing correctly initially.

BUG= 837684 
TEST=covered by test

Change-Id: I0bf67a15bc2af0ae42df07378908d3c1a8d90885
Reviewed-on: https://chromium-review.googlesource.com/1070689
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561328}
[modify] https://crrev.com/4235ac0aea0b0bdfbac6e6b20e27e2980f64986f/services/ui/ws2/window_service_client.cc
[modify] https://crrev.com/4235ac0aea0b0bdfbac6e6b20e27e2980f64986f/services/ui/ws2/window_service_client.h
[modify] https://crrev.com/4235ac0aea0b0bdfbac6e6b20e27e2980f64986f/services/ui/ws2/window_service_client_test_helper.cc
[modify] https://crrev.com/4235ac0aea0b0bdfbac6e6b20e27e2980f64986f/services/ui/ws2/window_service_client_test_helper.h
[modify] https://crrev.com/4235ac0aea0b0bdfbac6e6b20e27e2980f64986f/services/ui/ws2/window_service_client_unittest.cc

Project Member

Comment 35 by bugdroid1@chromium.org, May 25 2018

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

commit 1caf60ef8424e2b3823d510f04518a2e292ecb67
Author: Scott Violet <sky@chromium.org>
Date: Fri May 25 00:14:35 2018

chromeos: introduces Embedding class in ws2

Embedding owns the embedded WindowServiceClient (child). The ClientWindow
the embedding is in owns the Embedding. This fixes a bug where destroying
the root of an embedding didn't destroy the embedded WindowServiceClient.

I will have to change this to support re-embedding, but until then I think this
makes things clearer.

BUG= 837684 
TEST=covered by tests

Change-Id: I9e3b1d144ec63c80b660dabf1dd50fdb6c20c568
Reviewed-on: https://chromium-review.googlesource.com/1072806
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561701}
[modify] https://crrev.com/1caf60ef8424e2b3823d510f04518a2e292ecb67/services/ui/ws2/BUILD.gn
[modify] https://crrev.com/1caf60ef8424e2b3823d510f04518a2e292ecb67/services/ui/ws2/client_root.cc
[modify] https://crrev.com/1caf60ef8424e2b3823d510f04518a2e292ecb67/services/ui/ws2/client_window.cc
[modify] https://crrev.com/1caf60ef8424e2b3823d510f04518a2e292ecb67/services/ui/ws2/client_window.h
[add] https://crrev.com/1caf60ef8424e2b3823d510f04518a2e292ecb67/services/ui/ws2/embedding.cc
[add] https://crrev.com/1caf60ef8424e2b3823d510f04518a2e292ecb67/services/ui/ws2/embedding.h
[add] https://crrev.com/1caf60ef8424e2b3823d510f04518a2e292ecb67/services/ui/ws2/embedding_unittest.cc
[modify] https://crrev.com/1caf60ef8424e2b3823d510f04518a2e292ecb67/services/ui/ws2/focus_handler_unittest.cc
[modify] https://crrev.com/1caf60ef8424e2b3823d510f04518a2e292ecb67/services/ui/ws2/window_service_client.cc
[modify] https://crrev.com/1caf60ef8424e2b3823d510f04518a2e292ecb67/services/ui/ws2/window_service_client.h
[modify] https://crrev.com/1caf60ef8424e2b3823d510f04518a2e292ecb67/services/ui/ws2/window_service_client_test_helper.cc
[modify] https://crrev.com/1caf60ef8424e2b3823d510f04518a2e292ecb67/services/ui/ws2/window_service_client_test_helper.h
[modify] https://crrev.com/1caf60ef8424e2b3823d510f04518a2e292ecb67/services/ui/ws2/window_service_client_unittest.cc
[modify] https://crrev.com/1caf60ef8424e2b3823d510f04518a2e292ecb67/services/ui/ws2/window_service_test_setup.cc
[modify] https://crrev.com/1caf60ef8424e2b3823d510f04518a2e292ecb67/services/ui/ws2/window_service_test_setup.h

Project Member

Comment 36 by bugdroid1@chromium.org, May 25 2018

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

commit a8e07c8ccbd8ca0d008d29278d04e1f5e4765017
Author: Scott Violet <sky@chromium.org>
Date: Fri May 25 02:55:16 2018

chromeos: track intercepts_events in Embedding

intercepts_events only applies at an Embedding, so that tracking at
WindowServiceClient was misleading. This moves tracking of this state to
Embedding.

This patch shouldn't change anything, it just moves tracking of
intercepts_events to where it applies (applying at the client level
wasn't right, and came from old code).

BUG= 837684 
TEST=covered by tests

Change-Id: I99f9a0b32b046ee70215fb15727904a1ea346853
Reviewed-on: https://chromium-review.googlesource.com/1072369
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561748}
[modify] https://crrev.com/a8e07c8ccbd8ca0d008d29278d04e1f5e4765017/services/ui/test_ws/test_ws.cc
[modify] https://crrev.com/a8e07c8ccbd8ca0d008d29278d04e1f5e4765017/services/ui/ws2/client_window.cc
[modify] https://crrev.com/a8e07c8ccbd8ca0d008d29278d04e1f5e4765017/services/ui/ws2/embedding.cc
[modify] https://crrev.com/a8e07c8ccbd8ca0d008d29278d04e1f5e4765017/services/ui/ws2/embedding.h
[modify] https://crrev.com/a8e07c8ccbd8ca0d008d29278d04e1f5e4765017/services/ui/ws2/focus_handler_unittest.cc
[modify] https://crrev.com/a8e07c8ccbd8ca0d008d29278d04e1f5e4765017/services/ui/ws2/window_service.cc
[modify] https://crrev.com/a8e07c8ccbd8ca0d008d29278d04e1f5e4765017/services/ui/ws2/window_service.h
[modify] https://crrev.com/a8e07c8ccbd8ca0d008d29278d04e1f5e4765017/services/ui/ws2/window_service_client.cc
[modify] https://crrev.com/a8e07c8ccbd8ca0d008d29278d04e1f5e4765017/services/ui/ws2/window_service_client.h
[modify] https://crrev.com/a8e07c8ccbd8ca0d008d29278d04e1f5e4765017/services/ui/ws2/window_service_client_binding.cc
[modify] https://crrev.com/a8e07c8ccbd8ca0d008d29278d04e1f5e4765017/services/ui/ws2/window_service_client_binding.h
[modify] https://crrev.com/a8e07c8ccbd8ca0d008d29278d04e1f5e4765017/services/ui/ws2/window_service_test_setup.cc
[modify] https://crrev.com/a8e07c8ccbd8ca0d008d29278d04e1f5e4765017/services/ui/ws2/window_service_test_setup.h
[modify] https://crrev.com/a8e07c8ccbd8ca0d008d29278d04e1f5e4765017/services/ui/ws2/window_tree_factory.cc

Project Member

Comment 37 by bugdroid1@chromium.org, May 30 2018

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

commit 3a4683ff0def7e037b918d79b1d0e50c4d6fd775
Author: Scott Violet <sky@chromium.org>
Date: Wed May 30 16:50:14 2018

chromeos: minor cleanup of test code

Most places that create windows don't care about the window id, and the random
ids make it mildly harder to understand. So, make the window id optional.

BUG= 837684 
TEST=test only change

Change-Id: I418692d5e4cb0a52c24d0a448eff5c4645ec99d2
Reviewed-on: https://chromium-review.googlesource.com/1079205
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562867}
[modify] https://crrev.com/3a4683ff0def7e037b918d79b1d0e50c4d6fd775/services/ui/ws2/embedding_unittest.cc
[modify] https://crrev.com/3a4683ff0def7e037b918d79b1d0e50c4d6fd775/services/ui/ws2/focus_handler_unittest.cc
[modify] https://crrev.com/3a4683ff0def7e037b918d79b1d0e50c4d6fd775/services/ui/ws2/window_service_client_test_helper.cc
[modify] https://crrev.com/3a4683ff0def7e037b918d79b1d0e50c4d6fd775/services/ui/ws2/window_service_client_test_helper.h
[modify] https://crrev.com/3a4683ff0def7e037b918d79b1d0e50c4d6fd775/services/ui/ws2/window_service_client_unittest.cc

Comment 38 by sky@chromium.org, May 30 2018

Blockedon: -836449

Comment 39 by sky@chromium.org, May 30 2018

Blocking: 841941

Comment 40 by sky@chromium.org, May 30 2018

Blockedon: -841941
Project Member

Comment 41 by bugdroid1@chromium.org, Jun 1 2018

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

commit 75800102558e55df6381be223a92e21a9705fd93
Author: James Cook <jamescook@chromium.org>
Date: Fri Jun 01 21:36:14 2018

chromeos: Remove ash::Config::MUS

Config::MUS isn't used any more since --enable-features=Mus was deleted.
We're switching to ws2 ("window service as a library").

Bug:  706913 ,  837684 ,  841941 
Change-Id: Ifc05e369d558ede4f04692fe54eabc1e357c95b7
Reviewed-on: https://chromium-review.googlesource.com/1082916
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563806}
[modify] https://crrev.com/75800102558e55df6381be223a92e21a9705fd93/ash/display/mirror_window_controller.cc
[modify] https://crrev.com/75800102558e55df6381be223a92e21a9705fd93/ash/display/mirror_window_controller_unittest.cc
[modify] https://crrev.com/75800102558e55df6381be223a92e21a9705fd93/ash/public/cpp/config.h
[modify] https://crrev.com/75800102558e55df6381be223a92e21a9705fd93/ash/shell.cc
[modify] https://crrev.com/75800102558e55df6381be223a92e21a9705fd93/ash/test/ash_test_base.h
[modify] https://crrev.com/75800102558e55df6381be223a92e21a9705fd93/ash/window_manager.cc
[modify] https://crrev.com/75800102558e55df6381be223a92e21a9705fd93/chrome/browser/ui/views/chrome_browser_main_extra_parts_views.h
[modify] https://crrev.com/75800102558e55df6381be223a92e21a9705fd93/testing/buildbot/filters/mash.ash_unittests.filter

Comment 42 by sky@chromium.org, Jun 5 2018

Blockedon: -842295

Comment 43 by sky@chromium.org, Jun 13 2018

Blockedon: 852440

Comment 44 by sky@chromium.org, Jun 13 2018

Blockedon: 852439

Comment 45 by sky@chromium.org, Jun 20 2018

Blockedon: 854700
Blocking: 847992
Blockedon: 866628
Status: Fixed (was: Started)

Sign in to add a comment