Bug 78680 - [GTK] change the way of registering DeviceOrientation clients
Summary: [GTK] change the way of registering DeviceOrientation clients
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gyuyoung Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-15 01:09 PST by Gyuyoung Kim
Modified: 2012-02-17 18:51 PST (History)
7 users (show)

See Also:


Attachments
Patch (4.37 KB, patch)
2012-02-15 01:15 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (4.33 KB, patch)
2012-02-15 01:44 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (4.67 KB, patch)
2012-02-15 19:37 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (2.41 KB, patch)
2012-02-17 07:23 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2012-02-15 01:09:04 PST
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.
Comment 1 Gyuyoung Kim 2012-02-15 01:15:08 PST
Created attachment 127131 [details]
Patch
Comment 2 Gyuyoung Kim 2012-02-15 01:16:25 PST
CC'ing MORITA Hajime, Martin and Joone.
Comment 3 Tomasz Morawski 2012-02-15 01:41:28 PST
Why do you have two "Reviewed by NOBODY (OOPS!)." section in Source/WebKit/gtk/ChangeLog changlog?
Comment 4 Gyuyoung Kim 2012-02-15 01:44:17 PST
Created attachment 127136 [details]
Patch
Comment 5 Gyuyoung Kim 2012-02-15 01:44:40 PST
(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.
Comment 6 WebKit Review Bot 2012-02-15 04:02:59 PST
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.
Comment 7 WebKit Review Bot 2012-02-15 04:06:42 PST
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.
Comment 8 Martin Robinson 2012-02-15 07:30:33 PST
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?
Comment 9 Gyuyoung Kim 2012-02-15 19:37:19 PST
Created attachment 127297 [details]
Patch
Comment 10 Gyuyoung Kim 2012-02-15 19:39:21 PST
(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.
Comment 11 Gyuyoung Kim 2012-02-17 07:23:45 PST
Created attachment 127586 [details]
Patch
Comment 12 Gyuyoung Kim 2012-02-17 07:26:12 PST
EFL port was fixed it by r108066. This patch  is only for GTK port now.
Comment 13 Gustavo Noronha (kov) 2012-02-17 07:37:22 PST
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? ;)
Comment 14 Gyuyoung Kim 2012-02-17 17:22:50 PST
(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.
Comment 15 WebKit Review Bot 2012-02-17 18:51:19 PST
Comment on attachment 127586 [details]
Patch

Clearing flags on attachment: 127586

Committed r108151: <http://trac.webkit.org/changeset/108151>
Comment 16 WebKit Review Bot 2012-02-17 18:51:26 PST
All reviewed patches have been landed.  Closing bug.