Skip to content
This repository was archived by the owner on Apr 3, 2020. It is now read-only.

Conversation

@astojilj
Copy link
Contributor

BUG=XWALK-5503

Some OSs (e.g. Windows and Linux) don't mark windows as hidden on screen
lock or when other opaque windows fully cover them.
Note that e.g. OSX and Android do this while Windows and Linux don't.
OnSoftVisibilityChanged enables that OS window state is kept as is (on
Linux and Windows) and that e.g. power saving PageVisibility API is still
properly triggered.

The first patch here targets Windows and screen lock.

@crosswalk-trybot
Copy link

Testing patch series with astojilj/chromium-crosswalk@6910fe5 as its head.

Bot Status
Content Shell Android-x86 [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/243)
Content Shell Linux [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/241)

@astojilj
Copy link
Contributor Author

I don't like the patch - it doesn't cover the case when another opaque window is covering the page fully but it only covers screen lock. Updating it...

@astojilj
Copy link
Contributor Author

@darktears Updated the signatures to make them more common. Patch is still handling screen lock only. Could you please review this? Thanks.

@darktears
Copy link
Contributor

@astojilj that's ok to land it here but do you think that Google would be interested to have the patch upstream. It sound legit to me that when I lock the screen the app get the event and can pause a game say.

@pozdnyakov any other feedback?

@astojilj
Copy link
Contributor Author

About upstream: initiated the CL discussion here:
https://codereview.chromium.org/1746013002
I'll submit the bug and handle the Linux case there too. It is a bug - don't see it's a problem to get people interested to get it upstream.

}

void NativeViewHost::OnSoftVisibilityChanged(bool visible) {
if (!native_view_ || !native_wrapper_.get())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: .get()is not needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep it - as it is on 3 other places in the same file like that if ... native_wrapper_.get()

BUG=XWALK-5503

Some OSs (e.g. Windows and Linux) don't mark windows as hiden on screen
lock or when other opaque windows fully cover them.
Note that e.g. OSX and Android do this while Windows and Linux don't.
OnSoftVisibilityChanged enables that OS window state is kept as is (on
Linux and Windows) and that e.g. power saving PageVisibility API is still
properly triggered.

The first patch here targets Windows and screen lock.

branch pagevis
@crosswalk-trybot
Copy link

Testing patch series with astojilj/chromium-crosswalk@9e10775 as its head.

Bot Status
Content Shell Android-x86 [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/244)
Content Shell Linux [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/242)

@astojilj
Copy link
Contributor Author

astojilj commented Mar 1, 2016

@pozdnyakov Thanks. Updated patch; 2 issues fixed, didn't act on some of the nits.

@pozdnyakov
Copy link
Contributor

lgtm

rakuco pushed a commit that referenced this pull request Mar 3, 2016
Fixes the issue with the test on 10.10 for the commit: https://chromium.googlesource.com/chromium/src/+/048c293643242dc0357c8bc8651d1c251559a400

BUG=544307

Review URL: https://codereview.chromium.org/1650713003

[email protected]
Cr-Commit-Position: refs/heads/master@{#373695}
(cherry picked from commit cae2257)

Review URL: https://codereview.chromium.org/1681463003 .

Cr-Commit-Position: refs/branch-heads/2623@{#321}
Cr-Branched-From: 92d7753-refs/heads/master@{#369907}
rakuco pushed a commit that referenced this pull request Mar 3, 2016
Page Visibility API: visibilitychange on Windows screen lock/unlock
@rakuco rakuco merged commit 39c2e8e into crosswalk-project:master Mar 3, 2016
huningxin pushed a commit to huningxin/chromium-croswalk that referenced this pull request May 17, 2016
Previously MediaWebContentsObserver DCHECKed that an active player
was never added twice. To avoid complicated logic in WMPI to keep track
of when it's okay to send a playing notification to
MediaWebContentsObserver, now it's allowed to send it more than once.

BUG=595903, 595970

Review URL: https://codereview.chromium.org/1810213002

Cr-Commit-Position: refs/heads/master@{#382098}
(cherry picked from commit 73fd2eb)

Review URL: https://codereview.chromium.org/1821753002 .

Cr-Commit-Position: refs/branch-heads/2661@{crosswalk-project#321}
Cr-Branched-From: ef6f6ae-refs/heads/master@{#378081}
fujunwei pushed a commit to fujunwei/chromium-crosswalk that referenced this pull request May 27, 2016
> [DevTools] Reset tooltip on resize.
>
> This prevents body being scrolled because of absoultely positioned tooltip.
>
> BUG=567454
>
> Review URL: https://codereview.chromium.org/1502383003
>
> Cr-Commit-Position: refs/heads/master@{#363733}
TBR=pfeldman
(cherry picked from commit 27f8f7e)

Review URL: https://codereview.chromium.org/1518983002 .

Cr-Commit-Position: refs/branch-heads/2564@{crosswalk-project#321}
Cr-Branched-From: 1283eca-refs/heads/master@{#359700}
huningxin pushed a commit to huningxin/chromium-croswalk that referenced this pull request Oct 9, 2016
BUG=618506

Review-Url: https://codereview.chromium.org/2050303003
Cr-Commit-Position: refs/heads/master@{#398995}
(cherry picked from commit bb1df9d)

Review URL: https://codereview.chromium.org/2062563002 .

Cr-Commit-Position: refs/branch-heads/2743@{crosswalk-project#321}
Cr-Branched-From: 2b3ae3b-refs/heads/master@{#394939}
imreotto pushed a commit to tenta-browser/chromium-crosswalk that referenced this pull request Nov 2, 2017
This fix sets the content description on the view for each menu item in
the bottom nav menu instead of the menu item itself. Setting content
description on menu items is only available in API 26.

Bug: 763067
Change-Id: I7b3ef1eec846159a3e7ffa983956937b1ea65dc0
Reviewed-on: https://chromium-review.googlesource.com/671652
Reviewed-by: Theresa <[email protected]>
Commit-Queue: Troy Hildebrandt <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#502702}
TBR: [email protected]
Reviewed-on: https://chromium-review.googlesource.com/673303
Cr-Commit-Position: refs/branch-heads/3202@{crosswalk-project#321}
Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants