Bug 100628 - Remove an un-necessary connection to X-Server
Summary: Remove an un-necessary connection to X-Server
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-28 22:40 PDT by Kalyan
Modified: 2012-11-01 04:22 PDT (History)
1 user (show)

See Also:


Attachments
proposed-patch (1.77 KB, patch)
2012-10-28 22:42 PDT, Kalyan
no flags Details | Formatted Diff | Diff
patch2 (1.80 KB, patch)
2012-10-30 13:39 PDT, Kalyan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kalyan 2012-10-28 22:40:42 PDT
In GraphicsSurfacePrivate constructor we make a connection to Xserver
        for gaining access to the screens in the display.Immediately(in constructor itself)
        we ask the display from offscreenwindow.
        Removing the former connection as we use the Display returned by offscreenwindow.
Comment 1 Kalyan 2012-10-28 22:42:26 PDT
Created attachment 171159 [details]
proposed-patch

Patch
Comment 2 Kalyan 2012-10-28 23:03:47 PDT
After the following changeset we always use the Display returned by offscreenwindow:

 https://bugs.webkit.org/show_bug.cgi?id=100523
Comment 3 Kenneth Rohde Christiansen 2012-10-30 02:57:04 PDT
Comment on attachment 171159 [details]
proposed-patch

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

> Source/WebCore/ChangeLog:8
> +        for gaining access to the screens in the display.Immediately(in constructor itself)
> +        we ask the display from offscreenwindow.

Space before Immediately and before the (. Maybe it should be on a new line

Try to make the changelogs easy readable

> Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:133
>          , m_hasAlpha(false)
>      {
>          GLXContext shareContextObject = 0;
> -        m_display = XOpenDisplay(0);
> +        m_display = m_offScreenWindow.display();

sorry is m_offScreenWindow initialized here? it is not in the initializer list
Comment 4 Kalyan 2012-10-30 03:41:23 PDT
(In reply to comment #3)
> (From update of attachment 171159 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=171159&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        for gaining access to the screens in the display.Immediately(in constructor itself)
> > +        we ask the display from offscreenwindow.
> 
> Space before Immediately and before the (. Maybe it should be on a new line
> 
> Try to make the changelogs easy readable
> 
> > Source/WebCore/platform/graphics/surfaces/qt/GraphicsSurfaceGLX.cpp:133
> >          , m_hasAlpha(false)
> >      {
> >          GLXContext shareContextObject = 0;
> > -        m_display = XOpenDisplay(0);
> > +        m_display = m_offScreenWindow.display();
> 
> sorry is m_offScreenWindow initialized here? it is not in the initializer list
m_offScreenWindow is a member variable of GraphicsSurfacePrivate. 
It is created here and increments refcount (static member variable) in its constructor.
It would create a window and open connection to display on calls to 
getXWindow () and display () respectively.
Comment 5 Kalyan 2012-10-30 13:39:56 PDT
Created attachment 171513 [details]
patch2

Review changes
Comment 6 WebKit Review Bot 2012-11-01 04:22:21 PDT
Comment on attachment 171513 [details]
patch2

Clearing flags on attachment: 171513

Committed r133147: <http://trac.webkit.org/changeset/133147>
Comment 7 WebKit Review Bot 2012-11-01 04:22:24 PDT
All reviewed patches have been landed.  Closing bug.