[chromedriver] Add shadow DOM support to chromedriver.

[chromedriver] Add shadow DOM support to chromedriver.

BUG=chromedriver:852
TEST= added some unit tests (beginning with testShadowDom) to chrome/test/chromedriver/test/run_py_tests.py

R=samuong@chromium.org

Committed: https://crrev.com/713e22356d84f46633890df2be12860b0d6a16ee
Cr-Commit-Position: refs/heads/master@{#309002}

[samuong, 2014-08-25 17:20:35]

The changes look good to me so far but I'm not able to run the tests because the
CL no longer applies cleanly. Could you please do a rebase and another git cl
upload?

I think the rebase should also fix the issue that you're having with the atoms.*
files not being minimal. We've made a few changes to the atoms recently so your
patch doesn't apply cleanly anymore. So you'll need to:

1. rebase your chromium sources (git fetch origin && git rebase origin/master &&
gclient sync)
2. for your selenium checkout, do a fetch and reset to
37ece1505b0c6856c3a5ae84b3d47c8e789ecb88
3. apply our patch, which is in chromium/src/third_party/webdriver/patch.diff
4. reapply your shadow dom patch to the selenium sources, fixing any conflicts
that you get
5. regenerate atoms.* and do a git cl upload

If these are still not minimal, please let me know and I'll help you get this
working.

[chromium-reviews, 2014-08-25 17:38:42]

Will do, but next week. I'm holidaying this week - in the driving rain :-(

Mark
On 25 Aug 2014 18:20, <samuong@chromium.org> wrote:

> The changes look good to me so far but I'm not able to run the tests
> because the
> CL no longer applies cleanly. Could you please do a rebase and another git
> cl
> upload?
>
> I think the rebase should also fix the issue that you're having with the
> atoms.*
> files not being minimal. We've made a few changes to the atoms recently so
> your
> patch doesn't apply cleanly anymore. So you'll need to:
>
> 1. rebase your chromium sources (git fetch origin && git rebase
> origin/master &&
> gclient sync)
> 2. for your selenium checkout, do a fetch and reset to
> 37ece1505b0c6856c3a5ae84b3d47c8e789ecb88
> 3. apply our patch, which is in chromium/src/third_party/
> webdriver/patch.diff
> 4. reapply your shadow dom patch to the selenium sources, fixing any
> conflicts
> that you get
> 5. regenerate atoms.* and do a git cl upload
>
> If these are still not minimal, please let me know and I'll help you get
> this
> working.
>
> https://codereview.chromium.org/480483002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2014-09-01 15:24:52]

Done. I couldn't get the patch to apply cleanly: had to manually put in the
changes to build.desc.

PTAL at https://codereview.chromium.org/529763002



On Mon, Aug 25, 2014 at 6:38 PM, Mark Charsley <mcharsley@google.com> wrote:

> Will do, but next week. I'm holidaying this week - in the driving rain :-(
>
> Mark
> On 25 Aug 2014 18:20, <samuong@chromium.org> wrote:
>
>> The changes look good to me so far but I'm not able to run the tests
>> because the
>> CL no longer applies cleanly. Could you please do a rebase and another
>> git cl
>> upload?
>>
>> I think the rebase should also fix the issue that you're having with the
>> atoms.*
>> files not being minimal. We've made a few changes to the atoms recently
>> so your
>> patch doesn't apply cleanly anymore. So you'll need to:
>>
>> 1. rebase your chromium sources (git fetch origin && git rebase
>> origin/master &&
>> gclient sync)
>> 2. for your selenium checkout, do a fetch and reset to
>> 37ece1505b0c6856c3a5ae84b3d47c8e789ecb88
>> 3. apply our patch, which is in chromium/src/third_party/
>> webdriver/patch.diff
>> 4. reapply your shadow dom patch to the selenium sources, fixing any
>> conflicts
>> that you get
>> 5. regenerate atoms.* and do a git cl upload
>>
>> If these are still not minimal, please let me know and I'll help you get
>> this
>> working.
>>
>> https://codereview.chromium.org/480483002/
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[samuong, 2014-09-02 20:58:00]

I think it would be better to upload a new patch set to this issue, rather than
create a new one. That way we preserve the change history. You can do this by
typing:

cd chromium/src
git cl issue 480483002
git cl upload

I'm surprised that you can't get the patch to apply cleanly. Could you let me
know what revision of the chromium repo you are based on, and what revision of
the selenium repo you are trying to apply the patch to?

And finally, please also append your selenium patch to
chromium/src/third_party/webdriver/patch.diff. We can remove it from our local
patches once it gets committed upstream, but for now it is better to have it
there in case anyone wants to regenerate atoms.cc/atoms.h.

Sam.

On 2014/09/01 15:24:52, chromium-reviews wrote:
> Done. I couldn't get the patch to apply cleanly: had to manually put in the
> changes to build.desc.
> 
> PTAL at https://codereview.chromium.org/529763002
> 
> 
> 
> On Mon, Aug 25, 2014 at 6:38 PM, Mark Charsley <mailto:mcharsley@google.com>
wrote:
> 
> > Will do, but next week. I'm holidaying this week - in the driving rain :-(
> >
> > Mark
> > On 25 Aug 2014 18:20, <mailto:samuong@chromium.org> wrote:
> >
> >> The changes look good to me so far but I'm not able to run the tests
> >> because the
> >> CL no longer applies cleanly. Could you please do a rebase and another
> >> git cl
> >> upload?
> >>
> >> I think the rebase should also fix the issue that you're having with the
> >> atoms.*
> >> files not being minimal. We've made a few changes to the atoms recently
> >> so your
> >> patch doesn't apply cleanly anymore. So you'll need to:
> >>
> >> 1. rebase your chromium sources (git fetch origin && git rebase
> >> origin/master &&
> >> gclient sync)
> >> 2. for your selenium checkout, do a fetch and reset to
> >> 37ece1505b0c6856c3a5ae84b3d47c8e789ecb88
> >> 3. apply our patch, which is in chromium/src/third_party/
> >> webdriver/patch.diff
> >> 4. reapply your shadow dom patch to the selenium sources, fixing any
> >> conflicts
> >> that you get
> >> 5. regenerate atoms.* and do a git cl upload
> >>
> >> If these are still not minimal, please let me know and I'll help you get
> >> this
> >> working.
> >>
> >> https://codereview.chromium.org/480483002/
> >>
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2014-09-03 16:03:08]

On Tue, Sep 2, 2014 at 9:58 PM, <samuong@chromium.org> wrote:

> I think it would be better to upload a new patch set to this issue, rather
> than
> create a new one. That way we preserve the change history. You can do this
> by
> typing:
>
> cd chromium/src
> git cl issue 480483002
> git cl upload
>

Done


> I'm surprised that you can't get the patch to apply cleanly. Could you let
> me
> know what revision of the chromium repo you are based on, and what
> revision of
> the selenium repo you are trying to apply the patch to?
>

The problem appears to be due to me applying my changes first. If I create
a fresh selenium tree, and apply the chromium patch first, then it works
fine (and then my patch fails to work).

And finally, please also append your selenium patch to
> chromium/src/third_party/webdriver/patch.diff. We can remove it from our
> local
> patches once it gets committed upstream, but for now it is better to have
> it
> there in case anyone wants to regenerate atoms.cc/atoms.h.
>

Will do, but that will have to wait until tomorrow, things have been a bit
hectic today :-(

Mark

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[samuong, 2014-12-16 07:56:59]

lgtm with nits

The issue description ends up being used as the git log message. So could you
please change this to the following?

[chromedriver] Add shadow DOM support to chromedriver.

BUG=chromedriver:852

https://codereview.chromium.org/480483002/diff/40001/chrome/test/chromedriver...
File chrome/test/chromedriver/test/run_py_tests.py (right):

https://codereview.chromium.org/480483002/diff/40001/chrome/test/chromedriver...
chrome/test/chromedriver/test/run_py_tests.py:813: # the butotn's ohClicked
handler changes the text box's value
"button's onClicked"?

https://codereview.chromium.org/480483002/diff/40001/chrome/test/data/chromed...
File chrome/test/data/chromedriver/shadow_dom_test.html (right):

https://codereview.chromium.org/480483002/diff/40001/chrome/test/data/chromed...
chrome/test/data/chromedriver/shadow_dom_test.html:65: 
trim away blank lines at the end of the file, and close with </body> and </html>

[mcharsley, 2014-12-18 13:03:42]

Nits fixed. Thanks

https://codereview.chromium.org/480483002/diff/40001/chrome/test/chromedriver...
File chrome/test/chromedriver/test/run_py_tests.py (right):

https://codereview.chromium.org/480483002/diff/40001/chrome/test/chromedriver...
chrome/test/chromedriver/test/run_py_tests.py:813: # the butotn's ohClicked
handler changes the text box's value
On 2014/12/16 07:56:59, samuong wrote:
> "button's onClicked"?

Done.

https://codereview.chromium.org/480483002/diff/40001/chrome/test/data/chromed...
File chrome/test/data/chromedriver/shadow_dom_test.html (right):

https://codereview.chromium.org/480483002/diff/40001/chrome/test/data/chromed...
chrome/test/data/chromedriver/shadow_dom_test.html:65: 
On 2014/12/16 07:56:59, samuong wrote:
> trim away blank lines at the end of the file, and close with </body> and
</html>

Done.

[mcharsley, 2014-12-18 13:04:46]

The CQ bit was checked by mcharsley@google.com

[commit-bot: I haz the power, 2014-12-18 13:05:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/480483002/60001

[commit-bot: I haz the power, 2014-12-18 14:02:30]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-12-18 14:03:08]

Patchset 4 (id:??) landed as
https://crrev.com/713e22356d84f46633890df2be12860b0d6a16ee
Cr-Commit-Position: refs/heads/master@{#309002}

[samuong, 2014-12-22 06:24:26]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/824603002/ by samuong@chromium.org.

The reason for reverting is: This CL breaks
ElementEqualityTest.testAnElementFoundInADifferentFrameViaJsShouldHaveSameId and
ElementFindingTest.testAnElementFoundInADifferentFrameViaJsCanBeUsed.

[chromium-reviews, 2015-01-05 17:20:01]

Hi Sam. I'm back in the office now. Will take a look tomorrow.
On 22 Dec 2014 06:24, <samuong@chromium.org> wrote:

> A revert of this CL (patchset #4 id:60001) has been created in
> https://codereview.chromium.org/824603002/ by samuong@chromium.org.
>
> The reason for reverting is: This CL breaks
> ElementEqualityTest.testAnElementFoundInADifferentFrameViaJsShouldHaveSameId
> and
> ElementFindingTest.testAnElementFoundInADifferentFrameViaJsCanBeUsed.
>
> https://codereview.chromium.org/480483002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.