[chromium-dev] Re: Mac/Linux heads-up: RenderWidgetHostView changes needed
I'm still a little lost in this discussion. I'm repeatedly reading the code and the patch, and I'll get back to you when I fully understand it. But what I wanted to say is that there is a significant difference in the paint pipeline as it currently exists on the Mac compared to Win/Lin. On Win, DidPaintRect() calls Redraw() which calls RedrawWindow(...RDW_UPDATENOW...) which (AFAIK) does a synchronous draw via Win32 callback to OnPaint(). On Lin, DidPaintRect() calls Paint() which blits via BackingStore::ShowRect(). Again, synchronously. On Mac, DidPaintRect() calls Redraw() which calls -[NSView setNeedsDisplayInRect:], setting the damaged rect in the view. The call returns, and sometime later, during the next pump of the message loop, Cocoa wakes up, decides to repaint all damaged windows, calls our -drawRect:, which then calls RenderWidgetHost::GetBackingStore(). That gap prevents recursion. If for some reason GetBackingStore ends up causing a DidPaintRect() message, what will happen is: - Original DidPaintRect(), adds damage ... message loop ... - -drawRect: -- GetBackingStore() --- New DidPaintRect(), adds more damage -- blitting ... Cocoa clears the damage region, all of it Avi --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Creating bugs from test_expectations.txt
Yes, the intention is to have something maintainable that gets run as part of the regular testing/merging/etc. process -- which means eventually rewriting in python. I wrote this in Java because we had a java lib for automatically creating bugs in the issue tracker, and I wanted to get it up and running quickly. We should have a python API for doing the same thing. I can easily change the script so it marks DEFERs as P3s. However, currently, the script doesn't alter any entries in the file that already have bugs -- meaning something like BUG1234 DEFER : ... or WONTFIX DEFER : ... won't change. Is this the correct behavior? I'm planning on running the script later today with the most recent version of test_expectations.txt -- there will be 200+ new bugs. Let me know if I should hold off on doing so. Ojan, I'll send you the review for test_expecations.txt. Regards, Glenn On Fri, Apr 10, 2009 at 4:36 PM, Ojan Vafai o...@google.com wrote: I hate to ask this, but any chance we could rewrite it in python? It would be a lot less code in python (one script file) and would match the rest of our codebase (which currently does all scripting in python). I'm mainly worried about usability and maintainability here. That all said, I think you should go ahead and run this script as a one off so we can get the test lists having bug IDs ASAP as that's kind of urgent to us being able to track the current state of layout tests. Then we can worry about checking in a python version later if we decide it's necessary. Pam, Darin, Jon, Mark: You might want to confirm that my request below makes sense with respect to future handling/triaging of layout tests bugs. Also for the initial one-off version, can you make any tests that are currently marked as DEFER be P3s? Then when you spit out the results to the test_expectations file, I don't think there's any need to include the DEFER anymore as we'll rely entirely on bug priorities to decide which tests need fixing for the next release (i.e. all P1 tests and maybe some P2 tests). That way there will be less initial triaging to do in order to make sure we keep the tree shippable. Eventually we'll have to go through all the P3s in the Untriaged state and up some of them to P2s, but there is no pressing need to do so right away. Ojan On Fri, Apr 10, 2009 at 3:42 PM, Glenn Wilson gwil...@chromium.orgwrote: (Ack...resending) Ok, I have the CL ready, if anyone with Java readability would be willing to do a review, please let me know. more inline... On Fri, Apr 10, 2009 at 2:07 PM, Pam Greene p...@chromium.org wrote: On Thu, Apr 9, 2009 at 6:49 PM, Ojan Vafai o...@google.com wrote: At a quick glance, this looks great. I didn't look over every bug, but the ones I did look at look good. Yep. You'll want some sort of default description for the ones that have none. I ran the script on a small test file...an example bug is here: http://code.google.com/p/chromium/issues/detail?id=9991 There's at least a small bit of indication of where it came from. I also added the line numbers from test_expectations.txt that generated the bug. Additionally, I also wrote the script to then add the created bug numbers to the file. This ends up re-arranging some of the flags (DEBUG DEFER ... etc. -- DEFER DEBUG ...), but all data is retained. 200+ bugs is certainly too many, but that's no reason not to file them. (Sorry, couldn't resist. Seriously, yes, definitely file them. Better in the issue tracker than getting lost.) There are probably some improvements to be had to better group some of the bugs, but it's probably not an order of magnitude improvement. I just didn't want to create tons of bugs that were not useful. It would be great to check in a version of this script that we could run when a number of tests fail (e.g. when doing a bad webkit merge). That way, we can add them all to the local test_expectations.txt file and have it spit out the new results. Fixing the file we have and moving forward are slightly different use cases, but yes. In the long run, we shouldn't need any script to fix an existing bug-less expectations file, only the part that adds bugs for newly added tests. I'm not sure whether that part should be controlled by adding the tests to the file and having the script file bugs, or making it a fully interactive app: give it a list of files and a description, and it both changes the file and creates a bug. There may be a short-term solution and a long-term solution. The short term solution seems to be for someone (assumedly the merger) to add the failing tests to the file, run the script, then commit the file. In the long term, this could be one step, or perhaps be driven right from a roll DEPS to new version of WebKit script...right? Really, it would be great if the script filed bugs and then just modified test_expectations.txt directly (without committing it). Also, the
[chromium-dev] Re: Mac/Linux heads-up: RenderWidgetHostView changes needed
On Mon, Apr 13, 2009 at 8:43 AM, Avi Drissman a...@chromium.org wrote: I'm still a little lost in this discussion. I'm repeatedly reading the code and the patch, and I'll get back to you when I fully understand it. But what I wanted to say is that there is a significant difference in the paint pipeline as it currently exists on the Mac compared to Win/Lin. On Win, DidPaintRect() calls Redraw() which calls RedrawWindow(...RDW_UPDATENOW...) which (AFAIK) does a synchronous draw via Win32 callback to OnPaint(). On Lin, DidPaintRect() calls Paint() which blits via BackingStore::ShowRect(). Again, synchronously. On Mac, DidPaintRect() calls Redraw() which calls -[NSView setNeedsDisplayInRect:], setting the damaged rect in the view. The call returns, and sometime later, during the next pump of the message loop, Cocoa wakes up, decides to repaint all damaged windows, calls our -drawRect:, which then calls RenderWidgetHost::GetBackingStore(). The Mac implementation is wrong. On a single core machine, the renderer could starve the UI thread of the browser, preventing any pixels from ever being updated on the screen. You need to change the Mac implementation to flush to the screen when DidPaintRect is not called withing drawRect / GetBackingStore. Prior to Peter's change it was not possible for DidPaintRect to be called from within GetBackingStore. Now that is possible, so the consumer needs to take care. -Darin That gap prevents recursion. If for some reason GetBackingStore ends up causing a DidPaintRect() message, what will happen is: - Original DidPaintRect(), adds damage ... message loop ... - -drawRect: -- GetBackingStore() --- New DidPaintRect(), adds more damage -- blitting ... Cocoa clears the damage region, all of it Avi --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Mac/Linux heads-up: RenderWidgetHostView changes needed
On Mon, Apr 13, 2009 at 12:13 PM, Darin Fisher da...@chromium.org wrote: You need to change the Mac implementation to flush to the screen when DidPaintRect is not called withing drawRect / GetBackingStore. I'll put it on my list of things to do. (First is fixing a bug that is causing drawing to not work at all.) Research time, though, how to do a direct draw in Cocoa. Avi --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Creating bugs from test_expectations.txt
On Mon, Apr 13, 2009 at 9:09 AM, Glenn Wilson gwil...@chromium.org wrote: Yes, the intention is to have something maintainable that gets run as part of the regular testing/merging/etc. process -- which means eventually rewriting in python. I wrote this in Java because we had a java lib for automatically creating bugs in the issue tracker, and I wanted to get it up and running quickly. We should have a python API for doing the same thing. I can easily change the script so it marks DEFERs as P3s. However, currently, the script doesn't alter any entries in the file that already have bugs -- meaning something like BUG1234 DEFER : ... or WONTFIX DEFER : ... won't change. Is this the correct behavior? The end goal is to take DEFER out and let the bug priorities run the show. But if we're not entirely confident that this will work (both the script and the human tracking process afterward), it's easy enough to strip the DEFER modifiers later. - Pam I'm planning on running the script later today with the most recent version of test_expectations.txt -- there will be 200+ new bugs. Let me know if I should hold off on doing so. Ojan, I'll send you the review for test_expecations.txt. Regards, Glenn On Fri, Apr 10, 2009 at 4:36 PM, Ojan Vafai o...@google.com wrote: I hate to ask this, but any chance we could rewrite it in python? It would be a lot less code in python (one script file) and would match the rest of our codebase (which currently does all scripting in python). I'm mainly worried about usability and maintainability here. That all said, I think you should go ahead and run this script as a one off so we can get the test lists having bug IDs ASAP as that's kind of urgent to us being able to track the current state of layout tests. Then we can worry about checking in a python version later if we decide it's necessary. Pam, Darin, Jon, Mark: You might want to confirm that my request below makes sense with respect to future handling/triaging of layout tests bugs. Also for the initial one-off version, can you make any tests that are currently marked as DEFER be P3s? Then when you spit out the results to the test_expectations file, I don't think there's any need to include the DEFER anymore as we'll rely entirely on bug priorities to decide which tests need fixing for the next release (i.e. all P1 tests and maybe some P2 tests). That way there will be less initial triaging to do in order to make sure we keep the tree shippable. Eventually we'll have to go through all the P3s in the Untriaged state and up some of them to P2s, but there is no pressing need to do so right away. Ojan On Fri, Apr 10, 2009 at 3:42 PM, Glenn Wilson gwil...@chromium.orgwrote: (Ack...resending) Ok, I have the CL ready, if anyone with Java readability would be willing to do a review, please let me know. more inline... On Fri, Apr 10, 2009 at 2:07 PM, Pam Greene p...@chromium.org wrote: On Thu, Apr 9, 2009 at 6:49 PM, Ojan Vafai o...@google.com wrote: At a quick glance, this looks great. I didn't look over every bug, but the ones I did look at look good. Yep. You'll want some sort of default description for the ones that have none. I ran the script on a small test file...an example bug is here: http://code.google.com/p/chromium/issues/detail?id=9991 There's at least a small bit of indication of where it came from. I also added the line numbers from test_expectations.txt that generated the bug. Additionally, I also wrote the script to then add the created bug numbers to the file. This ends up re-arranging some of the flags (DEBUG DEFER ... etc. -- DEFER DEBUG ...), but all data is retained. 200+ bugs is certainly too many, but that's no reason not to file them. (Sorry, couldn't resist. Seriously, yes, definitely file them. Better in the issue tracker than getting lost.) There are probably some improvements to be had to better group some of the bugs, but it's probably not an order of magnitude improvement. I just didn't want to create tons of bugs that were not useful. It would be great to check in a version of this script that we could run when a number of tests fail (e.g. when doing a bad webkit merge). That way, we can add them all to the local test_expectations.txt file and have it spit out the new results. Fixing the file we have and moving forward are slightly different use cases, but yes. In the long run, we shouldn't need any script to fix an existing bug-less expectations file, only the part that adds bugs for newly added tests. I'm not sure whether that part should be controlled by adding the tests to the file and having the script file bugs, or making it a fully interactive app: give it a list of files and a description, and it both changes the file and creates a bug. There may be a short-term solution and a long-term solution. The short term solution seems to be for someone (assumedly the merger) to add
[chromium-dev] Re: Mac/Linux heads-up: RenderWidgetHostView changes needed
On Mon, Apr 13, 2009 at 12:13 PM, Darin Fisher da...@chromium.org wrote: The Mac implementation is wrong. On a single core machine, the renderer could starve the UI thread of the browser, preventing any pixels from ever being updated on the screen. That seems wrong--the OS-prompted draw should be just blitting from the backing store, right? But I'm more concerned about this starvation problem--the renderer should not be able to starve the UI thread under any circumstances--we always need to return to the main runloop every second or two or the OS will decide we're unresponsive and beachball. What code path can cause this? --Amanda --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Creating bugs from test_expectations.txt
On Mon, Apr 13, 2009 at 9:35 AM, Pam Greene p...@chromium.org wrote: On Mon, Apr 13, 2009 at 9:09 AM, Glenn Wilson gwil...@chromium.orgwrote: Yes, the intention is to have something maintainable that gets run as part of the regular testing/merging/etc. process -- which means eventually rewriting in python. I wrote this in Java because we had a java lib for automatically creating bugs in the issue tracker, and I wanted to get it up and running quickly. We should have a python API for doing the same thing. I can easily change the script so it marks DEFERs as P3s. However, currently, the script doesn't alter any entries in the file that already have bugs -- meaning something like BUG1234 DEFER : ... or WONTFIX DEFER : ... won't change. Is this the correct behavior? The end goal is to take DEFER out and let the bug priorities run the show. But if we're not entirely confident that this will work (both the script and the human tracking process afterward), it's easy enough to strip the DEFER modifiers later. Makes sense to me. Lets leave in the defer modifiers, but mark any *new* bugs for deferred tests as P3s. The current behavior of leaving existing bugs unaltered seems correct to me. BTW, doing it in Java for expediency sake is totally reasonable. Ojan - Pam I'm planning on running the script later today with the most recent version of test_expectations.txt -- there will be 200+ new bugs. Let me know if I should hold off on doing so. Ojan, I'll send you the review for test_expecations.txt. Regards, Glenn On Fri, Apr 10, 2009 at 4:36 PM, Ojan Vafai o...@google.com wrote: I hate to ask this, but any chance we could rewrite it in python? It would be a lot less code in python (one script file) and would match the rest of our codebase (which currently does all scripting in python). I'm mainly worried about usability and maintainability here. That all said, I think you should go ahead and run this script as a one off so we can get the test lists having bug IDs ASAP as that's kind of urgent to us being able to track the current state of layout tests. Then we can worry about checking in a python version later if we decide it's necessary. Pam, Darin, Jon, Mark: You might want to confirm that my request below makes sense with respect to future handling/triaging of layout tests bugs. Also for the initial one-off version, can you make any tests that are currently marked as DEFER be P3s? Then when you spit out the results to the test_expectations file, I don't think there's any need to include the DEFER anymore as we'll rely entirely on bug priorities to decide which tests need fixing for the next release (i.e. all P1 tests and maybe some P2 tests). That way there will be less initial triaging to do in order to make sure we keep the tree shippable. Eventually we'll have to go through all the P3s in the Untriaged state and up some of them to P2s, but there is no pressing need to do so right away. Ojan On Fri, Apr 10, 2009 at 3:42 PM, Glenn Wilson gwil...@chromium.orgwrote: (Ack...resending) Ok, I have the CL ready, if anyone with Java readability would be willing to do a review, please let me know. more inline... On Fri, Apr 10, 2009 at 2:07 PM, Pam Greene p...@chromium.org wrote: On Thu, Apr 9, 2009 at 6:49 PM, Ojan Vafai o...@google.com wrote: At a quick glance, this looks great. I didn't look over every bug, but the ones I did look at look good. Yep. You'll want some sort of default description for the ones that have none. I ran the script on a small test file...an example bug is here: http://code.google.com/p/chromium/issues/detail?id=9991 There's at least a small bit of indication of where it came from. I also added the line numbers from test_expectations.txt that generated the bug. Additionally, I also wrote the script to then add the created bug numbers to the file. This ends up re-arranging some of the flags (DEBUG DEFER ... etc. -- DEFER DEBUG ...), but all data is retained. 200+ bugs is certainly too many, but that's no reason not to file them. (Sorry, couldn't resist. Seriously, yes, definitely file them. Better in the issue tracker than getting lost.) There are probably some improvements to be had to better group some of the bugs, but it's probably not an order of magnitude improvement. I just didn't want to create tons of bugs that were not useful. It would be great to check in a version of this script that we could run when a number of tests fail (e.g. when doing a bad webkit merge). That way, we can add them all to the local test_expectations.txt file and have it spit out the new results. Fixing the file we have and moving forward are slightly different use cases, but yes. In the long run, we shouldn't need any script to fix an existing bug-less expectations file, only the part that adds bugs for newly added tests. I'm not sure whether that part should be controlled by
[chromium-dev] Re: question on profiles ChromeThread
I don't know of a reason we need more than 1 history thread, but I can't say for sure. HistoryService is responsible today for starting/stopping/destroying the history thread, so if we have multiple services then we need to be smarter about that... Anyone opposed to moving history_thread to BrowserProcess and mimic what is done for the other threads? HistoryService::Cleanup does some fancy footwork and ultimately joins on it's thread, but afaict it doesn't need to [join]. I can think of other options but they feel much less cool, like saying that the default profile's history service manges the history thread, or having a refcountable HistoryThread type, ... On Sat, Apr 11, 2009 at 3:51 PM, Adam Barth aba...@chromium.org wrote: Oops. Wrong address. See below: On Sat, Apr 11, 2009 at 3:47 PM, Adam Barth abarth@ wrote: That sound fine. The other option is to make the history thread not well-known (i.e., take away its ChromeThread::HISTORY name). Is there a reason to have N history threads with N profiles? Adam On Sat, Apr 11, 2009 at 3:02 PM, Tim Steele t...@chromium.org wrote: I promise I'm not back to my antics of suggesting we stick to the one profile per process model, but I hit a little snag trying to write a test where I want to create a different profile. I'm wondering what I'm doing wrong / how this was originally intended to work in the multiple profile per process world... willing to fix things if need be. It's an InProcessBrowserTest, so I've got a standard Browser object already up and running with the Default profile for the test user data directory. I then use ProfileManager to CreateProfile; so far so good. As soon as you try and do anything with this profile that touches HistoryService (for example), we choke because the lazy-initializer for the second profile's HistoryService wants to create a ChromeThread::HISTORY, which already exists. This is something we don't hit with incognito because it reuses the same service, but I think a long, long time ago when we had some notion of true multi-profile support this must have worked. I assume the original intent was to re-use the same history thread for multiple profiles. Not sure how to make that work at the moment, ChromeThread is pretty tight-lipped from an API standpoint. Naively it feels like we could just check-if-exists-and-reuse-else-create in this case. Is that sane? Tim --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Mac/Linux heads-up: RenderWidgetHostView changes needed
On Mon, Apr 13, 2009 at 10:59 AM, Adam Langley a...@chromium.org wrote: On Fri, Apr 10, 2009 at 1:16 PM, Peter Kasting pkast...@google.com wrote: I strongly suggest picking Darin's brain directly for more detail as he has a stronger grasp of this than I do :) I don't want to block landing this patch because of Linux. However, I would currently just #ifdef in the old behaviour which I believe is correct for Linux. Certainly doing that is a safe. Perhaps not safe, but not worse. However, I'd rather wait to land than #ifdef. The old behavior is certainly wrong (did you not see my + Darin's emails above?), and ifdefs are ugly. There's nothing urgently pushing me to get this in in the next few hours. PK --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Mac/Linux heads-up: RenderWidgetHostView changes needed
On Mon, Apr 13, 2009 at 11:12 AM, Peter Kasting pkast...@google.com wrote: Perhaps not safe, but not worse. It's not that don't want to deal with this right now, it's that I believe that the current behaviour is correct on Linux. I've reread your patch and email and I haven't understood any arguments about why there's a problem. (I get that there might be something about the priority of Windows WM_PAINT messages that causes an issue, but that obviously doesn't apply with X.) AGL --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Mac/Linux heads-up: RenderWidgetHostView changes needed
On Mon, Apr 13, 2009 at 11:28 AM, Adam Langley a...@chromium.org wrote: It's not that don't want to deal with this right now, it's that I believe that the current behaviour is correct on Linux. Yes, I grasped that. I've reread your patch and email and I haven't understood any arguments about why there's a problem. (I get that there might be something about the priority of Windows WM_PAINT messages that causes an issue, but that obviously doesn't apply with X.) The reason I had to write a patch at all is a cross-platform issue, not a Windows-specific one. The Windows-specific stuff in the comments is merely an example of why an API like DidPaintRect() is useful in the first place, but we're not adding that API in this patch, just explaining what's going on. The issue I'm fixing is that if we get an updated rect from the renderer while DidPaintRect() is disabled, we don't repaint any extra damaged area in that rect until the next time it happens to get damaged, because we don't send anyone any notifications. This is because we were putting in an anti-recursion check in the model when instead it should have been in the view. Fixing this means that any platform that (correctly) synchrously paints from DidPaintRect() is now at risk of infinite recursion, and the new DCHECK is designed to detect this. It fires on Linux because Linux was doing the right thing before; it doesn't fire on Mac and at some point Avi will fix the Mac implementation. To fix _this_ issue the platform-specific code needs to have its own anti-recursion check, as Windows now has. This is what I'm asking Linux to write. You write one by detecting (inside DidPaintRect()) that you're already in a paint and not recursively painting again. PK --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Mac/Linux heads-up: RenderWidgetHostView changes needed
On Mon, Apr 13, 2009 at 11:47 AM, Peter Kasting pkast...@google.com wrote:
The issue I'm fixing is that if we get an updated rect from the renderer
while DidPaintRect() is disabled, we don't repaint any extra damaged area in
that rect until the next time it happens to get damaged, because we don't
send anyone any notifications. This is because we were putting in an
anti-recursion check in the model when instead it should have been in the
view.
I always thought that, when we paint without a backing store, we're
painting the whole thing so this isn't an issue.
After patching in your change, the following makes Linux work for me. Hopefully
I have the correct end of the stick this time.
Gmail will probably mangle this patch, so it's attached as well.
diff --git a/chrome/browser/renderer_host/render_widget_host_view_gtk.cc
b/chrome/browser/renderer_host/render_widget_host_view_gtk.cc
index 32135f9..eb1b966 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_gtk.cc
+++ b/chrome/browser/renderer_host/render_widget_host_view_gtk.cc
@@ -159,6 +159,7 @@
RenderWidgetHostViewGtk::RenderWidgetHostViewGtk(RenderWidgetHost*
widget_host)
parent_(NULL),
popup_signal_id_(0),
activatable_(true),
+ about_to_validate_and_paint_(false),
is_loading_(false) {
host_-set_view(this);
}
@@ -277,7 +278,10 @@ void RenderWidgetHostViewGtk::IMEUpdateStatus(int control,
}
void RenderWidgetHostViewGtk::DidPaintRect(const gfx::Rect rect) {
- Paint(rect);
+ if (about_to_validate_and_paint_)
+invalid_rect_ = invalid_rect_.Union(rect);
+ else
+Paint(rect);
}
void RenderWidgetHostViewGtk::DidScrollRect(const gfx::Rect rect, int dx,
@@ -338,7 +342,13 @@ void
RenderWidgetHostViewGtk::PasteFromSelectionClipboard() {
}
void RenderWidgetHostViewGtk::Paint(const gfx::Rect damage_rect) {
+ DCHECK(!about_to_validate_and_paint_);
+
+ invalid_rect_ = damage_rect;
+ about_to_validate_and_paint_ = true;
BackingStore* backing_store = host_-GetBackingStore();
+ // Calling GetBackingStore maybe have changed |invalid_rect_|...
+ about_to_validate_and_paint_ = false;
GdkWindow* window = view_.get()-window;
if (backing_store) {
@@ -347,7 +357,7 @@ void RenderWidgetHostViewGtk::Paint(const
gfx::Rect damage_rect) {
// Destroy()ed yet and it receives paint messages...
if (window) {
backing_store-ShowRect(
- damage_rect, x11_util::GetX11WindowFromGtkWidget(view_.get()));
+ invalid_rect_, x11_util::GetX11WindowFromGtkWidget(view_.get()));
}
} else {
if (window)
diff --git a/chrome/browser/renderer_host/render_widget_host_view_gtk.h
b/chrome/browser/renderer_host/render_widget_host_view_gtk.h
index 215549d..cd49a8d 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_gtk.h
+++ b/chrome/browser/renderer_host/render_widget_host_view_gtk.h
@@ -101,6 +101,12 @@ class RenderWidgetHostViewGtk : public
RenderWidgetHostView {
// activatable popup: select dropdown. Example of non-activatable popup:
// form autocomplete.
bool activatable_;
+ // This is true when we are currently painting and thus should handle extra
+ // paint requests by expanding the invalid rect rather than actually
+ // painting.
+ bool about_to_validate_and_paint_;
+ // This is the rectangle which we'll paint.
+ gfx::Rect invalid_rect_;
// Whether we are currently loading.
bool is_loading_;
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
patch
Description: Binary data
[chromium-dev] Re: question on profiles ChromeThread
On Mon, Apr 13, 2009 at 11:01 AM, Tim Steele t...@chromium.org wrote: I don't know of a reason we need more than 1 history thread, but I can't say for sure. HistoryService is responsible today for starting/stopping/destroying the history thread, so if we have multiple services then we need to be smarter about that... Anyone opposed to moving history_thread to BrowserProcess and mimic what is done for the other threads? HistoryService::Cleanup does some fancy footwork and ultimately joins on it's thread, but afaict it doesn't need to [join]. I can think of other options but they feel much less cool, like saying that the default profile's history service manges the history thread, or having a refcountable HistoryThread type, ... We need the UI thread to Join with the history thread to ensure that all the data is written. If it doesn't join, the history thread will be force-terminated when the UI thread exits. However, the BrowserProcess destructor should do this join for you when it deletes the thread. I don't think there's any reason other than completeness that the history thread needs a name, so just removing that is the easiest thing. As you can see, history shutdown is tricky. But I don't have an objection if you want to move it to BrowserProcess, either, as long as you fix any problems that arise :) Brett --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: localStorage design document - please offer feedback!
I've gotten very few comments. I assume no news is good news? :-) On Tue, Apr 7, 2009 at 9:27 PM, Jeremy Orlow jor...@chromium.org wrote: Actually, if you want to edit, try these links: http://docs.google.com/a/chromium.org/Doc?id=dhs4g97m_0hjv3nqc7hl=en http://docs.google.com/a/chromium.org/Doc?id=dhs4g97m_1v79p3wddhl=en http://docs.google.com/a/chromium.org/Doc?id=dhs4g97m_2c2bvftf6hl=en On Tue, Apr 7, 2009 at 9:12 PM, Jeremy Orlow jor...@chromium.org wrote: After several chromium-dev and whatwg email threads, I think the major issues with the spec and how to implement localStorage have been hammered out. I've written up my current thoughts in terms of a couple documents (listed below). This is my first major chunk of code in Chromium, so I've tried to be as precise as possible. There are a couple of sections highlighted in yellow that are things I'm not sure about (and thus need to research more). If I say anything in the doc that seems suspect, please point it out! I've made these writable for anyone with a chromium.org account, so feel free to make comments inline using the 'Insert Comment' menu. I'll either incorporate the feedback into the doc or reply in this thread. http://docs.google.com/Doc?id=dhs4g97m_0hjv3nqc7 - DOM Storage Overview http://docs.google.com/Doc?id=dhs4g97m_1v79p3wdd - LocalStorage http://docs.google.com/Doc?id=dhs4g97m_2c2bvftf6 - SessionStorage Thanks a lot! Jeremy --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Creating bugs from test_expectations.txt
Ok, I ran the script and created 211 bugs (in the ~10270 - 10500 range). As far as I can tell, all the bugs were created successfullythe script now picks up 0 tests without bugs. Ojan, I sent you the review of test_expectations: http://codereview.chromium.org/73026/show Two things I noticed: 1. The script didn't recognize layout tests in chrome/ and not in LayoutTests/ -- it's only a handful, so updating those by hand shouldn't take very long. 2. The script re-arranged some of the flags, as expected. If it is important that we keep the prior ordering of the flags, let me know. Also, I'd love to get this script reviewed and checked in, if anyone would be willing to do a Java review. Regards, Glenn On Mon, Apr 13, 2009 at 10:19 AM, Ojan Vafai o...@google.com wrote: On Mon, Apr 13, 2009 at 9:35 AM, Pam Greene p...@chromium.org wrote: On Mon, Apr 13, 2009 at 9:09 AM, Glenn Wilson gwil...@chromium.orgwrote: Yes, the intention is to have something maintainable that gets run as part of the regular testing/merging/etc. process -- which means eventually rewriting in python. I wrote this in Java because we had a java lib for automatically creating bugs in the issue tracker, and I wanted to get it up and running quickly. We should have a python API for doing the same thing. I can easily change the script so it marks DEFERs as P3s. However, currently, the script doesn't alter any entries in the file that already have bugs -- meaning something like BUG1234 DEFER : ... or WONTFIX DEFER : ... won't change. Is this the correct behavior? The end goal is to take DEFER out and let the bug priorities run the show. But if we're not entirely confident that this will work (both the script and the human tracking process afterward), it's easy enough to strip the DEFER modifiers later. Makes sense to me. Lets leave in the defer modifiers, but mark any *new* bugs for deferred tests as P3s. The current behavior of leaving existing bugs unaltered seems correct to me. BTW, doing it in Java for expediency sake is totally reasonable. Ojan - Pam I'm planning on running the script later today with the most recent version of test_expectations.txt -- there will be 200+ new bugs. Let me know if I should hold off on doing so. Ojan, I'll send you the review for test_expecations.txt. Regards, Glenn On Fri, Apr 10, 2009 at 4:36 PM, Ojan Vafai o...@google.com wrote: I hate to ask this, but any chance we could rewrite it in python? It would be a lot less code in python (one script file) and would match the rest of our codebase (which currently does all scripting in python). I'm mainly worried about usability and maintainability here. That all said, I think you should go ahead and run this script as a one off so we can get the test lists having bug IDs ASAP as that's kind of urgent to us being able to track the current state of layout tests. Then we can worry about checking in a python version later if we decide it's necessary. Pam, Darin, Jon, Mark: You might want to confirm that my request below makes sense with respect to future handling/triaging of layout tests bugs. Also for the initial one-off version, can you make any tests that are currently marked as DEFER be P3s? Then when you spit out the results to the test_expectations file, I don't think there's any need to include the DEFER anymore as we'll rely entirely on bug priorities to decide which tests need fixing for the next release (i.e. all P1 tests and maybe some P2 tests). That way there will be less initial triaging to do in order to make sure we keep the tree shippable. Eventually we'll have to go through all the P3s in the Untriaged state and up some of them to P2s, but there is no pressing need to do so right away. Ojan On Fri, Apr 10, 2009 at 3:42 PM, Glenn Wilson gwil...@chromium.orgwrote: (Ack...resending) Ok, I have the CL ready, if anyone with Java readability would be willing to do a review, please let me know. more inline... On Fri, Apr 10, 2009 at 2:07 PM, Pam Greene p...@chromium.org wrote: On Thu, Apr 9, 2009 at 6:49 PM, Ojan Vafai o...@google.com wrote: At a quick glance, this looks great. I didn't look over every bug, but the ones I did look at look good. Yep. You'll want some sort of default description for the ones that have none. I ran the script on a small test file...an example bug is here: http://code.google.com/p/chromium/issues/detail?id=9991 There's at least a small bit of indication of where it came from. I also added the line numbers from test_expectations.txt that generated the bug. Additionally, I also wrote the script to then add the created bug numbers to the file. This ends up re-arranging some of the flags (DEBUG DEFER ... etc. -- DEFER DEBUG ...), but all data is retained. 200+ bugs is certainly too many, but that's no reason not to file them. (Sorry, couldn't resist. Seriously, yes, definitely file them. Better in the
