66706 2011-08-22 13:25:20 -0700 Change return type of StringImpl::characters() to StringCharacterIterator 2012-01-22 14:28:41 -0800 1 1 1 Unclassified WebKit Web Template Framework 528+ (Nightly build) Unspecified Unspecified RESOLVED WONTFIX P2 Normal --- 66286 66161 1 sullivan sullivan abarth andersca darin jamesr msaboff rniwa sam simonjam wangxianzhu webkit.review.bot oldest_to_newest 454847 0 sullivan 2011-08-22 13:25:20 -0700 In order to fix bug 66161 (storing strings in 8-bit buffers when possible), we want to reduce the number of places where the underlying data format in WTF::String is exposed. The main thing that needs to get fixed are ~200 usages of the String::characters() method. After discussion in bug 66286, we think the best approach to the conversion is the following: 1. Typedef const UChar * (the return value of characters() ) StringIterator, and make characters() return StringIterator. 2. Replace usages of const UChar* in callers with StringIterator. At this point, the functionality of the code hasn't changed at all, but it would be possible to implement StringIterator with operator[], etc. in a way that hides the underlying data format of the string. 3. Write various implementations of StringIterator and do performance analysis on how they affect the entire codebase (as opposed to relying solely on microbenchmarks). 4. Pick a performant implementation of StringIterator, and actually convert the code to use 8-bit buffers when possible using this implementation. This bug addresses step 1. I pulled it out of one of the patches from bug 66286 because there is a lot of discussion on that bug and I wanted to make sure what we're doing here is clear. 454849 1 104726 sullivan 2011-08-22 13:26:43 -0700 Created attachment 104726 Patch 454856 2 darin 2011-08-22 13:32:00 -0700 A string iterator sounds like something that iterates across strings. This iterates across characters within a string. I think this should be named StringCharacterIterator. 454864 3 104729 sullivan 2011-08-22 13:43:46 -0700 Created attachment 104729 Patch Updated name from StringIterator to StringCharacterIterator 454867 4 webkit.review.bot 2011-08-22 13:46:37 -0700 Attachment 104729 did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2 Last 3072 characters of output: chromium.org/svn/trunk/deps/third_party/libjpeg_turbo@95800 should_process: True processed: True requirements: ['./', 'chromium_deps', 'third_party'] name: third_party/v8-i18n url: From('chromium_deps', 'src/third_party/v8-i18n') parsed_url: http://v8-i18n.googlecode.com/svn/trunk@4 should_process: True processed: True requirements: ['./', 'chromium_deps', 'third_party'] name: tools/grit url: http://src.chromium.org/svn/trunk/src/tools/grit@97698 parsed_url: http://src.chromium.org/svn/trunk/src/tools/grit@97698 should_process: True processed: True requirements: ['./'] name: base url: http://src.chromium.org/svn/trunk/src/base@97698 parsed_url: http://src.chromium.org/svn/trunk/src/base@97698 _file_list: ['/mnt/git/webkit-style-queue/Source/WebKit/chromium/base/event_recorder.cc', '/mnt/git/webkit-style-queue/Source/WebKit/chromium/base/event_recorder_win.cc', '/mnt/git/webkit-style-queue/Source/WebKit/chromium/base/base.gypi'] should_process: True processed: True requirements: ['./'] name: third_party/leveldb url: From('chromium_deps', 'src/third_party/leveldb') should_process: True requirements: ['./', 'chromium_deps', 'third_party'] name: sql url: http://src.chromium.org/svn/trunk/src/sql@97698 should_process: True requirements: ['./'] name: v8 url: From('chromium_deps', 'src/v8') should_process: True requirements: ['./', 'chromium_deps'] name: testing/gtest url: From('chromium_deps', 'src/testing/gtest') should_process: True requirements: ['./', 'chromium_deps', 'testing'] name: third_party/libwebp url: http://src.chromium.org/svn/trunk/src/third_party/libwebp@97698 should_process: True requirements: ['./', 'third_party'] name: googleurl url: From('chromium_deps', 'src/googleurl') should_process: True requirements: ['./', 'chromium_deps'] name: third_party/skia/include url: From('chromium_deps', 'src/third_party/skia/include') should_process: True requirements: ['./', 'chromium_deps', 'third_party'] name: third_party/ots url: From('chromium_deps', 'src/third_party/ots') should_process: True requirements: ['./', 'chromium_deps', 'third_party'] name: third_party/snappy/src url: From('chromium_deps', 'src/third_party/snappy/src') should_process: True requirements: ['./', 'chromium_deps', 'third_party'] ________ running 'svn update /mnt/git/webkit-style-queue/Source/WebKit/chromium/base --revision 97698 --ignore-externals' in '/mnt/git/webkit-style-queue/Source/WebKit/chromium' D /mnt/git/webkit-style-queue/Source/WebKit/chromium/base/event_recorder.cc A /mnt/git/webkit-style-queue/Source/WebKit/chromium/base/event_recorder_win.cc U /mnt/git/webkit-style-queue/Source/WebKit/chromium/base/base.gypi Updated to revision 97698. Died at Tools/Scripts/update-webkit-chromium line 69. No such file or directory at Tools/Scripts/update-webkit line 100. If any of these errors are false positives, please file a bug against check-webkit-style. 454879 5 sullivan 2011-08-22 13:53:15 -0700 I think there is something wrong with style-queue; event_recorder_win.cc isn't even in my patch. The style check passed locally when I uploaded the patch. 454880 6 104729 rniwa 2011-08-22 13:53:40 -0700 Comment on attachment 104729 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104729&action=review > Source/JavaScriptCore/wtf/text/StringImpl.h:68 > +typedef const UChar* StringCharacterIterator; This is going to be a class, right? I think we should define a class here and modify the call sites of characters() to use StringCharacterIterator. By the way, we already have TextIterator and CharacterIterator and they have completely different semantics so I'm not sure if we should name this class StringCharacterIterator. How about ConstCharacterAccessor? 454904 7 sullivan 2011-08-22 14:11:23 -0700 (In reply to comment #6) > (From update of attachment 104729 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=104729&action=review > > > Source/JavaScriptCore/wtf/text/StringImpl.h:68 > > +typedef const UChar* StringCharacterIterator; > > This is going to be a class, right? I think we should define a class here and modify the call sites of characters() to use StringCharacterIterator. Eventually it will be a class, yes. But we can make much smaller and less risky changes if we start with the typedef. Using the typedef doesn't change the performance or functionality at all. And we can update the code in smaller patches, instead of making a giant patch that changes all 200 callers and the subsequent calls they make, and adds in a new class at the same time. So when we get to step 3 of the plan in the bug description above, we can try various implementations of the class and test performance on various benchmarks, and check in/roll back that single change on its own. > By the way, we already have TextIterator and CharacterIterator and they have completely different semantics so I'm not sure if we should name this class StringCharacterIterator. > > How about ConstCharacterAccessor? I don't have a strong opinion. Darin, what do you think? 454911 8 104729 darin 2011-08-22 14:19:18 -0700 Comment on attachment 104729 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104729&action=review >>> Source/JavaScriptCore/wtf/text/StringImpl.h:68 >>> +typedef const UChar* StringCharacterIterator; >> >> This is going to be a class, right? I think we should define a class here and modify the call sites of characters() to use StringCharacterIterator. >> By the way, we already have TextIterator and CharacterIterator and they have completely different semantics so I'm not sure if we should name this class StringCharacterIterator. >> >> How about ConstCharacterAccessor? > > Eventually it will be a class, yes. But we can make much smaller and less risky changes if we start with the typedef. Using the typedef doesn't change the performance or functionality at all. And we can update the code in smaller patches, instead of making a giant patch that changes all 200 callers and the subsequent calls they make, and adds in a new class at the same time. > > So when we get to step 3 of the plan in the bug description above, we can try various implementations of the class and test performance on various benchmarks, and check in/roll back that single change on its own. I recognize that we already have a CharacterIterator, but it seems to me that StringCharacterIterator is clearly something related to the string class. Another possibility would be to name it String::CharacterIterator. Our existing CharacterIterator could be renamed and given a slightly longer, clearer name. It's used in a small enough number of places that it would be straightforward. I do think it’s a problem for the name StringCharacterIterator that this class is for read-only iteration and “iterator” usually means read-write access. It does indeed seem fine to start using this typedef to keep the size of the patch smaller once it becomes a class instead of a typedef. But it might give us a false sense of accomplishment. Some things, such as passing a const UChar* to external library functions such as CFStringCreateWithCharacters, won’t work at all with the iterator. And we won’t uncover those until we make it a class. So we may be putting this typedef in places where it does no good. If I was doing this, I would do it more like this: 1) Check in the typedef. 2) Locally change to a class. 3) Start landing changes to use the typedef in the tree, having already tested with the class that I have not yet checked in. 4) Land the real class when the time is right. 454941 9 sullivan 2011-08-22 14:42:44 -0700 (In reply to comment #8) > (From update of attachment 104729 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=104729&action=review > > >>> Source/JavaScriptCore/wtf/text/StringImpl.h:68 > >>> +typedef const UChar* StringCharacterIterator; > >> > >> This is going to be a class, right? I think we should define a class here and modify the call sites of characters() to use StringCharacterIterator. > >> By the way, we already have TextIterator and CharacterIterator and they have completely different semantics so I'm not sure if we should name this class StringCharacterIterator. > >> > >> How about ConstCharacterAccessor? > > > > Eventually it will be a class, yes. But we can make much smaller and less risky changes if we start with the typedef. Using the typedef doesn't change the performance or functionality at all. And we can update the code in smaller patches, instead of making a giant patch that changes all 200 callers and the subsequent calls they make, and adds in a new class at the same time. > > > > So when we get to step 3 of the plan in the bug description above, we can try various implementations of the class and test performance on various benchmarks, and check in/roll back that single change on its own. > > I recognize that we already have a CharacterIterator, but it seems to me that StringCharacterIterator is clearly something related to the string class. Another possibility would be to name it String::CharacterIterator. > > Our existing CharacterIterator could be renamed and given a slightly longer, clearer name. It's used in a small enough number of places that it would be straightforward. I'd be happy to file a blocking bug and write a patch for this. Looks like both CharacterIterator and BackwardsCharacterIterator would be changed, right? Any opinions on the new name? TextCharacterIterator/BackwardsTextCharacterIterator? > I do think it’s a problem for the name StringCharacterIterator that this class is for read-only iteration and “iterator” usually means read-write access. That's true. Do you think ConstCharacterAccessor captures that better? Or maybe something like String::ReadOnlyCharIterator would be better? > It does indeed seem fine to start using this typedef to keep the size of the patch smaller once it becomes a class instead of a typedef. But it might give us a false sense of accomplishment. Some things, such as passing a const UChar* to external library functions such as CFStringCreateWithCharacters, won’t work at all with the iterator. And we won’t uncover those until we make it a class. So we may be putting this typedef in places where it does no good. > > If I was doing this, I would do it more like this: > > 1) Check in the typedef. > 2) Locally change to a class. > 3) Start landing changes to use the typedef in the tree, having already tested with the class that I have not yet checked in. > 4) Land the real class when the time is right. In practice, we will definitely do something more like this. Sorry for oversimplifying in my comments so far; I wanted to be clear what order the patches would be submitted in. 454952 10 darin 2011-08-22 14:49:12 -0700 (In reply to comment #9) > I'd be happy to file a blocking bug and write a patch for this. Looks like both CharacterIterator and BackwardsCharacterIterator would be changed, right? Any opinions on the new name? TextCharacterIterator/BackwardsTextCharacterIterator? That seems OK. > > I do think it’s a problem for the name StringCharacterIterator that this class is for read-only iteration and “iterator” usually means read-write access. > > That's true. Do you think ConstCharacterAccessor captures that better? Or maybe something like String::ReadOnlyCharIterator would be better? The word “const” seems OK if not great. I think ReadOnly is slightly better but more wordy. The word “accessor” seems clearly not as good as iterator to express the fact that it is something you move from one character to the next. The abbreviation “Char” is not to my taste. > > If I was doing this, I would do it more like this: > > > > 1) Check in the typedef. > > 2) Locally change to a class. > > 3) Start landing changes to use the typedef in the tree, having already tested with the class that I have not yet checked in. > > 4) Land the real class when the time is right. > > In practice, we will definitely do something more like this. Sorry for oversimplifying in my comments so far; I wanted to be clear what order the patches would be submitted in. I am not sure we are understanding each other. This patch changes lots of call sites to use the iterator class instead of const UChar*. Have those call sites been tested with a class? If we were doing my (3) then we would have had to test these with the class and so we would know which ones were truly “iteration” and which were other kinds of const UChar* uses. 454969 11 sullivan 2011-08-22 15:03:42 -0700 (In reply to comment #10) > (In reply to comment #9) > > I'd be happy to file a blocking bug and write a patch for this. Looks like both CharacterIterator and BackwardsCharacterIterator would be changed, right? Any opinions on the new name? TextCharacterIterator/BackwardsTextCharacterIterator? > > That seems OK. Okay, I'll do that. > > > I do think it’s a problem for the name StringCharacterIterator that this class is for read-only iteration and “iterator” usually means read-write access. > > > > That's true. Do you think ConstCharacterAccessor captures that better? Or maybe something like String::ReadOnlyCharIterator would be better? > > The word “const” seems OK if not great. I think ReadOnly is slightly better but more wordy. The word “accessor” seems clearly not as good as iterator to express the fact that it is something you move from one character to the next. The abbreviation “Char” is not to my taste. Without abbreviating, StringReadOnlyCharacterIterator is the best name I can think of. Should I change the patch to that? > > > If I was doing this, I would do it more like this: > > > > > > 1) Check in the typedef. > > > 2) Locally change to a class. > > > 3) Start landing changes to use the typedef in the tree, having already tested with the class that I have not yet checked in. > > > 4) Land the real class when the time is right. > > > > In practice, we will definitely do something more like this. Sorry for oversimplifying in my comments so far; I wanted to be clear what order the patches would be submitted in. > > I am not sure we are understanding each other. This patch changes lots of call sites to use the iterator class instead of const UChar*. Have those call sites been tested with a class? If we were doing my (3) then we would have had to test these with the class and so we would know which ones were truly “iteration” and which were other kinds of const UChar* uses. The patch on bug 66286 does indeed change lots of callsites, but the new patch I posted on this bug is just the typedef. I'm sorry if I'm still misunderstanding something? 454978 12 rniwa 2011-08-22 15:09:59 -0700 (In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #9) > > > I'd be happy to file a blocking bug and write a patch for this. Looks like both CharacterIterator and BackwardsCharacterIterator would be changed, right? Any opinions on the new name? TextCharacterIterator/BackwardsTextCharacterIterator? > > > > That seems OK. > > Okay, I'll do that. I'd prefer keeping CharacterIterator and BackwardsCharacterIterator as is, and add String::ConstIterator. 454979 13 sullivan 2011-08-22 15:11:08 -0700 (In reply to comment #12) > (In reply to comment #11) > > (In reply to comment #10) > > > (In reply to comment #9) > > > > I'd be happy to file a blocking bug and write a patch for this. Looks like both CharacterIterator and BackwardsCharacterIterator would be changed, right? Any opinions on the new name? TextCharacterIterator/BackwardsTextCharacterIterator? > > > > > > That seems OK. > > > > Okay, I'll do that. > > I'd prefer keeping CharacterIterator and BackwardsCharacterIterator as is, and add String::ConstIterator. Darin, is that okay with you? 454990 14 darin 2011-08-22 15:18:09 -0700 (In reply to comment #13) > > I'd prefer keeping CharacterIterator and BackwardsCharacterIterator as is, and add String::ConstIterator. > > Darin, is that okay with you? I don’t think it will be easy to make a type used in StringImpl be defined as a member of the String class. The name seems OK. 454999 15 sullivan 2011-08-22 15:25:04 -0700 > > I am not sure we are understanding each other. This patch changes lots of call sites to use the iterator class instead of const UChar*. Have those call sites been tested with a class? If we were doing my (3) then we would have had to test these with the class and so we would know which ones were truly “iteration” and which were other kinds of const UChar* uses. > > The patch on bug 66286 does indeed change lots of callsites, but the new patch I posted on this bug is just the typedef. I'm sorry if I'm still misunderstanding something? Wait, I think I see where I was misunderstanding you. What I did was change the return value of characters(), assuming all callsites would be StringIterator, when it's clearly true that at least a few will really need access to the underlying bytes. Instead, would you prefer we leave characters() alone and add a second function which returns StringIterator, then in step 3 we change callsites to use it once we've verified that they're really iterators? (We'd use the preferred name, of course). 455000 16 sullivan 2011-08-22 15:25:26 -0700 (In reply to comment #14) > (In reply to comment #13) > > > I'd prefer keeping CharacterIterator and BackwardsCharacterIterator as is, and add String::ConstIterator. > > > > Darin, is that okay with you? > > I don’t think it will be easy to make a type used in StringImpl be defined as a member of the String class. The name seems OK. Oops, right. Is StringConstIterator okay? 455005 17 darin 2011-08-22 15:32:26 -0700 (In reply to comment #15) > What I did was change the return value of characters() Maybe you did that in your private tree. But in the public tree we are just using a typedef and it's still the same return type as before, so we did not yet change the return type of characters(). > Instead, would you prefer we leave characters() alone and add a second function which returns StringIterator I don’t really care all that much about what the function is named. It might be clearer to name it characterIterator() rather than characters(). Having two functions is OK if it helps us test what places will work with the iterator in the future. We should only deploy the new typedef at call sites where we know it will work. I don’t care precisely how we do that testing, but once we know it will work, then it’s great to land the change now in a way that is safe and has no effect until we land more of the work later. It’s not as great to land it places where it was not tested, might not work, and might have to rewritten later. I’d rather have those call sites say const UChar* rather than being churned multiple times. 455007 18 darin 2011-08-22 15:33:14 -0700 (In reply to comment #16) > Oops, right. Is StringConstIterator okay? I guess we’re back to the original name, StringIterator, only this time we added “const”. I still have my original objection. It seems like an iterator that iterates over strings, not over characters within a string, to me. 455029 19 sullivan 2011-08-22 16:03:35 -0700 (In reply to comment #18) > (In reply to comment #16) > > Oops, right. Is StringConstIterator okay? > > I guess we’re back to the original name, StringIterator, only this time we added “const”. I still have my original objection. It seems like an iterator that iterates over strings, not over characters within a string, to me. StringConstCharacterIterator is slightly shorter than StringReadOnlyCharacterIterator. And I'm assuming you don't want to abbreviate "str", "char", "iter", etc. So I think that's the best we can do? (In reply to comment #17) > (In reply to comment #15) > > What I did was change the return value of characters() > > Maybe you did that in your private tree. But in the public tree we are just using a typedef and it's still the same return type as before, so we did not yet change the return type of characters(). > > > Instead, would you prefer we leave characters() alone and add a second function which returns StringIterator > > I don’t really care all that much about what the function is named. It might be clearer to name it characterIterator() rather than characters(). Having two functions is OK if it helps us test what places will work with the iterator in the future. I think having the two functions will help us test. > We should only deploy the new typedef at call sites where we know it will work. I don’t care precisely how we do that testing, but once we know it will work, then it’s great to land the change now in a way that is safe and has no effect until we land more of the work later. > > It’s not as great to land it places where it was not tested, might not work, and might have to rewritten later. I’d rather have those call sites say const UChar* rather than being churned multiple times. We'll definitely test call sites before changing them. 455057 20 104765 sullivan 2011-08-22 16:46:04 -0700 Created attachment 104765 Patch Updated name to StringConstCharacterIterator and added characterIterator() function that returns StringConstCharacterIterator instead of const UChar* 540318 21 sam 2012-01-22 14:28:41 -0800 StringImpl supports 8-bit buffers, so it seems like this work is not necessary now. Please re-open if there is still value to doing this work. 104726 2011-08-22 13:26:43 -0700 2011-08-22 13:43:41 -0700 Patch bug-66706-20110822162642.patch text/plain 2479 sullivan U3VidmVyc2lvbiBSZXZpc2lvbjogOTM1MzAKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDhk NmQxNTQyNGE5ZDE2NDg5YmZjMzljYjgwZDI3OWEzM2ZkNDgyYTkuLmRhOTRiZGJkNjA0NDQ3NDVm MjMyODA3OTA2MDU4MDQ2ZjE1MGQ1YzkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwpAQCAtMSwz ICsxLDE1IEBACisyMDExLTA4LTIyICBBbm5pZSBTdWxsaXZhbiAgPHN1bGxpdmFuQGNocm9taXVt Lm9yZz4KKworICAgICAgICBDaGFuZ2UgcmV0dXJuIHR5cGUgb2YgU3RyaW5nSW1wbDo6Y2hhcmFj dGVycygpIHRvIFN0cmluZ0l0ZXJhdG9yCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD02NjcwNgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgorCisgICAgICAgICogd3RmL3RleHQvU3RyaW5nSW1wbC5oOiBBZGRzIGEgdHlwZWRlZiBz byB0aGF0IFN0cmluZ0l0ZXJhdG9yIGlzIGNvbnN0IFVDaGFyKgorICAgICAgICAoV1RGOjpTdHJp bmdJbXBsOjpjaGFyYWN0ZXJzKTogQ2hhbmdlIHJldHVybiB0eXBlIHRvIFN0cmluZ0l0ZXJhdG9y CisgICAgICAgICogd3RmL3RleHQvV1RGU3RyaW5nLmg6CisgICAgICAgIChXVEY6OlN0cmluZzo6 Y2hhcmFjdGVycyk6IENoYW5nZSByZXR1cm4gdHlwZSB0byBTdHJpbmdJdGVyYXRvcgorCiAyMDEx LTA4LTIxICBNYXJ0aW4gUm9iaW5zb24gIDxtcm9iaW5zb25AaWdhbGlhLmNvbT4KIAogICAgICAg ICBGaXggJ21ha2UgZGlzdCcgZm9yIFdlYktpdEdUSysuCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvd3RmL3RleHQvU3RyaW5nSW1wbC5oIGIvU291cmNlL0phdmFTY3JpcHRDb3Jl L3d0Zi90ZXh0L1N0cmluZ0ltcGwuaAppbmRleCA1ZWM2ZDA3NTdjNmI1OTU3ZjYzNjY3MjJmZmIw MDAzNDUxY2E2YTg4Li4xMDY0OTlmODZlODdlMTcyNTk5ZmY5ZWE3NTU2NjBhNDFiN2UyNmY4IDEw MDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL3RleHQvU3RyaW5nSW1wbC5oCisr KyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvdGV4dC9TdHJpbmdJbXBsLmgKQEAgLTYzLDYg KzYzLDEwIEBAIHR5cGVkZWYgQ3Jvc3NUaHJlYWRSZWZDb3VudGVkPFNoYXJhYmxlVUNoYXI+IFNo YXJlZFVDaGFyOwogdHlwZWRlZiBib29sICgqQ2hhcmFjdGVyTWF0Y2hGdW5jdGlvblB0cikoVUNo YXIpOwogdHlwZWRlZiBib29sICgqSXNXaGl0ZVNwYWNlRnVuY3Rpb25QdHIpKFVDaGFyKTsKIAor Ly8gRklYTUU6IFRoaXMgdHlwZWRlZiBpcyBhIHRlbXBvcmFyeSBzdGVwIHRvIFN0cmluZ0ltcGwn cyA4LWJpdCBidWZmZXIgc3VwcG9ydC4KKy8vIEV2ZW50dWFsbHkgaXQgd2lsbCBiZSBhIGNsYXNz IHRvIHJlcGxhY2UgYWxtb3N0IGFsbCAnY29uc3QgVUNoYXIqJyB1c2FnZXMuCit0eXBlZGVmIGNv bnN0IFVDaGFyKiBTdHJpbmdJdGVyYXRvcjsKKwogY2xhc3MgU3RyaW5nSW1wbCA6IHB1YmxpYyBT dHJpbmdJbXBsQmFzZSB7CiAgICAgZnJpZW5kIHN0cnVjdCBKU0M6OklkZW50aWZpZXJDU3RyaW5n VHJhbnNsYXRvcjsKICAgICBmcmllbmQgc3RydWN0IEpTQzo6SWRlbnRpZmllclVDaGFyQnVmZmVy VHJhbnNsYXRvcjsKQEAgLTE5OSw3ICsyMDMsNyBAQCBwdWJsaWM6CiAgICAgc3RhdGljIFBhc3NS ZWZQdHI8U3RyaW5nSW1wbD4gYWRvcHQoU3RyaW5nQnVmZmVyJik7CiAKICAgICBTaGFyZWRVQ2hh ciogc2hhcmVkQnVmZmVyKCk7Ci0gICAgY29uc3QgVUNoYXIqIGNoYXJhY3RlcnMoKSBjb25zdCB7 IHJldHVybiBtX2RhdGE7IH0KKyAgICBTdHJpbmdJdGVyYXRvciBjaGFyYWN0ZXJzKCkgY29uc3Qg eyByZXR1cm4gbV9kYXRhOyB9CiAKICAgICBzaXplX3QgY29zdCgpCiAgICAgewpkaWZmIC0tZ2l0 IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3d0Zi90ZXh0L1dURlN0cmluZy5oIGIvU291cmNlL0ph dmFTY3JpcHRDb3JlL3d0Zi90ZXh0L1dURlN0cmluZy5oCmluZGV4IGZhOGNkY2QyYmI0MmQ3OGU5 M2RiZWM2ZWQ5ZDg5Mjk2NTMzMTExMGUuLjEzNTZlMjg2NDY4MzliZjU3YWQ3ZTEwOTA3YmYyODMz OTY2MDA4ZGEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvdGV4dC9XVEZT dHJpbmcuaAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL3RleHQvV1RGU3RyaW5nLmgK QEAgLTEzMiw3ICsxMzIsNyBAQCBwdWJsaWM6CiAgICAgICAgIHJldHVybiBtX2ltcGwtPmxlbmd0 aCgpOwogICAgIH0KIAotICAgIGNvbnN0IFVDaGFyKiBjaGFyYWN0ZXJzKCkgY29uc3QKKyAgICBT dHJpbmdJdGVyYXRvciBjaGFyYWN0ZXJzKCkgY29uc3QKICAgICB7CiAgICAgICAgIGlmICghbV9p bXBsKQogICAgICAgICAgICAgcmV0dXJuIDA7Cg== 104729 2011-08-22 13:43:46 -0700 2011-08-22 16:45:58 -0700 Patch bug-66706-20110822164344.patch text/plain 2542 sullivan U3VidmVyc2lvbiBSZXZpc2lvbjogOTM1MzAKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDhk NmQxNTQyNGE5ZDE2NDg5YmZjMzljYjgwZDI3OWEzM2ZkNDgyYTkuLjBlOWNjMmYxMjQ2NDAxZDFk YTZhM2FmOTNkZjQ4NmFmY2Q1OTY0ZDQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwpAQCAtMSwz ICsxLDE1IEBACisyMDExLTA4LTIyICBBbm5pZSBTdWxsaXZhbiAgPHN1bGxpdmFuQGNocm9taXVt Lm9yZz4KKworICAgICAgICBDaGFuZ2UgcmV0dXJuIHR5cGUgb2YgU3RyaW5nSW1wbDo6Y2hhcmFj dGVycygpIHRvIFN0cmluZ0NoYXJhY3Rlckl0ZXJhdG9yCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02NjcwNgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgICogd3RmL3RleHQvU3RyaW5nSW1wbC5oOiBBZGRzIGEg dHlwZWRlZiBzbyB0aGF0IFN0cmluZ0NoYXJhY3Rlckl0ZXJhdG9yIGlzIGNvbnN0IFVDaGFyKgor ICAgICAgICAoV1RGOjpTdHJpbmdJbXBsOjpjaGFyYWN0ZXJzKTogQ2hhbmdlIHJldHVybiB0eXBl IHRvIFN0cmluZ0NoYXJhY3Rlckl0ZXJhdG9yCisgICAgICAgICogd3RmL3RleHQvV1RGU3RyaW5n Lmg6CisgICAgICAgIChXVEY6OlN0cmluZzo6Y2hhcmFjdGVycyk6IENoYW5nZSByZXR1cm4gdHlw ZSB0byBTdHJpbmdDaGFyYWN0ZXJJdGVyYXRvcgorCiAyMDExLTA4LTIxICBNYXJ0aW4gUm9iaW5z b24gIDxtcm9iaW5zb25AaWdhbGlhLmNvbT4KIAogICAgICAgICBGaXggJ21ha2UgZGlzdCcgZm9y IFdlYktpdEdUSysuCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL3RleHQv U3RyaW5nSW1wbC5oIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3d0Zi90ZXh0L1N0cmluZ0ltcGwu aAppbmRleCA1ZWM2ZDA3NTdjNmI1OTU3ZjYzNjY3MjJmZmIwMDAzNDUxY2E2YTg4Li5lOGVlOGE4 MDMzYzE5ODQ4NTBiMTg5MmVhMGIwMGY1NzcwYTUwZDAzIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvd3RmL3RleHQvU3RyaW5nSW1wbC5oCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0 Q29yZS93dGYvdGV4dC9TdHJpbmdJbXBsLmgKQEAgLTYzLDYgKzYzLDEwIEBAIHR5cGVkZWYgQ3Jv c3NUaHJlYWRSZWZDb3VudGVkPFNoYXJhYmxlVUNoYXI+IFNoYXJlZFVDaGFyOwogdHlwZWRlZiBi b29sICgqQ2hhcmFjdGVyTWF0Y2hGdW5jdGlvblB0cikoVUNoYXIpOwogdHlwZWRlZiBib29sICgq SXNXaGl0ZVNwYWNlRnVuY3Rpb25QdHIpKFVDaGFyKTsKIAorLy8gRklYTUU6IFRoaXMgdHlwZWRl ZiBpcyBhIHRlbXBvcmFyeSBzdGVwIHRvIFN0cmluZ0ltcGwncyA4LWJpdCBidWZmZXIgc3VwcG9y dC4KKy8vIEV2ZW50dWFsbHkgaXQgd2lsbCBiZSBhIGNsYXNzIHRvIHJlcGxhY2UgYWxtb3N0IGFs bCAnY29uc3QgVUNoYXIqJyB1c2FnZXMuCit0eXBlZGVmIGNvbnN0IFVDaGFyKiBTdHJpbmdDaGFy YWN0ZXJJdGVyYXRvcjsKKwogY2xhc3MgU3RyaW5nSW1wbCA6IHB1YmxpYyBTdHJpbmdJbXBsQmFz ZSB7CiAgICAgZnJpZW5kIHN0cnVjdCBKU0M6OklkZW50aWZpZXJDU3RyaW5nVHJhbnNsYXRvcjsK ICAgICBmcmllbmQgc3RydWN0IEpTQzo6SWRlbnRpZmllclVDaGFyQnVmZmVyVHJhbnNsYXRvcjsK QEAgLTE5OSw3ICsyMDMsNyBAQCBwdWJsaWM6CiAgICAgc3RhdGljIFBhc3NSZWZQdHI8U3RyaW5n SW1wbD4gYWRvcHQoU3RyaW5nQnVmZmVyJik7CiAKICAgICBTaGFyZWRVQ2hhciogc2hhcmVkQnVm ZmVyKCk7Ci0gICAgY29uc3QgVUNoYXIqIGNoYXJhY3RlcnMoKSBjb25zdCB7IHJldHVybiBtX2Rh dGE7IH0KKyAgICBTdHJpbmdDaGFyYWN0ZXJJdGVyYXRvciBjaGFyYWN0ZXJzKCkgY29uc3QgeyBy ZXR1cm4gbV9kYXRhOyB9CiAKICAgICBzaXplX3QgY29zdCgpCiAgICAgewpkaWZmIC0tZ2l0IGEv U291cmNlL0phdmFTY3JpcHRDb3JlL3d0Zi90ZXh0L1dURlN0cmluZy5oIGIvU291cmNlL0phdmFT Y3JpcHRDb3JlL3d0Zi90ZXh0L1dURlN0cmluZy5oCmluZGV4IGZhOGNkY2QyYmI0MmQ3OGU5M2Ri ZWM2ZWQ5ZDg5Mjk2NTMzMTExMGUuLjFjMzZkZGJmM2MyODA0ZWRkZTZlNWYxNWI0Nzc1ZDA5NGI0 YjE3YzQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvdGV4dC9XVEZTdHJp bmcuaAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL3RleHQvV1RGU3RyaW5nLmgKQEAg LTEzMiw3ICsxMzIsNyBAQCBwdWJsaWM6CiAgICAgICAgIHJldHVybiBtX2ltcGwtPmxlbmd0aCgp OwogICAgIH0KIAotICAgIGNvbnN0IFVDaGFyKiBjaGFyYWN0ZXJzKCkgY29uc3QKKyAgICBTdHJp bmdDaGFyYWN0ZXJJdGVyYXRvciBjaGFyYWN0ZXJzKCkgY29uc3QKICAgICB7CiAgICAgICAgIGlm ICghbV9pbXBsKQogICAgICAgICAgICAgcmV0dXJuIDA7Cg== 104765 2011-08-22 16:46:04 -0700 2012-01-22 14:27:52 -0800 Patch bug-66706-20110822194602.patch text/plain 3231 sullivan U3VidmVyc2lvbiBSZXZpc2lvbjogOTM1NTAKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDhk NmQxNTQyNGE5ZDE2NDg5YmZjMzljYjgwZDI3OWEzM2ZkNDgyYTkuLjcxMjExYzU3NWY4ODhhZTdm NzQyOGNlMWVhYmNjZTE3MmQ5ZjUwMjggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwpAQCAtMSwz ICsxLDIxIEBACisyMDExLTA4LTIyICBBbm5pZSBTdWxsaXZhbiAgPHN1bGxpdmFuQGNocm9taXVt Lm9yZz4KKworICAgICAgICBUeXBlZGVmIGNvbnN0IFVDaGFyKiB0byBTdHJpbmdDb25zdENoYXJh Y3Rlckl0ZXJhdG9yCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD02NjcwNgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIFRoaXMgcGF0Y2ggaXMgdG8gaGVscCB3aXRoIHJlZHVjaW5nIGNhbGxlcnMgdG8gU3RyaW5n OjpjaGFyYWN0ZXJzIHdoaWNoIGRvbid0IGFjdHVhbGx5CisgICAgICAgIG5lZWQgdG8gYWNjZXNz IHRoZSB1bmRlcmx5aW5nIGRhdGEgZm9ybWF0LiBJdCBhZGRzIGEgdHlwZWRlZiBmb3IgU3RyaW5n Q29uc3RDaGFyYWN0ZXJJdGVyYXRvcgorICAgICAgICBhbmQgYSBtZXRob2QgY2hhcmFjdGVySXRl cmF0b3IoKSB0aGF0IHJldHVybnMgdGhhdCB0eXBlZGVmLiBVc2FnZXMgb2YgU3RyaW5nOjpvcGVy YXRvcltdCisgICAgICAgIHdpbGwgYmUgY29udmVydGVkIHRvIGNhbGwgY2hhcmFjdGVySXRlcmF0 b3IoKSwgYW5kIGFmdGVyIHRoZSBjb252ZXJzaW9uIFN0cmluZ0NvbnN0Q2hhcmFjdGVySXRlcmF0 b3IKKyAgICAgICAgd2lsbCBiZSBpbXBsZW1lbnRlZCBhcyBhIGNsYXNzIHRoYXQgY2FuIGhhbmRs ZSA4LWJpdCBvciAxNi1iaXQgY2hhcmFjdGVyIGJ1ZmZlcnMuCisKKyAgICAgICAgKiB3dGYvdGV4 dC9TdHJpbmdJbXBsLmg6IFR5cGVkZWYgY29uc3QgVUNoYXIqIHRvIFN0cmluZ0NvbnN0Q2hhcmFj dGVySXRlcmF0b3IKKyAgICAgICAgKFdURjo6U3RyaW5nSW1wbDo6Y2hhcmFjdGVySXRlcmF0b3Ip OiBBZGQgbmV3IG1ldGhvZCB0byBiZSB1c2VkIGluIGNvbnZlcnNpb24gdG8gU3RyaW5nQ29uc3RD aGFyYWN0ZXJJdGVyYXRvcgorICAgICAgICAqIHd0Zi90ZXh0L1dURlN0cmluZy5oOgorICAgICAg ICAoV1RGOjpTdHJpbmc6OmNoYXJhY3Rlckl0ZXJhdG9yKTogQWRkIG5ldyBtZXRob2QgdG8gYmUg dXNlZCBpbiBjb252ZXJzaW9uIHRvIFN0cmluZ0NvbnN0Q2hhcmFjdGVySXRlcmF0b3IKKwogMjAx MS0wOC0yMSAgTWFydGluIFJvYmluc29uICA8bXJvYmluc29uQGlnYWxpYS5jb20+CiAKICAgICAg ICAgRml4ICdtYWtlIGRpc3QnIGZvciBXZWJLaXRHVEsrLgpkaWZmIC0tZ2l0IGEvU291cmNlL0ph dmFTY3JpcHRDb3JlL3d0Zi90ZXh0L1N0cmluZ0ltcGwuaCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS93dGYvdGV4dC9TdHJpbmdJbXBsLmgKaW5kZXggNWVjNmQwNzU3YzZiNTk1N2Y2MzY2NzIyZmZi MDAwMzQ1MWNhNmE4OC4uNTEyOTMxNjNkZGI5ZjYwMjA2ZjgyNDA1MWM0NTk5ZGFlYTU3ZGIxMSAx MDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3d0Zi90ZXh0L1N0cmluZ0ltcGwuaAor KysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL3RleHQvU3RyaW5nSW1wbC5oCkBAIC02Myw2 ICs2MywxMCBAQCB0eXBlZGVmIENyb3NzVGhyZWFkUmVmQ291bnRlZDxTaGFyYWJsZVVDaGFyPiBT aGFyZWRVQ2hhcjsKIHR5cGVkZWYgYm9vbCAoKkNoYXJhY3Rlck1hdGNoRnVuY3Rpb25QdHIpKFVD aGFyKTsKIHR5cGVkZWYgYm9vbCAoKklzV2hpdGVTcGFjZUZ1bmN0aW9uUHRyKShVQ2hhcik7CiAK Ky8vIEZJWE1FOiBUaGlzIHR5cGVkZWYgaXMgYSB0ZW1wb3Jhcnkgc3RlcCB0byBTdHJpbmdJbXBs J3MgOC1iaXQgYnVmZmVyIHN1cHBvcnQuCisvLyBFdmVudHVhbGx5IGl0IHdpbGwgYmUgYSBjbGFz cyB0byByZXBsYWNlIGFsbW9zdCBhbGwgJ2NvbnN0IFVDaGFyKicgdXNhZ2VzLgordHlwZWRlZiBj b25zdCBVQ2hhciogU3RyaW5nQ29uc3RDaGFyYWN0ZXJJdGVyYXRvcjsKKwogY2xhc3MgU3RyaW5n SW1wbCA6IHB1YmxpYyBTdHJpbmdJbXBsQmFzZSB7CiAgICAgZnJpZW5kIHN0cnVjdCBKU0M6Oklk ZW50aWZpZXJDU3RyaW5nVHJhbnNsYXRvcjsKICAgICBmcmllbmQgc3RydWN0IEpTQzo6SWRlbnRp ZmllclVDaGFyQnVmZmVyVHJhbnNsYXRvcjsKQEAgLTIwMCw2ICsyMDQsNyBAQCBwdWJsaWM6CiAK ICAgICBTaGFyZWRVQ2hhciogc2hhcmVkQnVmZmVyKCk7CiAgICAgY29uc3QgVUNoYXIqIGNoYXJh Y3RlcnMoKSBjb25zdCB7IHJldHVybiBtX2RhdGE7IH0KKyAgICBTdHJpbmdDb25zdENoYXJhY3Rl ckl0ZXJhdG9yIGNoYXJhY3Rlckl0ZXJhdG9yKCkgY29uc3QgeyByZXR1cm4gbV9kYXRhOyB9CiAK ICAgICBzaXplX3QgY29zdCgpCiAgICAgewpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRD b3JlL3d0Zi90ZXh0L1dURlN0cmluZy5oIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3d0Zi90ZXh0 L1dURlN0cmluZy5oCmluZGV4IGZhOGNkY2QyYmI0MmQ3OGU5M2RiZWM2ZWQ5ZDg5Mjk2NTMzMTEx MGUuLjFkZTYxN2M2YmIxMzI4YmRiMDI5ZTA3ZTdkNTkxMGY2NzUzMmY5ZDcgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvdGV4dC9XVEZTdHJpbmcuaAorKysgYi9Tb3VyY2Uv SmF2YVNjcmlwdENvcmUvd3RmL3RleHQvV1RGU3RyaW5nLmgKQEAgLTEzOSw2ICsxMzksMTMgQEAg cHVibGljOgogICAgICAgICByZXR1cm4gbV9pbXBsLT5jaGFyYWN0ZXJzKCk7CiAgICAgfQogCisg ICAgU3RyaW5nQ29uc3RDaGFyYWN0ZXJJdGVyYXRvciBjaGFyYWN0ZXJJdGVyYXRvcigpIGNvbnN0 CisgICAgeworICAgICAgICBpZiAoIW1faW1wbCkKKyAgICAgICAgICAgIHJldHVybiAwOworICAg ICAgICByZXR1cm4gbV9pbXBsLT5jaGFyYWN0ZXJJdGVyYXRvcigpOworICAgIH0KKwogICAgIFdU Rl9FWFBPUlRfUFJJVkFURSBDU3RyaW5nIGFzY2lpKCkgY29uc3Q7CiAgICAgV1RGX0VYUE9SVF9Q UklWQVRFIENTdHJpbmcgbGF0aW4xKCkgY29uc3Q7CiAgICAgV1RGX0VYUE9SVF9QUklWQVRFIENT dHJpbmcgdXRmOChib29sIHN0cmljdCA9IGZhbHNlKSBjb25zdDsK