Assistance retrofitting a patch into chromium [SOLVED]

Message

graysky · Post by **graysky** » Sat Nov 16, 2013 10:18 pm

I have been maintaining the scroll-pixel patch for chromium since upstream removed it. Thus far, I have been successfully modifying the original code to work with each new release of chromium until version 31.x. I find myself unable to adapt the original patch into the new source tree. Anyone with experience is welcomed to take a look at the version 30 patch which worked just fine and the v31 source code and suggest a fix to restore this functionality.

My github repo with v30 patch: https://github.com/graysky2/chromium-sc ... xels-patch
Source code to chromium v31: https://commondatastorage.googleapis.co ... .57.tar.xz

Thanks all.

eyoung100 · Post by **eyoung100** » Mon Nov 18, 2013 4:19 pm

Can you post the dif from when you try to integrate the patch

See GNOME Bug 692666. Comment that the patch you've retrofitted is no longer working.

graysky · Post by **graysky** » Mon Nov 18, 2013 9:03 pm

eyoung100 wrote:Can you post the dif from when you try to integrate the patch See GNOME Bug 692666. Comment that the patch you've retrofitted is no longer working.

I did take a stab at it; see the unstable branch of the github link I posted. It applies but does not compile.

eyoung100 · Post by **eyoung100** » Mon Nov 18, 2013 9:09 pm

graysky wrote:
eyoung100 wrote:Can you post the dif from when you try to integrate the patch See GNOME Bug 692666. Comment that the patch you've retrofitted is no longer working.
I did take a stab at it; see the unstable branch of the github link I posted. It applies but does not compile.

Remove lines 59 - 89

graysky · Post by **graysky** » Mon Nov 18, 2013 10:15 pm

eyoung100 wrote:Remove lines 59 - 89

OK... git commit. Thanks for the time and attention. I am attempting to build now... will post back in 20 min or so.

graysky · Post by **graysky** » Mon Nov 18, 2013 10:37 pm

Ack... build aborted.

Code: Select all

...
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/render_widget_host_view_base.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/render_widget_host_view_gtk.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/render_widget_host_view_guest.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/synthetic_gesture_calculator.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/synthetic_gesture_controller.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/socket_stream_dispatcher_host.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/socket_stream_host.o
content/browser/renderer_host/render_widget_host_view_gtk.cc: In static member function 'static float content::RenderWidgetHostViewGtkWidget::GetScrollPixelsPerTick()':
content/browser/renderer_host/render_widget_host_view_gtk.cc:459:23: error: 'kDefaultScrollPixelsPerTick' was not declared in this scope
       scroll_pixels = kDefaultScrollPixelsPerTick;
                       ^
  CXX(target) out/Release/obj.target/content_browser/content/browser/resolve_proxy_msg_helper.o
content/content_browser.target.mk:824: recipe for target 'out/Release/obj.target/content_browser/content/browser/renderer_host/render_widget_host_view_gtk.o' failed
make: *** [out/Release/obj.target/content_browser/content/browser/renderer_host/render_widget_host_view_gtk.o] Error 1
make: *** Waiting for unfinished jobs....

eyoung100 · Post by **eyoung100** » Mon Nov 18, 2013 10:45 pm

now readd lines 59 - 84 of the patch file you should me earlier, and post the failure if any.

graysky · Post by **graysky** » Tue Nov 19, 2013 1:51 am

OK... git commit 2.

Aborted again:

Code: Select all

...
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/render_widget_host_view_base.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/render_widget_host_view_gtk.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/render_widget_host_view_guest.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/synthetic_gesture_calculator.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/synthetic_gesture_controller.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/socket_stream_dispatcher_host.o
content/browser/renderer_host/render_widget_host_view_gtk.cc:477:16: error: 'static float content::RenderWidgetHostViewGtkWidget::GetScrollPixelsPerTick()' cannot be overloaded
   static float GetScrollPixelsPerTick() {
                ^
content/browser/renderer_host/render_widget_host_view_gtk.cc:455:16: error: with 'static float content::RenderWidgetHostViewGtkWidget::GetScrollPixelsPerTick()'
   static float GetScrollPixelsPerTick() {
                ^
content/browser/renderer_host/render_widget_host_view_gtk.cc: In static member function 'static float content::RenderWidgetHostViewGtkWidget::GetScrollPixelsPerTick()':
content/browser/renderer_host/render_widget_host_view_gtk.cc:459:23: error: 'kDefaultScrollPixelsPerTick' was not declared in this scope
       scroll_pixels = kDefaultScrollPixelsPerTick;
                       ^
content/browser/renderer_host/render_widget_host_view_gtk.cc: In static member function 'static float content::RenderWidgetHostViewGtkWidget::GetScrollPixelsPerTick()':
content/browser/renderer_host/render_widget_host_view_gtk.cc:481:23: error: 'kDefaultScrollPixelsPerTick' was not declared in this scope
       scroll_pixels = kDefaultScrollPixelsPerTick;
                       ^
content/content_browser.target.mk:824: recipe for target 'out/Release/obj.target/content_browser/content/browser/renderer_host/render_widget_host_view_gtk.o' failed
make: *** [out/Release/obj.target/content_browser/content/browser/renderer_host/render_widget_host_view_gtk.o] Error 1
make: *** Waiting for unfinished jobs....

eyoung100 · Post by **eyoung100** » Tue Nov 19, 2013 3:04 pm

Variable Not Declared in this Scope.

Why are line's 7 - 26 the same as 59 - 84

Remove 59 - 84 again and read the link I posted here. Focusing on lines 7 - 26, it looks as though you may need to read some header files

graysky · Post by **graysky** » Tue Nov 19, 2013 3:42 pm

OK... git commit 3.

I read though the link you posted, but most of it was lost on me having no C programming experience; I barely qualify as a shell scripter for that matter

Any suggestions or pull requests are welcomed.

Aborted again:

Code: Select all

...
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/render_widget_host_view_base.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/render_widget_host_view_gtk.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/render_widget_host_view_guest.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/synthetic_gesture_calculator.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/synthetic_gesture_controller.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/socket_stream_dispatcher_host.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/renderer_host/socket_stream_host.o
  CXX(target) out/Release/obj.target/content_browser/content/browser/resolve_proxy_msg_helper.o
content/browser/renderer_host/render_widget_host_view_gtk.cc: In static member function 'static float content::RenderWidgetHostViewGtkWidget::GetScrollPixelsPerTick()':
content/browser/renderer_host/render_widget_host_view_gtk.cc:459:23: error: 'kDefaultScrollPixelsPerTick' was not declared in this scope
       scroll_pixels = kDefaultScrollPixelsPerTick;
                       ^
content/content_browser.target.mk:824: recipe for target 'out/Release/obj.target/content_browser/content/browser/renderer_host/render_widget_host_view_gtk.o' failed
make: *** [out/Release/obj.target/content_browser/content/browser/renderer_host/render_widget_host_view_gtk.o] Error 1
make: *** Waiting for unfinished jobs....

eyoung100 · Post by **eyoung100** » Tue Nov 19, 2013 3:49 pm

Do you have OO experience in any language

graysky · Post by **graysky** » Tue Nov 19, 2013 4:01 pm

eyoung100 wrote:Do you have OO experience in any language

None. To date, retrofitting the patch was as simple as diffing the original files. It more more difficult when the code moved into other files, but grepping for it is how I assembled the various patches. With v31, I hit the roadblock in my first post.

eyoung100 · Post by **eyoung100** » Tue Nov 19, 2013 4:57 pm

The reason it's gone now is because upstream removed the function definition for the function you patch back in from the class header file. You need to put it back in the header file...

graysky · Post by **graysky** » Tue Nov 19, 2013 6:52 pm

eyoung100 wrote:The reason it's gone now is because upstream removed the function definition for the function you patch back in from the class header file. You need to put it back in the header file...

Well, if the function is "kDefaultScrollPixelsPerTick", the only file from the v30 source that contains that text (from grepping for it) is: content/browser/renderer_host/render_widget_host_view_gtk.cc.

Code: Select all

% pagrep kDefaultScrollPixelsPerTick
./content/browser/renderer_host/render_widget_host_view_gtk.cc:75:const float kDefaultScrollPixelsPerTick = 160.0f / 3.0f;
./content/browser/renderer_host/render_widget_host_view_gtk.cc:503:    return num_clicks * kDefaultScrollPixelsPerTick;
./content/browser/renderer_host/render_widget_host_view_gtk.cc:527:        web_event.deltaY = kDefaultScrollPixelsPerTick;
./content/browser/renderer_host/render_widget_host_view_gtk.cc:529:        web_event.deltaY = -kDefaultScrollPixelsPerTick;
./content/browser/renderer_host/render_widget_host_view_gtk.cc:533:        web_event.deltaX = kDefaultScrollPixelsPerTick;
./content/browser/renderer_host/render_widget_host_view_gtk.cc:535:        web_event.deltaX = -kDefaultScrollPixelsPerTick

When I look at what I believe to be the header file -- the one that ends in .h -- if I diff the two version, here is what results, but I think this is beyond my abilities to fix.

Code: Select all

% diff -u chromium-30.0.1599.114/content/browser/renderer_host/render_widget_host_view_gtk.h chromium-31.0.1650.57/content/browser/renderer_host/render_widget_host_view_gtk.h
--- chromium-30.0.1599.114/content/browser/renderer_host/render_widget_host_view_gtk.h	2013-10-24 03:11:27.000000000 -0400
+++ chromium-31.0.1650.57/content/browser/renderer_host/render_widget_host_view_gtk.h	2013-11-15 19:17:37.000000000 -0500
@@ -17,8 +17,6 @@
 #include "content/browser/renderer_host/render_widget_host_view_base.h"
 #include "content/common/content_export.h"
 #include "ipc/ipc_sender.h"
-#include "ui/base/animation/animation_delegate.h"
-#include "ui/base/animation/slide_animation.h"
 #include "ui/base/gtk/gtk_signal.h"
 #include "ui/base/gtk/gtk_signal_registrar.h"
 #include "ui/base/gtk/owned_widget_gtk.h"
@@ -83,8 +81,8 @@
   virtual void UpdateCursor(const WebCursor& cursor) OVERRIDE;
   virtual void SetIsLoading(bool is_loading) OVERRIDE;
   virtual void TextInputTypeChanged(ui::TextInputType type,
-                                    bool can_compose_inline,
-                                    ui::TextInputMode input_mode) OVERRIDE;
+                                    ui::TextInputMode input_mode,
+                                    bool can_compose_inline) OVERRIDE;
   virtual void ImeCancelComposition() OVERRIDE;
   virtual void DidUpdateBackingStore(
       const gfx::Rect& scroll_rect,
@@ -98,7 +96,7 @@
   virtual void SetTooltipText(const string16& tooltip_text) OVERRIDE;
   virtual void SelectionChanged(const string16& text,
                                 size_t offset,
-                                const ui::Range& range) OVERRIDE;
+                                const gfx::Range& range) OVERRIDE;
   virtual void SelectionBoundsChanged(
       const ViewHostMsg_SelectionBounds_Params& params) OVERRIDE;
   virtual void ScrollOffsetChanged() OVERRIDE;
@@ -131,8 +129,8 @@
   virtual gfx::GLSurfaceHandle GetCompositingSurface() OVERRIDE;
   virtual bool LockMouse() OVERRIDE;
   virtual void UnlockMouse() OVERRIDE;
-  virtual void OnAccessibilityNotifications(
-      const std::vector<AccessibilityHostMsg_NotificationParams>& params)
+  virtual void OnAccessibilityEvents(
+      const std::vector<AccessibilityHostMsg_EventParams>& params)
       OVERRIDE;
 
   // ActiveWindowWatcherXObserver implementation.

eyoung100 · Post by **eyoung100** » Tue Nov 19, 2013 7:37 pm

Thats the file but don't diff but merge 31 into 32

graysky · Post by **graysky** » Tue Nov 19, 2013 7:45 pm

eyoung100 wrote:Thats the file but don't diff but merge 31 into 32

Sorry, I don't understand merge in this context. What do you mean?

eyoung100 · Post by **eyoung100** » Tue Nov 19, 2013 8:01 pm

A diff adds and subtracts from a file. If snippet a exists in file a, its looked for in file b, if it exists it's subtracted out, if it doesnt its added in file b. You need to merge only compare file a and file b and add file a and b together

graysky · Post by **graysky** » Tue Nov 19, 2013 8:08 pm

I'm not sure how to accomplish this... basically you want me to add the two files together, not accounting for the differences? Wouldn't that double some lines?

v30 file: http://pastebin.com/FT3x2gD3
v31 file: http://pastebin.com/2v0JBS3S

Can you create the merged file so I can see what it should look like?

eyoung100 · Post by **eyoung100** » Tue Nov 19, 2013 8:17 pm

Start with a blank file

If a line exists in File A, copy it into Empty. If the same line exists in File B, leave it out
If a line exists in File B that's not in A, copy it to Empty.

or Copy A and B to Empty, doubling the lines, and when the file is finished remove the doubles, leaving a single.

graysky · Post by **graysky** » Tue Nov 19, 2013 8:25 pm

eyoung100 wrote:Start with a blank file
Copy A and B to Empty, doubling the lines, and when the file is finished remove the doubles, leaving a single.

I'm not sure how to do this short of inspecting all 340+ lines. Catting them together will satisfy the first part, but I have no idea how to remove the dups when the resulting files isn't sorted.

eyoung100 · Post by **eyoung100** » Tue Nov 19, 2013 9:21 pm

There is no shortcut, open each file and the empty file in your preferred text editor, copy each line one at a time from each file in the order they are written in order to create a file like so:

Code: Select all

Line 1 from A
Line 1 from B
Line 2 From A
Line 2 From B

for all 340 lines. Save the file once they are all pasted in order, then reopen it. Now if Line X from File A = Line X From File B, remove one of the lines. This ensures no doubles will exist when a Line Exists in A that's not in B and a line that Exists in B that's not in A. When you Finish the files will be merged. We have to do it this way since you have no idea how to write a Header File in C++

graysky · Post by **graysky** » Wed Nov 20, 2013 7:53 pm

EDIT: I was mistaken.

dolohow · Post by **dolohow** » Mon Nov 25, 2013 10:06 am

Try with this header file for v31:
http://pastebin.com/nyhDS1j1

eyoung100 · Post by **eyoung100** » Mon Nov 25, 2013 3:33 pm

dolohow wrote:Try with this header file for v31:
http://pastebin.com/nyhDS1j1

Ill advised: If something was added in v32 that isn't in v31 we need v32's version. A line by line copy will achieve the desired result. The difference here is the header file info was taken out for this patch because the patch the OP is maintaining was "cleaned out" because upstream isn't using the option anymore.

dolohow · Post by **dolohow** » Mon Nov 25, 2013 3:53 pm

eyoung100 wrote:Ill advised: If something was added in v32 that isn't in v31 we need v32's version.

You mean v30 and v31? Because v32 is not stable yet.
I did a merge from this two files provided by OP and that's it.