Bug 78680 – [GTK] change the way of registering DeviceOrientation clients

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Gyuyoung Kim]

Bug 78085 removed deviceOrientationClient and deviceMotionClient from PageClients. Instead, DeviceOrientationClient and DeviceMotionClient should be registered by PageSupplement class. Chromium, mac and qt ports are already changed by Bug 78085.

[Gyuyoung Kim]

Created attachment 127131 [details]
Patch

[Gyuyoung Kim]

CC'ing MORITA Hajime, Martin and Joone.

[Tomasz Morawski]

Why do you have two "Reviewed by NOBODY (OOPS!)." section in Source/WebKit/gtk/ChangeLog changlog?

[Gyuyoung Kim]

Created attachment 127136 [details]
Patch

[Gyuyoung Kim]

(In reply to comment #3)
> Why do you have two "Reviewed by NOBODY (OOPS!)." section in Source/WebKit/gtk/ChangeLog changlog?

Oops, sorry, my mistake.

[WebKit Review Bot]

Attachment 127131 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
From git://git.webkit.org/WebKit
   c74927a..8567f8d  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 107800 = 2cae6de8e39e001704f4df389e619575a378fb30
r107797 = 8567f8d3c2539a28a496edaf1048483e973975c2
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
	M	LayoutTests/fast/forms/radio-nested-labels.html
	M	LayoutTests/ChangeLog
107798 = 3671b2d23de7ade4cb1d1e78a3f6f7673db6a6c9 already exists! Why are we refetching it?
 at /usr/lib/git-core/git-svn line 5210

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Attachment 127136 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
Index mismatch: 0dfd183742a71cb5de5dadc3ae177fc72b63a194 != 9cdcda984def14b8bf8a32b6da6784c8a6ef7b3a
rereading 8567f8d3c2539a28a496edaf1048483e973975c2
	M	LayoutTests/fast/forms/radio-nested-labels.html
	M	LayoutTests/ChangeLog
107798 = 3671b2d23de7ade4cb1d1e78a3f6f7673db6a6c9 already exists! Why are we refetching it?
 at /usr/lib/git-core/git-svn line 5210

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Martin Robinson]

Comment on attachment 127136 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=127136&action=review

> Source/WebKit/efl/ewk/ewk_view.cpp:611
>  #if ENABLE(DEVICE_ORIENTATION)
> -    pageClients.deviceMotionClient = new WebCore::DeviceMotionClientEfl;
> -    pageClients.deviceOrientationClient = new WebCore::DeviceOrientationClientEfl;
> +    WebCore::provideDeviceMotionTo(priv->page, new WebCore::DeviceMotionClientEfl);
> +    WebCore::provideDeviceOrientationTo(priv->page, new WebCore::DeviceOrientationClientEfl);
>  #endif
>      priv->page = new WebCore::Page(pageClients);

Aren't you passing priv->page before you assign it?

[Gyuyoung Kim]

Created attachment 127297 [details]
Patch

[Gyuyoung Kim]

(In reply to comment #8)

> Aren't you passing priv->page before you assign it?

Oops, that was my mistake. I fix it. BTW, it looks style bot was something wrong.

[Gyuyoung Kim]

Created attachment 127586 [details]
Patch

[Gyuyoung Kim]

EFL port was fixed it by r108066. This patch  is only for GTK port now.

[Gustavo Noronha (kov)]

Comment on attachment 127586 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=127586&action=review

> Source/WebKit/gtk/webkit/webkitwebview.cpp:3460
> +#if ENABLE(DEVICE_ORIENTATION)
> +    WebCore::provideDeviceMotionTo(priv->corePage, new DeviceMotionClientGtk);
> +    WebCore::provideDeviceOrientationTo(priv->corePage, new DeviceOrientationClientGtk);
> +#endif
> +

Looks like Qt has some code to use a mock for device orientation. I guess we could have the same and unskip a few tests, perhaps? ;)

[Gyuyoung Kim]

(In reply to comment #13)

> Looks like Qt has some code to use a mock for device orientation. I guess we could have the same and unskip a few tests, perhaps? ;)

Yes, it looks mac, chromium as well as qt use DeviceOrientationClientMock. I think we need to consider whether EFL and GTK port need to use it as well. I will file a new bug for this.

[WebKit Review Bot]

Comment on attachment 127586 [details]
Patch

Clearing flags on attachment: 127586

Committed r108151: <http://trac.webkit.org/changeset/108151>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.